Skip to contentSkip to conversation

*_trylock return on success?

DISCUSSION Roger Larsson 3 participants 8 messages Nov 25, 2000 – Nov 28, 2000

Subsystems (8)
Networking Network drivers VFS IPv4/IPv6 +4 more subsystems
lore
Share & tools
lore

Roger Larsson

author
#0 lore raw

Hi,

Background information:
compiled and tested a test11 with the Montavista preemptive patch. After pressing Magic-SysRq-M all processes that tried to do IO hung in 'D' Last message "Buffer memory ..."
Pressing Magic-SysRq-M again, all hung processes continued...

Checking the patch it looks like this

printk("Buffer memory: %6dkB\n",
atomic_read(&buffermem_pages) << (PAGE_SHIFT-10));

-#ifdef CONFIG_SMP /* trylock does nothing on UP and so we could deadlock */
- if (!spin_trylock(&lru_list_lock))
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
+ if (!mutex_trylock(&lru_list_mtx))
return;
for(nlist = 0; nlist < NR_LIST; nlist++) {

Ok, so I run some more code now than before (UP system with PREEMPT). mutex_trylock is defined as:

+#define mutex_trylock(x) down_trylock(x)

Noticed that if the spin_trylock returns 0 on success, I will get the behavior I see.
Not printing buffer info first time.
Holding the lock - stopping other fs processes.
Failing the mutex_trylock next attempt, interprete as success
- continuing and printing the buffer info.
- finally release the mutex

I removed the not (!) and now it works as expected.

Questions:
What are _trylocks supposed to return?
Does spin_trylock and down_trylock behave differently? Why isn't the expected return value documented?

/RogerL

Signature · 7 lines
--
Home page:
  http://www.norran.net/nra02596/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#1 lore raw
Quoted 2 lines · Roger Larsson

Questions:
What are _trylocks supposed to return?

It depends on the type of _trylock ;(

Quoted 2 lines

Does spin_trylock and down_trylock behave differently? Why isn't the expected return value documented?

The whole trylock stuff is, IMHO, a big mess. When you change from one type of trylock to another, you may be forced to invert the logic of your code since the return code from the different locks is different.

For bitflags, for example, the trylock returns the state the bit had before the lock (ie. 1 if the thing was already locked).

For spinlocks, it'll probably return something else ;/

regards,

Rik
--
Hollywood goes for world dumbination,
Trailer at 11.

http://www.conectiva.com/ http://www.surriel.com/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
In reply to Roger Larsson
Quoted 10 lines · Rik van Riel
Quoted 2 lines · Roger Larsson

Questions:
What are _trylocks supposed to return?

It depends on the type of _trylock ;(

Quoted 2 lines

Does spin_trylock and down_trylock behave differently? Why isn't the expected return value documented?

Quoted 8 lines

The whole trylock stuff is, IMHO, a big mess. When you change from one type of trylock to another, you may be forced to invert the logic of your code since the return code from the different locks is different.

For bitflags, for example, the trylock returns the state the bit had before the lock (ie. 1 if the thing was already locked).

I assume you're talking about test_and_{set,clear}_bit here. Their return value isn't consistent with the other _trylock functions since they're not _trylock functions.

I think the real problem is that people use test_and_set_bit for locks, which is almost never[1] a good idea. The overhead for a semaphore shouldn't be too much in most cases, and that way it is obvious what you want to do - and, hopefully, even more obvious if you end up with a semaphore that can be turned into a spinlock without further changes.

Quoted 1 line

For spinlocks, it'll probably return something else ;/

_trylock functions return 0 for success.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

author
#2 lore raw
In reply to Rik van Riel
Quoted 9 lines · Rik van Riel
Quoted 2 lines · Roger Larsson

Questions:
What are _trylocks supposed to return?

It depends on the type of _trylock ;(

Quoted 2 lines

Does spin_trylock and down_trylock behave differently? Why isn't the expected return value documented?

Quoted 9 lines

The whole trylock stuff is, IMHO, a big mess. When you change from one type of trylock to another, you may be forced to invert the logic of your code since the return code from the different locks is different.

For bitflags, for example, the trylock returns the state the bit had before the lock (ie. 1 if the thing was already locked).

This holds for down_trylocks too.
It looks like it is the spinlocks that are wrong... :-(

As most return values tend to be error returns that also matches other code in functionallity.

Quoted 2 lines

For spinlocks, it'll probably return something else ;/

It does...

I guess fixing this is too much too late?

It looks like ppc mixes the ways... from arch/ppc/lib/locks.c:46

int spin_trylock(spinlock_t *lock)

{
	if (__spin_trylock(&lock->lock))                  /* one on failure */
		return 0;				  /* zero on failure */ 
	lock->owner_cpu = smp_processor_id(); 
	lock->owner_pc = (unsigned long)__builtin_return_address(0);
	return 1;
}

BUT anyway...
The thing I hit is not a bug in the kernel proper - it is in the preemptive
stuff.

/RogerL

Signature · 7 lines
Home page:
  http://www.norran.net/nra02596/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
author
#4 lore raw
In reply to Roger Larsson
Quoted 29 lines · Philipp Rumpf
Quoted 9 lines · Rik van Riel
Quoted 2 lines · Roger Larsson

Questions:
What are _trylocks supposed to return?

It depends on the type of _trylock ;(

Quoted 2 lines

Does spin_trylock and down_trylock behave differently? Why isn't the expected return value documented?

Quoted 8 lines

The whole trylock stuff is, IMHO, a big mess. When you change from one type of trylock to another, you may be forced to invert the logic of your code since the return code from the different locks is different.

For bitflags, for example, the trylock returns the state the bit had before the lock (ie. 1 if the thing was already locked).

I assume you're talking about test_and_{set,clear}_bit here. Their return value isn't consistent with the other _trylock functions since they're not _trylock functions.

I think the real problem is that people use test_and_set_bit for locks, which is almost never[1] a good idea. The overhead for a semaphore shouldn't be too much in most cases, and that way it is obvious what you want to do - and, hopefully, even more obvious if you end up with a semaphore that can be turned into a spinlock without further changes.

Quoted 3 lines
Quoted 1 line

For spinlocks, it'll probably return something else ;/

_trylock functions return 0 for success.

Not spin_trylock

Simple example code from
code from include/asm-mips/spinlock.h:65 #define spin_trylock(lock) (!test_and_set_bit(0,(lock)))

/RogerL

Quoted 4 lines

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Signature · 7 lines
--
Home page:
  http://www.norran.net/nra02596/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
#5 lore raw
In reply to Roger Larsson
Quoted 3 lines · Roger Larsson
Quoted 1 line

_trylock functions return 0 for success.

Not spin_trylock

Argh, I missed the (recent ?) change to make x86 spinlocks use 1 to mean unlocked. You're correct, and obviously this should be fixed. -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

author
#6 lore raw
In reply to Roger Larsson
Quoted 11 lines · Philipp Rumpf
Quoted 3 lines · Roger Larsson
Quoted 1 line

_trylock functions return 0 for success.

Not spin_trylock

Argh, I missed the (recent ?) change to make x86 spinlocks use 1 to mean unlocked. You're correct, and obviously this should be fixed. -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

If this are to change in 2.4 I would suggest
to renaming it to mutex_lock (as in Nigels preemptive kernel patch)

Why?

A) the name spin_lock describes how the function is implemented and not the intended purpose.
B) with a preemptive kernel we will have more than four intended purposes:
1) SMP - spin_lock, prevent two processors to run currently
2) UP - not used, code can only be executed by one thread.
3) PREEMTIVE - lock a region for preemption to avoid concurrent execution.
4) debug - addition of debug checks.

