Messages in this thread Patch in this message | | | Date | Wed, 22 Feb 2006 17:50:53 +0100 (MET) | From | Esben Nielsen <> | Subject | Re: 2.6.15-rt17 |
| |
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/
| |