Re: [PATCH RFC] 'spinlock/rwlock fixes' V3 [1/1]

[Ingo Molnar]

* Linus Torvalds <torvalds@xxxxxxxx> wrote:

> > +static inline int read_can_lock(rwlock_t *rw)
> > +{
> > +	return rw->lock > 0;
> > +}
> 
> No, it does need the cast to signed, otherwise it will return true for
> the case where somebody holds the write-lock _and_ there's a pending
> reader waiting too (the write-lock will make it zero, the pending
> reader will wrap around and make it negative, but since "lock" is
> "unsigned", it will look like a large value to "read_can_lock".

indeed. Another solution would be to make the type signed - patch below. 
(like ppc64 does)

> I also think I'd prefer to do the things as macros, and do the
> type-safety by just renaming the "lock" field like Chris did. [...]

(i sent the original lock/slock renaming patch yesterday, and i think
Chris simply reworked&resent that one.)

	Ingo

--- linux/include/asm-i386/spinlock.h.orig
+++ linux/include/asm-i386/spinlock.h
@@ -179,7 +179,7 @@ static inline void _raw_spin_lock_flags 
  * read-locks.
  */
 typedef struct {
-	volatile unsigned int lock;
+	volatile signed int lock;
 #ifdef CONFIG_DEBUG_SPINLOCK
 	unsigned magic;
 #endif
-
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/