Re: [RFC] RT-patch update to remove the global pi_lock

[Steven Rostedt]

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 <rostedt@xxxxxxxxxxx>

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/