Re: 2.6.15-rt17

[Esben Nielsen <simlo@phys.au.dk>]

On Tue, 21 Feb 2006, Ingo Molnar wrote:

> i have released the 2.6.15-rt17 tree, which can be downloaded from the
> usual place:
>
>    http://redhat.com/~mingo/realtime-preempt/
>
> lots of changes all across the map. There are several bigger changes:
>
> the biggest change is the new PI code from Esben Nielsen, Thomas
> Gleixner and Steven Rostedt. This big rework simplifies and streamlines
> the PI code, and fixes a couple of bugs and races:
>
I didn't know anyone looked at my patch! I am ofcourse happy about it :-)

I switftly looked at 2.6.15-rt17 to see how it turned out. I found one
problem in get_blocked_on(task):

First the task->pi_lock is locked, lock = task->blocked_on->lock, then
task->pi_lock is unlocked. Now lock is used. This is not safe.
Once task->pi_lock is unlocked we can be preempted and wait for a long
while the lock can be freed (if forinstance a driver is unloaded). You
either have to hold task->pi_lock or lock->wait_lock while refering to
lock. Otherwise the lock can be deallocated behind your bag. I assume that
nobody deallocate a lock with waiters.

That was why I had _reversed_ the lock ordering relative to normal in the
original patch: First lock task->pi_lock. Assign lock. Lock
lock->wait_lock. Then unlock task->pi_lock. Now it is safe to refer to
lock. To avoid deadlocks I used _raw_spin_trylock() when locking the 2.
lock.

To make a long story short:

--- linux-2.6.15-rt17/kernel/rt.c.orig  2006-02-22 16:53:44.000000000
+0100
+++ linux-2.6.15-rt17/kernel/rt.c       2006-02-22 16:56:21.000000000
+0100
@@ -873,10 +873,17 @@
        }

        lock = task->blocked_on->lock;
+
+       /* Now we have to take lock->wait_lock _before_ releasing
+          task->pi_lock. Otherwise lock can be deallocated while we are
+          refering to it as the subsystem has no way of knowing about us
+          hanging around in here. */
+       if (!_raw_spin_trylock(&lock->wait_lock)) {
+               _raw_spin_unlock(&task->pi_lock);
+               goto try_again;
+        }
        _raw_spin_unlock(&task->pi_lock);

-       if (!_raw_spin_trylock(&lock->wait_lock))
-               goto try_again;

        owner = lock_owner(lock);
        if (owner)

It compiles, but I have not tested it. I can't see why it shouldn't work.

Esben

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/