With Nigels patch most are changed, with some additional stuff...

My suggestion, change the name to mutex_lock and negate let mutex_trylock follow the example of other _trylocks (returning 0 for success).

Ok?

If it is ok, I can prepare a patch (earliest monday)

/RogerL

Signature · 7 lines
Home page:
  http://www.norran.net/nra02596/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/
author
#7 lore raw
In reply to Roger Larsson
Quoted 8 lines · Roger Larsson
Quoted 7 lines · Philipp Rumpf
Quoted 3 lines · Roger Larsson
Quoted 1 line

_trylock functions return 0 for success.

Not spin_trylock

Argh, I missed the (recent ?) change to make x86 spinlocks use 1 to mean unlocked. You're correct, and obviously this should be fixed.

Have looked more into this now...
tasklet_trylock is also wrong (but there are only four of them) Is this 2.4 only, or where there spin locks earlier too?

My suggestion now is a few steps:
1) to release a kernel version that has corrected _trylocks; spin2_trylock and tasklet2_trylock.
[with corresponding updates in as many places as possible:
s/!spin_trylock/spin2_trylock/g s/spin_trylock/!spin2_trylock/g
. . .]
(ready for spin trylock, not done for tasklet yet..., attached,
hope it got included OK - not fully used to kmail)

2) This will in house only drives or compilations that in some strange way uses this calls...

3a) (DANGEROUS) global rename spin2_trylock to spin_trylock
[no logic change this time - only name]
3b) (dangerous) add compatibility interface
#define spin_trylock(L) (!spin2_trylock(L)) Probably not necessary since it can not be linked against. Binary modules will contain their own compatibility code :-) Probably preferred by those who maintain drivers for several releases; 2.2, 2.4, ...
3c) do not do anything more...

Alternative:
1b) do nothing at all - suffer later

/RogerL

Signature · 2 lines
Home page:
  http://www.norran.net/nra02596/
arch/alpha/kernel/alpha_ksyms.c +1 -1
diff -Naur v2.4.0-test11-linux/arch/alpha/kernel/alpha_ksyms.c linux/arch/alpha/kernel/alpha_ksyms.c
@@ -198,7 +198,7 @@
 #if DEBUG_SPINLOCK
 EXPORT_SYMBOL(spin_unlock);
 EXPORT_SYMBOL(debug_spin_lock);
-EXPORT_SYMBOL(debug_spin_trylock);
+EXPORT_SYMBOL(debug_spin2_trylock);
 #endif
 #if DEBUG_RWLOCK
 EXPORT_SYMBOL(write_lock);
arch/alpha/kernel/irq_smp.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/alpha/kernel/irq_smp.c linux/arch/alpha/kernel/irq_smp.c
@@ -96,7 +96,7 @@
 			if (!local_bh_count(cpu)
 			    && spin_is_locked(&global_bh_lock))
 				continue;
-			if (spin_trylock(&global_irq_lock))
+			if (!spin2_trylock(&global_irq_lock))
 				break;
 		}
 	}

@@ -105,7 +105,7 @@
 static inline void
 get_irqlock(int cpu, void* where)
 {
-	if (!spin_trylock(&global_irq_lock)) {
+	if (spin2_trylock(&global_irq_lock)) {
 		/* Do we already hold the lock?  */
 		if (cpu == global_irq_holder)
 			return;
arch/alpha/kernel/smp.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/alpha/kernel/smp.c linux/arch/alpha/kernel/smp.c
@@ -1078,10 +1078,10 @@
 }
 
 int
-debug_spin_trylock(spinlock_t * lock, const char *base_file, int line_no)
+debug_spin2_trylock(spinlock_t * lock, const char *base_file, int line_no)
 {
 	int ret;
-	if ((ret = !test_and_set_bit(0, lock))) {
+	if ((ret = test_and_set_bit(0, lock))) {
 		lock->on_cpu = smp_processor_id();
 		lock->previous = __builtin_return_address(0);
 		lock->task = current;
arch/mips64/sgi-ip27/ip27-irq.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/mips64/sgi-ip27/ip27-irq.c linux/arch/mips64/sgi-ip27/ip27-irq.c
@@ -480,7 +480,7 @@
 				continue;
 			if (!local_bh_count(cpu) && spin_is_locked(&global_bh_lock))
 				continue;
-			if (spin_trylock(&global_irq_lock))
+			if (!spin2_trylock(&global_irq_lock))
 				break;
 		}
 	}

@@ -497,7 +497,7 @@
 
 static inline void get_irqlock(int cpu)
 {
-	if (!spin_trylock(&global_irq_lock)) {
+	if (spin2_trylock(&global_irq_lock)) {
 		/* do we already hold the lock? */
 		if ((unsigned char) cpu == global_irq_holder)
 			return;
arch/ppc/kernel/misc.S +1 -0
diff -Naur v2.4.0-test11-linux/arch/ppc/kernel/misc.S linux/arch/ppc/kernel/misc.S
@@ -434,6 +434,7 @@
  * Environments Manual suggests not doing unnecessary stcwx.'s
  * since they may inhibit forward progress by other CPUs in getting
  * a lock.
+ * Returns:	0 on success ???
  */
 _GLOBAL(__spin_trylock)
 	mr	r4,r3
arch/ppc/kernel/ppc_ksyms.c +1 -1
diff -Naur v2.4.0-test11-linux/arch/ppc/kernel/ppc_ksyms.c linux/arch/ppc/kernel/ppc_ksyms.c
@@ -197,7 +197,7 @@
 EXPORT_SYMBOL(__global_restore_flags);
 EXPORT_SYMBOL(_spin_lock);
 EXPORT_SYMBOL(_spin_unlock);
-EXPORT_SYMBOL(spin_trylock);
+EXPORT_SYMBOL(spin2_trylock);
 EXPORT_SYMBOL(_read_lock);
 EXPORT_SYMBOL(_read_unlock);
 EXPORT_SYMBOL(_write_lock);
arch/ppc/lib/locks.c +3 -3
diff -Naur v2.4.0-test11-linux/arch/ppc/lib/locks.c linux/arch/ppc/lib/locks.c
@@ -43,13 +43,13 @@
 	lock->owner_cpu = cpu;
 }
 
-int spin_trylock(spinlock_t *lock)
+int spin2_trylock(spinlock_t *lock)
 {
 	if (__spin_trylock(&lock->lock))
-		return 0;
+		return 1;
 	lock->owner_cpu = smp_processor_id(); 
 	lock->owner_pc = (unsigned long)__builtin_return_address(0);
-	return 1;
+	return 0;
 }
arch/sparc/kernel/sparc_ksyms.c +1 -1
diff -Naur v2.4.0-test11-linux/arch/sparc/kernel/sparc_ksyms.c linux/arch/sparc/kernel/sparc_ksyms.c
@@ -101,7 +101,7 @@
 #ifdef SPIN_LOCK_DEBUG
 EXPORT_SYMBOL(_do_spin_lock);
 EXPORT_SYMBOL(_do_spin_unlock);
-EXPORT_SYMBOL(_spin_trylock);
+EXPORT_SYMBOL(_spin2_trylock);
 EXPORT_SYMBOL(_do_read_lock);
 EXPORT_SYMBOL(_do_read_unlock);
 EXPORT_SYMBOL(_do_write_lock);
arch/sparc/lib/debuglocks.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/sparc/lib/debuglocks.c linux/arch/sparc/lib/debuglocks.c
@@ -85,7 +85,7 @@
 	lock->owner_pc = (cpu & 3) | (caller & ~3);
 }
 
-int _spin_trylock(spinlock_t *lock)
+int _spin2_trylock(spinlock_t *lock)
 {
 	unsigned long val;
 	unsigned long caller;

@@ -98,7 +98,7 @@
 		/* We got it, record our identity for debugging. */
 		lock->owner_pc = (cpu & 3) | (caller & ~3);
 	}
-	return val == 0;
+	return val != 0;
 }
 
 void _do_spin_unlock(spinlock_t *lock)
arch/sparc64/kernel/sparc64_ksyms.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/sparc64/kernel/sparc64_ksyms.c linux/arch/sparc64/kernel/sparc64_ksyms.c
@@ -99,7 +99,7 @@
 #ifdef SPIN_LOCK_DEBUG
 extern void _do_spin_lock (spinlock_t *lock, char *str);
 extern void _do_spin_unlock (spinlock_t *lock);
-extern int _spin_trylock (spinlock_t *lock);
+extern int _spin2_trylock (spinlock_t *lock);
 extern void _do_read_lock(rwlock_t *rw, char *str);
 extern void _do_read_unlock(rwlock_t *rw, char *str);
 extern void _do_write_lock(rwlock_t *rw, char *str);

@@ -142,7 +142,7 @@
 #ifdef SPIN_LOCK_DEBUG
 EXPORT_SYMBOL(_do_spin_lock);
 EXPORT_SYMBOL(_do_spin_unlock);
-EXPORT_SYMBOL(_spin_trylock);
+EXPORT_SYMBOL(_spin2_trylock);
 EXPORT_SYMBOL(_do_read_lock);
 EXPORT_SYMBOL(_do_read_unlock);
 EXPORT_SYMBOL(_do_write_lock);
arch/sparc64/lib/debuglocks.c +2 -2
diff -Naur v2.4.0-test11-linux/arch/sparc64/lib/debuglocks.c linux/arch/sparc64/lib/debuglocks.c
@@ -78,7 +78,7 @@
 	lock->owner_cpu = cpu;
 }
 
-int _spin_trylock(spinlock_t *lock)
+int _spin2_trylock(spinlock_t *lock)
 {
 	unsigned long val, caller;
 	int cpu = smp_processor_id();

@@ -93,7 +93,7 @@
 		lock->owner_pc = ((unsigned int)caller);
 		lock->owner_cpu = cpu;
 	}
-	return val == 0;
+	return val != 0;
 }
 
 void _do_spin_unlock(spinlock_t *lock)
drivers/net/ppp_async.c +5 -5
diff -Naur v2.4.0-test11-linux/drivers/net/ppp_async.c linux/drivers/net/ppp_async.c
@@ -33,9 +33,9 @@
 #include <linux/init.h>
 #include <asm/uaccess.h>
 
-#ifndef spin_trylock_bh
-#define spin_trylock_bh(lock)	({ int __r; local_bh_disable();	\
-				   __r = spin_trylock(lock);	\
+#ifndef spin2_trylock_bh
+#define spin2_trylock_bh(lock)	({ int __r; local_bh_disable();	\
+				   __r = spin2_trylock(lock);	\
 				   if (!__r) local_bh_enable();	\
 				   __r; })
 #endif

