[PATCH -RT] Deadlock on down_trylock

[Steven Rostedt <rostedt@goodmis.org>]

Hi Ingo,

I've just uncovered a nasty little deadlock in rt.c wrt __down_trylock.
It started with the following code in fs/ext3/super.c (Always it's
because of ext3 ;)

static void ext3_write_super (struct super_block * sb)
{
	if (down_trylock(&sb->s_lock) == 0)
		BUG();
	sb->s_dirt = 0;
}

Now what it is doing here is testing to make sure that the down_trylock
fails. Sort of a assert_is_spin_locked for a semaphore.  But
unfortunately, my pi_locking code in rt.c doesn't account for this.

The locking order of the task->pi_lock and lock->wait_lock is very
dependent on the order of locks taken in the kernel that would not cause
a deadlock.  But trylock is not bound to this order, and it is even OK,
as seen above in ext3, for an owner to grab a lock that it owns with
trylock.

So to handle this, I'm supplying the following patch.  My machine with
my kernel and debugging would take about one or two hours to crash on
this, and it hasn't reached that point yet. But I believe that this
patch should fix the problem.  If my machine locks up again, I'll let
you know, and supply another patch ASAP.

-- Steve

PS.  Whose lovely idea was it to have spin_trylock return a one on
success, and down_trylock return a zero on success???  It took me a few
tries to get this right! (but please review it just in case)


Index: linux-2.6.14-rt22/kernel/rt.c
===================================================================
--- linux-2.6.14-rt22.orig/kernel/rt.c	2005-12-06 21:44:53.000000000 -0500
+++ linux-2.6.14-rt22/kernel/rt.c	2005-12-08 09:59:04.000000000 -0500
@@ -1970,7 +1970,15 @@
 	trace_lock_irqsave(&trace_lock, flags, ti);
 	TRACE_BUG_ON_LOCKED(!raw_irqs_disabled());
 	_raw_spin_lock(&task->pi_lock);
-	_raw_spin_lock(&lock->wait_lock);
+	/*
+	 * It is OK for the owner of the lock to do a trylock on
+	 * a lock it owns, so to prevent deadlocking, we must
+	 * do a trylock here, since another process may be trying
+	 * to grab this lock, and the order of these locks is
+	 * very important.
+	 */
+	if (!_raw_spin_trylock(&lock->wait_lock))
+		goto failed;
 
 	old_owner = lock_owner(lock);
 	init_lists(lock);
@@ -1987,6 +1995,7 @@
 		ret = 1;
 	}
 	_raw_spin_unlock(&lock->wait_lock);
+failed:
 	_raw_spin_unlock(&task->pi_lock);
 	trace_unlock_irqrestore(&trace_lock, flags, ti);
 


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