public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 1/3] spinlock fix #1
@ 2005-01-20 11:43 Ingo Molnar
  2005-01-20 11:59 ` [patch 2/3] spinlock fix #2: generalize [spin|rw]lock yielding Ingo Molnar
  0 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2005-01-20 11:43 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


i've split up spinlocking-fixes.patch into 3 parts and reworked them. 
This is the first one, against BK-curr:

it fixes the BUILD_LOCK_OPS() bug by introducing the following 3 new
locking primitives:

  spin_trylock_test(lock)
  read_trylock_test(lock)
  write_trylock_test(lock)

this is what is needed by BUILD_LOCK_OPS(): a nonintrusive test to check
whether the real (intrusive) trylock op would succeed or not. Semantics
and naming is completely symmetric to the trylock counterpart. No
changes to exit.c.

build/boot-tested on x86. Architectures that want to support PREEMPT
need to add these definitions.

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

--- linux/kernel/spinlock.c.orig
+++ linux/kernel/spinlock.c
@@ -173,8 +173,8 @@ EXPORT_SYMBOL(_write_lock);
  * (We do this in a function because inlining it would be excessive.)
  */
 
-#define BUILD_LOCK_OPS(op, locktype, is_locked_fn)			\
-void __lockfunc _##op##_lock(locktype *lock)				\
+#define BUILD_LOCK_OPS(op, locktype)					\
+void __lockfunc _##op##_lock(locktype##_t *lock)			\
 {									\
 	preempt_disable();						\
 	for (;;) {							\
@@ -183,7 +183,7 @@ void __lockfunc _##op##_lock(locktype *l
 		preempt_enable();					\
 		if (!(lock)->break_lock)				\
 			(lock)->break_lock = 1;				\
-		while (is_locked_fn(lock) && (lock)->break_lock)	\
+		while (!op##_trylock_test(lock) && (lock)->break_lock)	\
 			cpu_relax();					\
 		preempt_disable();					\
 	}								\
@@ -191,7 +191,7 @@ void __lockfunc _##op##_lock(locktype *l
 									\
 EXPORT_SYMBOL(_##op##_lock);						\
 									\
-unsigned long __lockfunc _##op##_lock_irqsave(locktype *lock)		\
+unsigned long __lockfunc _##op##_lock_irqsave(locktype##_t *lock)	\
 {									\
 	unsigned long flags;						\
 									\
@@ -205,7 +205,7 @@ unsigned long __lockfunc _##op##_lock_ir
 		preempt_enable();					\
 		if (!(lock)->break_lock)				\
 			(lock)->break_lock = 1;				\
-		while (is_locked_fn(lock) && (lock)->break_lock)	\
+		while (!op##_trylock_test(lock) && (lock)->break_lock)	\
 			cpu_relax();					\
 		preempt_disable();					\
 	}								\
@@ -214,14 +214,14 @@ unsigned long __lockfunc _##op##_lock_ir
 									\
 EXPORT_SYMBOL(_##op##_lock_irqsave);					\
 									\
-void __lockfunc _##op##_lock_irq(locktype *lock)			\
+void __lockfunc _##op##_lock_irq(locktype##_t *lock)			\
 {									\
 	_##op##_lock_irqsave(lock);					\
 }									\
 									\
 EXPORT_SYMBOL(_##op##_lock_irq);					\
 									\
-void __lockfunc _##op##_lock_bh(locktype *lock)				\
+void __lockfunc _##op##_lock_bh(locktype##_t *lock)			\
 {									\
 	unsigned long flags;						\
 									\
@@ -246,9 +246,9 @@ EXPORT_SYMBOL(_##op##_lock_bh)
  *         _[spin|read|write]_lock_irqsave()
  *         _[spin|read|write]_lock_bh()
  */
-BUILD_LOCK_OPS(spin, spinlock_t, spin_is_locked);
-BUILD_LOCK_OPS(read, rwlock_t, rwlock_is_locked);
-BUILD_LOCK_OPS(write, rwlock_t, spin_is_locked);
+BUILD_LOCK_OPS(spin, spinlock);
+BUILD_LOCK_OPS(read, rwlock);
+BUILD_LOCK_OPS(write, rwlock);
 
 #endif /* CONFIG_PREEMPT */
 
--- linux/include/linux/spinlock.h.orig
+++ linux/include/linux/spinlock.h
@@ -584,4 +584,10 @@ static inline int bit_spin_is_locked(int
 #define DEFINE_SPINLOCK(x) spinlock_t x = SPIN_LOCK_UNLOCKED
 #define DEFINE_RWLOCK(x) rwlock_t x = RW_LOCK_UNLOCKED
 
+/**
+ * spin_trylock_test - would spin_trylock() succeed?
+ * @lock: the spinlock in question.
+ */
+#define spin_trylock_test(lock)		(!spin_is_locked(lock))
+
 #endif /* __LINUX_SPINLOCK_H */
--- linux/include/asm-i386/spinlock.h.orig
+++ linux/include/asm-i386/spinlock.h
@@ -188,6 +188,18 @@ typedef struct {
 
 #define rwlock_is_locked(x) ((x)->lock != RW_LOCK_BIAS)
 
+/**
+ * read_trylock_test - would read_trylock() succeed?
+ * @lock: the rwlock in question.
+ */
+#define read_trylock_test(x) (atomic_read((atomic_t *)&(x)->lock) > 0)
+
+/**
+ * write_trylock_test - would write_trylock() succeed?
+ * @lock: the rwlock in question.
+ */
+#define write_trylock_test(x) ((x)->lock == RW_LOCK_BIAS)
+
 /*
  * On x86, we implement read-write locks as a 32-bit counter
  * with the high bit (sign) being the "contended" bit.

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: Horrible regression with -CURRENT from "Don't busy-lock-loop in preemptable spinlocks" patch
@ 2005-01-19 21:43 Paul Mackerras
  2005-01-20  2:34 ` [PATCH RFC] 'spinlock/rwlock fixes' V3 [1/1] Chris Wedgwood
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Mackerras @ 2005-01-19 21:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Peter Chubb, Tony Luck, Darren Williams, Andrew Morton,
	Chris Wedgwood, torvalds, benh, linux-kernel, Ia64 Linux,
	Christoph Hellwig