@@ -637,7 +637,7 @@
 	int tty_stuffed = 0;
 
 	set_bit(XMIT_WAKEUP, &ap->xmit_flags);
-	if (!spin_trylock_bh(&ap->xmit_lock))
+	if (spin2_trylock_bh(&ap->xmit_lock))
 		return 0;
 	for (;;) {
 		if (test_and_clear_bit(XMIT_WAKEUP, &ap->xmit_flags))

@@ -666,7 +666,7 @@
 		if (!(test_bit(XMIT_WAKEUP, &ap->xmit_flags)
 		      || (!tty_stuffed && ap->tpkt != 0)))
 			break;
-		if (!spin_trylock_bh(&ap->xmit_lock))
+		if (spin2_trylock_bh(&ap->xmit_lock))
 			break;
 	}
 	return done;
drivers/net/ppp_synctty.c +5 -5
diff -Naur v2.4.0-test11-linux/drivers/net/ppp_synctty.c linux/drivers/net/ppp_synctty.c
@@ -44,9 +44,9 @@
 #include <linux/init.h>
 #include <asm/uaccess.h>
 
-#ifndef spin_trylock_bh
-#define spin_trylock_bh(lock)	({ int __r; local_bh_disable();	\
-				   __r = spin_trylock(lock);	\
+#ifndef spin2_trylock_bh
+#define spin2_trylock_bh(lock)	({ int __r; local_bh_disable();	\
+				   __r = spin2_trylock(lock);	\
 				   if (!__r) local_bh_enable();	\
 				   __r; })
 #endif

