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

[Steven Rostedt]

Ingo,

This patch contains my previous change as well as an update to fix the
race conditions that the BKL may hold.  It is against -rt2. 

The first part of the patch will stop the pi_setprio loop if the process
has a lock_depth greater than or equal to zero.  Since that would mean
that the process either is running, or is about to release the BKL.

I still kept the change from rt1 to rt2 but changed the comment.

I added the lock release logic in the __up code incase the BKL is
causing loops in the pi chain.

I'm currently runnig -rt2 with these changes as I type.

-- Steve

Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>

Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c	(revision 310)
+++ linux_realtime_goliath/kernel/rt.c	(working copy)
@@ -800,7 +800,24 @@
 #endif
 
 		mutex_setprio(p, prio);
-		if (!w)
+
+		/*
+		 * The BKL can really be a pain. It can happen where the
+		 * BKL is being held by one task that is just about to
+		 * block on another task that is waiting for the BKL.
+		 * This isn't a deadlock, since the BKL is released
+		 * when the task goes to sleep.  This also means that
+		 * all holders of the BKL are not blocked, or are just
+		 * about to be blocked.
+		 *
+		 * Another side-effect of this is that there's a small
+		 * window where the spinlocks are not held, and the blocked
+		 * process hasn't released the BKL.  So if we are going
+		 * to boost the owner of the BKL, stop after that,
+		 * since that owner is either running, or about to sleep
+		 * but don't go any further or we are in a loop.
+		 */
+		if (!w || unlikely(p->lock_depth >= 0))
 			break;
 		/*
 		 * If the task is blocked on a lock, and we just made
@@ -817,10 +834,9 @@
 		TRACE_BUG_ON_LOCKED(!lock);
 
 		/*
-		 * The BKL can really be a pain.  It can happen that the lock
-		 * we are blocked on is owned by a task that is waiting for
-		 * the BKL, and we own it.  So, if this is the BKL and we own
-		 * it, then end the loop here.
+		 * The current task that is blocking can also the one
+		 * holding the BKL, and blocking on a task that wants 
+		 * it.  So if it were to get this far, we would deadlock.
 		 */
 		if (unlikely(l == &kernel_sem.lock) && lock_owner(l) == current_thread_info()) {
 			/*
@@ -1089,11 +1105,21 @@
 		__raw_spin_unlock(&new_owner->task->pi_lock);
 		goto try_again;
 	}
+	/*
+	 * Once again the BKL comes to play.  Since the BKL can be grabbed and released
+	 * out of the normal P1->L1->P2 order, there's a chance that someone has the
+	 * BKL owner's lock and is waiting on the new owner lock.
+	 */
+	if (unlikely(lock == &kernel_sem.lock)) {
+		if (!__raw_spin_trylock(&old_owner->task->pi_lock)) {
+			__raw_spin_unlock(&new_owner->task->pi_lock);
+			goto try_again;
+		}
+	} else
 #endif
+		__raw_spin_lock(&old_owner->task->pi_lock);
+
 	plist_del_init(&waiter->list, &lock->wait_list);
-
-	__raw_spin_lock(&old_owner->task->pi_lock);
-
 	plist_del(&waiter->pi_list, &old_owner->task->pi_waiters);
 	plist_init(&waiter->pi_list, waiter->ti->task->prio);
 


-
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/