Re: [ANNOUNCE] 3.2.9-rt17

On Fri, 2012-03-09 at 01:33 +0100, Thomas Gleixner wrote:

On Thu, 8 Mar 2012, Steven Rostedt wrote:

quoted

On Thu, 2012-03-08 at 22:20 +0100, Peter Zijlstra wrote:

quoted

Now put the thing on 2 cpus and both tasks can endlessly chase each
other's tail, no? 

How would this be different than what mainline does? When the lock is
released, it will wake up the other task.

Nonsense. That code is not causing any headache in mainline simply
because the lock holder cannot be preempted. So the contention case
runs on different cpus. On RT the failure case is when the trylocker
preempts the lock holder, which cannot be moved to a different cpu due
to the implicit migrate disable. Aside of that cpu_relax() and ticket
locks are there for a reason. They allow the other cpu to make
progress instead of allowing the trylocking cpu to monopolize the
cache line forever.

I understand that, I was replying to the "both tasks can endlessly chase
each other's tail". And I gave responses to that. The current solution
of having the task sleep for a tick still doesn't solve the issue of a
the owner being preempted by a higher priority task.

At least the solution I proposed wouldn't cause priority inversion,
where as the current solution can.

task-a (cpu 0)                     task-b(cpu1)        task-c(cpu1)
--------------                     ------------        ------------

retry:                             lock(y);

    lock(x);
                                        <<-------------  preempt task-b
    if (!try_lock(y)) {
	unlock(x);
	sleep(1);
	goto retry;
    }


Now task-a can be of the highest priority task in the system, and task-b
the lowest, but task-c is higher than task-b and lower than task-a. If c
is a CPU hog, then task-a will never get out of this loop.

With the lock inheritance, b will get to run over c.

The only case where mainline can fail is when a high prio task does a
mutex_trylock() loop and the mutex owner and the trylocker are pinned
on the same core. Though I have not yet found code like that, but I
have not looked too hard either :)

It's a simple RT problem, which has been there forever, but obviously
nobody did stress tests such code pathes on UP systems or if someone
did he was not able to trigger it. On SMP this was not a big issue
when task migration was almost always enabled. Due to the implicit
migrate disable withing spinlocked regions we just made it more likely
to happen.

Right, and I mentioned that the migrate disable causes new issues.

I'm trying to come up with a solution that doesn't "wait for some magic
event which is associated to the unlock of i_lock". Because that's what
we are doing right now. The magic event is the sleep hoping that it will
unlock the lock. And a solution that isn't as nasty as the multiple
readers lock.

Maybe the lock inheritance isn't the best solution, but I believe it's
better than the sleep and hope solution that's there now. And I don't
think it would be that complex to implement.

-- Steve