@@ -590,7 +590,7 @@
 	int tty_stuffed = 0;
 
 	set_bit(XMIT_WAKEUP, &ap->xmit_flags);
-	if (!spin_trylock_bh(&ap->xmit_lock))
+	if (spin2_trylock_bh(&ap->xmit_lock))
 		return 0;
 	for (;;) {
 		if (test_and_clear_bit(XMIT_WAKEUP, &ap->xmit_flags))

@@ -615,7 +615,7 @@
 		if (!(test_bit(XMIT_WAKEUP, &ap->xmit_flags)
 		      || (!tty_stuffed && ap->tpkt != 0)))
 			break;
-		if (!spin_trylock_bh(&ap->xmit_lock))
+		if (spin2_trylock_bh(&ap->xmit_lock))
 			break;
 	}
 	return done;
fs/buffer.c +1 -1
diff -Naur v2.4.0-test11-linux/fs/buffer.c linux/fs/buffer.c
@@ -2364,7 +2364,7 @@
 			atomic_read(&buffermem_pages) << (PAGE_SHIFT-10));
 
 #ifdef CONFIG_SMP /* trylock does nothing on UP and so we could deadlock */
-	if (!spin_trylock(&lru_list_lock))
+	if (spin2_trylock(&lru_list_lock))
 		return;
 	for(nlist = 0; nlist < NR_LIST; nlist++) {
 		found = locked = dirty = used = lastused = protected = 0;
include/asm-alpha/spinlock.h +3 -3
diff -Naur v2.4.0-test11-linux/include/asm-alpha/spinlock.h linux/include/asm-alpha/spinlock.h
@@ -41,10 +41,10 @@
 #if DEBUG_SPINLOCK
 extern void spin_unlock(spinlock_t * lock);
 extern void debug_spin_lock(spinlock_t * lock, const char *, int);
-extern int debug_spin_trylock(spinlock_t * lock, const char *, int);
+extern int debug_spin2_trylock(spinlock_t * lock, const char *, int);
 
 #define spin_lock(LOCK) debug_spin_lock(LOCK, __BASE_FILE__, __LINE__)
-#define spin_trylock(LOCK) debug_spin_trylock(LOCK, __BASE_FILE__, __LINE__)
+#define spin2_trylock(LOCK) debug_spin2_trylock(LOCK, __BASE_FILE__, __LINE__)
 
 #define spin_lock_own(LOCK, LOCATION)					\
 do {									\

@@ -84,7 +84,7 @@
 	: "m"(lock->lock) : "memory");
 }
 
-#define spin_trylock(lock) (!test_and_set_bit(0,(lock)))
+#define spin2_trylock(lock) (test_and_set_bit(0,(lock)))
 #define spin_lock_own(LOCK, LOCATION)	((void)0)
 #endif /* DEBUG_SPINLOCK */
include/asm-i386/spinlock.h +2 -2
diff -Naur v2.4.0-test11-linux/include/asm-i386/spinlock.h linux/include/asm-i386/spinlock.h
@@ -65,14 +65,14 @@
 #define spin_unlock_string \
 	"movb $1,%0"
 
-static inline int spin_trylock(spinlock_t *lock)
+static inline int spin2_trylock(spinlock_t *lock)
 {
 	char oldval;
 	__asm__ __volatile__(
 		"xchgb %b0,%1"
 		:"=q" (oldval), "=m" (lock->lock)
 		:"0" (0) : "memory");
-	return oldval > 0;
+	return oldval == 0;
 }
 
 static inline void spin_lock(spinlock_t *lock)