Ingo Molnar writes:

> * Peter Chubb <peterc@gelato.unsw.edu.au> wrote:
> 
> > >> Here's a patch that adds the missing read_is_locked() and
> > >> write_is_locked() macros for IA64.  When combined with Ingo's
> > >> patch, I can boot an SMP kernel with CONFIG_PREEMPT on.
> > >> 
> > >> However, I feel these macros are misnamed: read_is_locked() returns
> > >> true if the lock is held for writing; write_is_locked() returns
> > >> true if the lock is held for reading or writing.
> > 
> > Ingo> well, 'read_is_locked()' means: "will a read_lock() succeed"
> > 
> > Fail, surely?
> 
> yeah ... and with that i proved beyond doubt that the naming is indeed
> unintuitive :-)

Yes.  Intuitively read_is_locked() is true when someone has done a
read_lock and write_is_locked() is true when someone has done a write
lock.

I suggest read_poll(), write_poll(), spin_poll(), which are like
{read,write,spin}_trylock but don't do the atomic op to get the lock,
that is, they don't change the lock value but return true if the
trylock would succeed, assuming no other cpu takes the lock in the
meantime.

Regards,
Paul.

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2005-01-20 22:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-01-20 11:43 [patch 1/3] spinlock fix #1 Ingo Molnar
2005-01-20 11:59 ` [patch 2/3] spinlock fix #2: generalize [spin|rw]lock yielding Ingo Molnar
2005-01-20 12:09   ` [patch 3/3] spinlock fix #3: type-checking spinlock primitives, x86 Ingo Molnar
2005-01-20 12:18     ` [patch] stricter type-checking rwlock " Ingo Molnar
2005-01-20 12:22       ` [patch] minor spinlock cleanups Ingo Molnar
2005-01-20 22:51     ` [patch 3/3] spinlock fix #3: type-checking spinlock primitives, x86 J.A. Magallon
  -- strict thread matches above, loose matches on Subject: below --
2005-01-19 21:43 Horrible regression with -CURRENT from "Don't busy-lock-loop in preemptable spinlocks" patch Paul Mackerras
2005-01-20  2:34 ` [PATCH RFC] 'spinlock/rwlock fixes' V3 [1/1] Chris Wedgwood
2005-01-20  3:01   ` Andrew Morton
2005-01-20  3:18     ` Chris Wedgwood
2005-01-20  8:59       ` Peter Chubb
2005-01-20 15:51         ` Linus Torvalds
2005-01-20 16:08           ` [patch 1/3] spinlock fix #1, *_can_lock() primitives Ingo Molnar
2005-01-20 16:11             ` [patch 2/3] spinlock fix #2: generalize [spin|rw]lock yielding Ingo Molnar
2005-01-20 16:12               ` [patch 3/3] spinlock fix #3: type-checking spinlock primitives, x86 Ingo Molnar
2005-01-20 16:14                 ` [patch] stricter type-checking rwlock " Ingo Molnar
2005-01-20 16:16                   ` [patch] minor spinlock cleanups Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox