[PATCH] Fix __raw_read_trylock() to allow multiple readers

[PATCH] Fix __raw_read_trylock() to allow multiple readers

[Dave Johnson]

Code is from linux-mips.org's git tree at 2.6.18 plus some patches
from 2.6.18.7.

I've run into a deadlock that occurs with the following config:

CONFIG_SMP
CONFIG_PREEMPT

An individual call to __raw_read_trylock() will just return failure if
there is already a reader.

Any code that tries to take a read lock by looping around
__raw_read_trylock() can deadlock if there is already a reader and the
lock in question has a mix of irq and non-irq readers.

This is the case in the _read_lock() from BUILD_LOCK_OPS() in
kernel/spinlock.c.

Perhaps someone can confirm this bug by a code inspection, but I
think the below patch will fix the issue.

Patch should apply ok to 2.6.18.7 (and a similar one to >= 2.6.19)

-- 
Dave Johnson
Starent Networks

========================  PATCH FOLLOWS  ========================

Fix __raw_read_trylock() to allow multiple readers.

A deadlock can occur for mixed irq and non-irq rwlock readers if
a 2nd reader attempts to take lock by looping around __raw_read_trylock().

Signed-off-by: Dave Johnson <djohnson+linux-mips@sw.starentnetworks.com>

--- old/include/asm-mips/spinlock.h	2007-03-05 20:34:51.000000000 -0500
+++ new/include/asm-mips/spinlock.h	2007-03-05 20:35:18.000000000 -0500
@@ -249,7 +249,7 @@
 		"	.set	noreorder	# __raw_read_trylock	\n"
 		"	li	%2, 0					\n"
 		"1:	ll	%1, %3					\n"
-		"	bnez	%1, 2f					\n"
+		"	bltz	%1, 2f					\n"
 		"	 addu	%1, 1					\n"
 		"	sc	%1, %0					\n"
 		"	beqzl	%1, 1b					\n"
@@ -267,7 +267,7 @@
 		"	.set	noreorder	# __raw_read_trylock	\n"
 		"	li	%2, 0					\n"
 		"1:	ll	%1, %3					\n"
-		"	bnez	%1, 2f					\n"
+		"	bltz	%1, 2f					\n"
 		"	 addu	%1, 1					\n"
 		"	sc	%1, %0					\n"
 		"	beqz	%1, 1b					\n"

Re: [PATCH] Fix __raw_read_trylock() to allow multiple readers

[Ralf Baechle]

On Mon, Mar 05, 2007 at 08:50:27PM -0500, Dave Johnson wrote:

> Perhaps someone can confirm this bug by a code inspection, but I
> think the below patch will fix the issue.

You're right, thanks!

Patch applied,

  Ralf