include/asm-ia64/spinlock.h +3 -3
diff -Naur v2.4.0-test11-linux/include/asm-ia64/spinlock.h linux/include/asm-ia64/spinlock.h
@@ -48,7 +48,7 @@
 		: "ar.ccv", "ar.pfs", "b7", "p15", "r28", "r29", "r30", "memory");	\
 }
 
-#define spin_trylock(x)								\
+#define spin2_trylock(x)								\
 ({										\
 	register char *addr __asm__ ("r31") = (char *) &(x)->lock;		\
 	register long result;							\

@@ -59,7 +59,7 @@
 		";;\n"								\
 		IA64_SEMFIX"cmpxchg1.acq %0=[%1],r30,ar.ccv\n"			\
 		: "=r"(result) : "r"(addr) : "ar.ccv", "r30", "memory");	\
-	(result == 0);								\
+	(result != 0);								\
 })
 
 #define spin_is_locked(x)	((x)->lock != 0)

@@ -98,7 +98,7 @@
 
 #define spin_is_locked(x)	((x)->lock != 0)
 #define spin_unlock(x)		do {((spinlock_t *) x)->lock = 0; barrier(); } while (0)
-#define spin_trylock(x)		(cmpxchg_acq(&(x)->lock, 0, 1) == 0)
+#define spin2_trylock(x)	(cmpxchg_acq(&(x)->lock, 0, 1) != 0)
 #define spin_unlock_wait(x)	do { barrier(); } while ((x)->lock)
 
 #endif /* !NEW_LOCK */
include/asm-mips/spinlock.h +1 -1
diff -Naur v2.4.0-test11-linux/include/asm-mips/spinlock.h linux/include/asm-mips/spinlock.h
@@ -62,7 +62,7 @@
 	: "memory");
 }
 
-#define spin_trylock(lock) (!test_and_set_bit(0,(lock)))
+#define spin2_trylock(lock) (test_and_set_bit(0,(lock)))
 
 /*
  * Read-write spinlocks, allowing multiple readers but only one writer.
include/asm-mips64/spinlock.h +3 -3
diff -Naur v2.4.0-test11-linux/include/asm-mips64/spinlock.h linux/include/asm-mips64/spinlock.h
@@ -65,12 +65,12 @@
 	: "memory");
 }
 
-static inline unsigned int spin_trylock(spinlock_t *lock)
+static inline unsigned int spin2_trylock(spinlock_t *lock)
 {
 	unsigned int temp, res;
 
 	__asm__ __volatile__(
-	".set\tnoreorder\t\t\t# spin_trylock\n\t"
+	".set\tnoreorder\t\t\t# spin2_trylock\n\t"
 	"1:\tll\t%0, %1\n\t"
 	"or\t%2, %0, %3\n\t"
 	"sc\t%2, %1\n\t"

@@ -81,7 +81,7 @@
 	:"r" (1), "m" (*lock)
 	: "memory");
 
-	return res == 0;
+	return res != 0;
 }
 
 /*
include/asm-ppc/spinlock.h +2 -2
diff -Naur v2.4.0-test11-linux/include/asm-ppc/spinlock.h linux/include/asm-ppc/spinlock.h
@@ -19,12 +19,12 @@
 
 extern void _spin_lock(spinlock_t *lock);
 extern void _spin_unlock(spinlock_t *lock);
-extern int spin_trylock(spinlock_t *lock);
+extern int spin2_trylock(spinlock_t *lock);
 
 #define spin_lock(lp)			_spin_lock(lp)
 #define spin_unlock(lp)			_spin_unlock(lp)
 
-extern unsigned long __spin_trylock(volatile unsigned long *lock);
+extern unsigned long __spin2_trylock(volatile unsigned long *lock);
 
 /*
  * Read-write spinlocks, allowing multiple readers
include/asm-s390/spinlock.h +2 -2
diff -Naur v2.4.0-test11-linux/include/asm-s390/spinlock.h linux/include/asm-s390/spinlock.h
@@ -37,7 +37,7 @@
 			   : "0" (lp->lock) : "0", "1");
 }
 
-extern inline int spin_trylock(spinlock_t *lp)
+extern inline int spin2_trylock(spinlock_t *lp)
 {
 	unsigned long result;
 	__asm__ __volatile("    slr   %1,%1\n"

@@ -45,7 +45,7 @@
 			   "0:  cs    %1,0,%0"
 			   : "=m" (lp->lock), "=&d" (result)
 			   : "0" (lp->lock) : "0");
-	return !result;
+	return result != 0;
 }
include/asm-sparc/spinlock.h +4 -4
diff -Naur v2.4.0-test11-linux/include/asm-sparc/spinlock.h linux/include/asm-sparc/spinlock.h
@@ -33,10 +33,10 @@
 #define spin_unlock_wait(lp)	do { barrier(); } while(*(volatile unsigned char *)(&(lp)->lock))
 
 extern void _do_spin_lock(spinlock_t *lock, char *str);
-extern int _spin_trylock(spinlock_t *lock);
+extern int _spin2_trylock(spinlock_t *lock);
 extern void _do_spin_unlock(spinlock_t *lock);
 
-#define spin_trylock(lp)	_spin_trylock(lp)
+#define spin2_trylock(lp)	_spin2_trylock(lp)
 #define spin_lock(lock)		_do_spin_lock(lock, "spin_lock")
 #define spin_unlock(lock)	_do_spin_unlock(lock)
 

@@ -115,14 +115,14 @@
 	: "g2", "memory", "cc");
 }
 
-extern __inline__ int spin_trylock(spinlock_t *lock)
+extern __inline__ int spin2_trylock(spinlock_t *lock)
 {
 	unsigned int result;
 	__asm__ __volatile__("ldstub [%1], %0"
 			     : "=r" (result)
 			     : "r" (lock)
 			     : "memory");
-	return (result == 0);
+	return (result != 0);
 }
 
 extern __inline__ void spin_unlock(spinlock_t *lock)
include/asm-sparc64/spinlock.h +4 -4
diff -Naur v2.4.0-test11-linux/include/asm-sparc64/spinlock.h linux/include/asm-sparc64/spinlock.h
@@ -55,7 +55,7 @@
 	: "g7", "memory");
 }
 
-extern __inline__ int spin_trylock(spinlock_t *lock)
+extern __inline__ int spin2_trylock(spinlock_t *lock)
 {
 	unsigned int result;
 	__asm__ __volatile__("ldstub [%1], %0\n\t"

@@ -63,7 +63,7 @@
 			     : "=r" (result)
 			     : "r" (lock)
 			     : "memory");
-	return (result == 0);
+	return (result != 0);
 }
 
 extern __inline__ void spin_unlock(spinlock_t *lock)

@@ -95,9 +95,9 @@
 
 extern void _do_spin_lock (spinlock_t *lock, char *str);
 extern void _do_spin_unlock (spinlock_t *lock);
-extern int _spin_trylock (spinlock_t *lock);
+extern int _spin2_trylock (spinlock_t *lock);
 
-#define spin_trylock(lp)	_spin_trylock(lp)
+#define spin2_trylock(lp)	_spin2_trylock(lp)
 #define spin_lock(lock)		_do_spin_lock(lock, "spin_lock")
 #define spin_unlock(lock)	_do_spin_unlock(lock)
include/linux/spinlock.h +6 -3
diff -Naur v2.4.0-test11-linux/include/linux/spinlock.h linux/include/linux/spinlock.h
@@ -58,7 +58,7 @@
 #define spin_lock_init(lock)	do { } while(0)
 #define spin_lock(lock)		(void)(lock) /* Not "unused variable". */
 #define spin_is_locked(lock)	(0)
-#define spin_trylock(lock)	({1; })
+#define spin2_trylock(lock)	({0; }) /* Note: returns 0 on success */
 #define spin_unlock_wait(lock)	do { } while(0)
 #define spin_unlock(lock)	do { } while(0)
 

@@ -71,7 +71,9 @@
 
 #define spin_lock_init(x)	do { (x)->lock = 0; } while (0)
 #define spin_is_locked(lock)	(test_bit(0,(lock)))
-#define spin_trylock(lock)	(!test_and_set_bit(0,(lock)))
+#define spin2_trylock(lock)	(test_and_set_bit(0,(lock))) /* Note:
+								returns 0 on
+								success */
 
 #define spin_lock(x)		do { (x)->lock = 1; } while (0)
 #define spin_unlock_wait(x)	do { } while (0)

@@ -90,7 +92,8 @@
 
 #define spin_lock_init(x)	do { (x)->lock = 0; } while (0)
 #define spin_is_locked(lock)	(test_bit(0,(lock)))
-#define spin_trylock(lock)	(!test_and_set_bit(0,(lock)))
+#define spin2_trylock(lock)	(test_and_set_bit(0,(lock))) /* Note: returns
+								0 on success */
 
 #define spin_lock(x)		do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s:%d: spin_lock(%s:%p) already locked\n", __BASE_FILE__,__LINE__, (x)->module, (x));(x)->babble--;} (x)->lock = 1; restore_flags(__spinflags);} while (0)
 #define spin_unlock_wait(x)	do {unsigned long __spinflags; save_flags(__spinflags); cli(); if ((x)->lock&&(x)->babble) {printk("%s:%d: spin_unlock_wait(%s:%p) deadlock\n", __BASE_FILE__,__LINE__, (x)->module, (x));(x)->babble--;} restore_flags(__spinflags);} while (0)
kernel/softirq.c +1 -1
diff -Naur v2.4.0-test11-linux/kernel/softirq.c linux/kernel/softirq.c
@@ -246,7 +246,7 @@
 {
 	int cpu = smp_processor_id();
 
-	if (!spin_trylock(&global_bh_lock))
+	if (spin2_trylock(&global_bh_lock))
 		goto resched;
 
 	if (!hardirq_trylock(cpu))
net/core/dev.c +1 -1
diff -Naur v2.4.0-test11-linux/net/core/dev.c linux/net/core/dev.c
@@ -1251,7 +1251,7 @@
 			smp_mb__before_clear_bit();
 			clear_bit(__LINK_STATE_SCHED, &dev->state);
 
-			if (spin_trylock(&dev->queue_lock)) {
+			if (!spin2_trylock(&dev->queue_lock)) {
 				qdisc_run(dev);
 				spin_unlock(&dev->queue_lock);
 			} else {
net/core/dst.c +1 -1
diff -Naur v2.4.0-test11-linux/net/core/dst.c linux/net/core/dst.c
@@ -45,7 +45,7 @@
 	int    delayed = 0;
 	struct dst_entry * dst, **dstp;
 
-	if (!spin_trylock(&dst_lock)) {
+	if (spin2_trylock(&dst_lock)) {
 		mod_timer(&dst_gc_timer, jiffies + HZ/10);
 		return;
 	}
net/ipv4/icmp.c +1 -1
diff -Naur v2.4.0-test11-linux/net/ipv4/icmp.c linux/net/ipv4/icmp.c
@@ -349,7 +349,7 @@
 
 static int icmp_xmit_lock_bh(void)
 {
-	if (!spin_trylock(&icmp_socket->sk->lock.slock)) {
+	if (spin2_trylock(&icmp_socket->sk->lock.slock)) {
 		if (icmp_xmit_holder == smp_processor_id())
 			return -EAGAIN;
 		spin_lock(&icmp_socket->sk->lock.slock);
net/ipv4/ipmr.c +1 -1
diff -Naur v2.4.0-test11-linux/net/ipv4/ipmr.c linux/net/ipv4/ipmr.c
@@ -319,7 +319,7 @@
 	unsigned long expires;
 	struct mfc_cache *c, **cp;
 
-	if (!spin_trylock(&mfc_unres_lock)) {
+	if (spin2_trylock(&mfc_unres_lock)) {
 		mod_timer(&ipmr_expire_timer, jiffies+HZ/10);
 		return;
 	}
net/ipv6/icmp.c +1 -1
diff -Naur v2.4.0-test11-linux/net/ipv6/icmp.c linux/net/ipv6/icmp.c
@@ -92,7 +92,7 @@
 
 static int icmpv6_xmit_lock_bh(void)
 {
-	if (!spin_trylock(&icmpv6_socket->sk->lock.slock)) {
+	if (spin2_trylock(&icmpv6_socket->sk->lock.slock)) {
 		if (icmpv6_xmit_holder == smp_processor_id())
 			return -EAGAIN;
 		spin_lock(&icmpv6_socket->sk->lock.slock);
net/ipv6/ip6_fib.c +1 -1
diff -Naur v2.4.0-test11-linux/net/ipv6/ip6_fib.c linux/net/ipv6/ip6_fib.c
@@ -1174,7 +1174,7 @@
 		gc_args.timeout = (int)dummy;
 	} else {
 		local_bh_disable();
-		if (!spin_trylock(&fib6_gc_lock)) {
+		if (spin2_trylock(&fib6_gc_lock)) {
 			mod_timer(&ip6_fib_timer, jiffies + HZ);
 			local_bh_enable();
 			return;
net/sched/sch_generic.c +1 -1
diff -Naur v2.4.0-test11-linux/net/sched/sch_generic.c linux/net/sched/sch_generic.c
@@ -81,7 +81,7 @@
 
 	/* Dequeue packet */
 	if ((skb = q->dequeue(q)) != NULL) {
-		if (spin_trylock(&dev->xmit_lock)) {
+		if (!spin2_trylock(&dev->xmit_lock)) {
 			/* Remember that the driver is grabbed by us. */
 			dev->xmit_lock_owner = smp_processor_id();
net/sched/sch_teql.c +1 -1
diff -Naur v2.4.0-test11-linux/net/sched/sch_teql.c linux/net/sched/sch_teql.c
@@ -302,7 +302,7 @@
 
 		switch (teql_resolve(skb, skb_res, slave)) {
 		case 0:
-			if (spin_trylock(&slave->xmit_lock)) {
+			if (!spin2_trylock(&slave->xmit_lock)) {
 				slave->xmit_lock_owner = smp_processor_id();
 				if (!netif_queue_stopped(slave) &&
 				    slave->hard_start_xmit(skb, slave) == 0) {