On Thu, 2005-08-25 at 16:09 -0400, Steven Rostedt wrote:
> A word of caution (aka. disclaimer). This is still new. I still expect
> there are some cases in the code that was missed and can cause a dead
> lock or other bad side effect. Hopefully, we can iron these all out.
> Also, I noticed that since the task takes it's own pi_lock for most of
> the code, if something locks up and a NMI goes off, the down_trylock in
> printk will also lock when it tries to take it's own pi_lock.
OK, found my first bug :-)
Just so everyone knows. In rt.c, all pi_waiter access (reading or
writing) must be protected by the task's pi_lock, and all access to the
lock's wait_list must be protected by the lock's wait_lock. The magic
is in the locking order :-).
-- Steve
Signed-off-by: Steven Rostedt <[email protected]>
Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c (revision 306)
+++ linux_realtime_goliath/kernel/rt.c (working copy)
@@ -671,6 +671,7 @@
struct rt_mutex_waiter *w;
struct plist *curr1;
+ __raw_spin_lock(old_owner->task->pi_lock);
TRACE_WARN_ON_LOCKED(plist_empty(&waiter->pi_list));
TRACE_WARN_ON_LOCKED(lock_owner(lock));
@@ -681,6 +682,7 @@
}
TRACE_WARN_ON_LOCKED(1);
ok:
+ __raw_spin_unlock(old_owner->task->pi_lock);
return;
}
@@ -734,6 +736,8 @@
if (old_owner == new_owner)
return;
+ TRACE_BUG_ON_LOCKED(!spin_is_locked(&old_owner->task->pi_lock));
+ TRACE_BUG_ON_LOCKED(!spin_is_locked(&new_owner->task->pi_lock));
plist_for_each_safe(curr1, next1, &old_owner->task->pi_waiters) {
w = plist_entry(curr1, struct rt_mutex_waiter, pi_list);
if (w->lock == lock) {
@@ -932,6 +936,8 @@
/*
* Add SCHED_NORMAL tasks to the end of the waitqueue (FIFO):
*/
+ TRACE_BUG_ON_LOCKED(!spin_is_locked(&task->pi_lock));
+ TRACE_BUG_ON_LOCKED(!spin_is_locked(&lock->wait_lock));
#ifndef ALL_TASKS_PI
if (!rt_task(task)) {
plist_add(&waiter->list, &lock->wait_list);
@@ -939,6 +945,7 @@
return;
}
#endif
+ __raw_spin_lock(&lock_owner(lock)->task->pi_lock);
plist_add(&waiter->pi_list, &lock_owner(lock)->task->pi_waiters);
/*
* Add RT tasks to the head:
@@ -949,11 +956,9 @@
* If the waiter has higher priority than the owner
* then temporarily boost the owner:
*/
- if (task->prio < lock_owner(lock)->task->prio) {
- __raw_spin_lock(&lock_owner(lock)->task->pi_lock);
+ if (task->prio < lock_owner(lock)->task->prio)
pi_setprio(lock, lock_owner(lock)->task, task->prio);
- __raw_spin_unlock(&lock_owner(lock)->task->pi_lock);
- }
+ __raw_spin_unlock(&lock_owner(lock)->task->pi_lock);
}
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |