linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Linus Torvalds <torvalds@osdl.org>
Cc: Peter Chubb <peterc@gelato.unsw.edu.au>,
	Chris Wedgwood <cw@f00f.org>, Andrew Morton <akpm@osdl.org>,
	paulus@samba.org, linux-kernel@vger.kernel.org,
	tony.luck@intel.com, dsw@gelato.unsw.edu.au,
	benh@kernel.crashing.org, linux-ia64@vger.kernel.org,
	hch@infradead.org, wli@holomorphy.com, jbarnes@sgi.com
Subject: [patch 1/3] spinlock fix #1, *_can_lock() primitives
Date: Thu, 20 Jan 2005 17:08:39 +0100	[thread overview]
Message-ID: <20050120160839.GA13067@elte.hu> (raw)
In-Reply-To: <Pine.LNX.4.58.0501200747230.8178@ppc970.osdl.org>


* Linus Torvalds <torvalds@osdl.org> wrote:

> We have a sense problem with the "trylock()" cases - some return "it
> was locked" (semaphores), and some return "I succeeded" (spinlocks),
> so not only is the sense not immediately obvious from the usage, it's
> actually _different_ for semaphores and for spinlocks.

well, this is primarily a problem of the semaphore primitives. 

anyway, here's my first patch again, with s/trylock_test/can_lock/.

	Ingo

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

  spin_can_lock(lock)
  read_can_lock(lock)
  write_can_lock(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.

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##_can_lock(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##_can_lock(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_can_lock - would spin_trylock() succeed?
+ * @lock: the spinlock in question.
+ */
+#define spin_can_lock(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_can_lock - would read_trylock() succeed?
+ * @lock: the rwlock in question.
+ */
+#define read_can_lock(x) (atomic_read((atomic_t *)&(x)->lock) > 0)
+
+/**
+ * write_can_lock - would write_trylock() succeed?
+ * @lock: the rwlock in question.
+ */
+#define write_can_lock(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.

  reply	other threads:[~2005-01-20 16:13 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-01-17  5:50 Horrible regression with -CURRENT from "Don't busy-lock-loop in preemptable spinlocks" patch Chris Wedgwood
2005-01-17  7:09 ` Andrew Morton
2005-01-17  7:33   ` Chris Wedgwood
2005-01-17  7:50     ` Paul Mackerras
2005-01-17  8:00       ` Chris Wedgwood
2005-01-17 14:33   ` Ingo Molnar
2005-01-18  1:47     ` Darren Williams
2005-01-18  4:28       ` Darren Williams
2005-01-18  7:08         ` Chris Wedgwood
2005-01-19  0:14       ` Peter Chubb
2005-01-19  8:04         ` Ingo Molnar
2005-01-19  9:18           ` Peter Chubb
2005-01-19  9:20             ` Ingo Molnar
2005-01-19 21:43               ` 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  3:33                       ` Andrew Morton
2005-01-20  8:59                       ` Peter Chubb
2005-01-20 13:04                         ` Ingo Molnar
2005-01-20 15:51                         ` Linus Torvalds
2005-01-20 16:08                           ` Ingo Molnar [this message]
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
2005-01-20 16:31                             ` [patch 1/3] spinlock fix #1, *_can_lock() primitives Linus Torvalds
2005-01-20 16:40                               ` Ingo Molnar
2005-01-20 17:48                                 ` Linus Torvalds
2005-01-20 17:53                                   ` Ingo Molnar
2005-01-20 18:22                                     ` [patch, BK-curr] nonintrusive spin-polling loop in kernel/spinlock.c Ingo Molnar
2005-01-20 18:25                                       ` [patch, BK-curr] rename 'lock' to 'slock' in asm-i386/spinlock.h Ingo Molnar
2005-01-20 23:45                                       ` [patch, BK-curr] nonintrusive spin-polling loop in kernel/spinlock.c Linus Torvalds
2005-01-20 16:44                               ` [patch 1/3] spinlock fix #1, *_can_lock() primitives Ingo Molnar
2005-01-20 16:59                                 ` Ingo Molnar
2005-01-20 16:47                               ` Ingo Molnar
2005-01-20 16:57                               ` Ingo Molnar
2005-01-20 16:05                       ` [PATCH RFC] 'spinlock/rwlock fixes' V3 [1/1] Linus Torvalds
2005-01-20 16:20                         ` Ingo Molnar
2005-01-20 16:18                   ` Linus Torvalds
2005-01-20 16:23                     ` Ingo Molnar
2005-01-20 17:30                       ` Linus Torvalds
2005-01-20 17:38                         ` Ingo Molnar
2005-01-20 16:28                     ` Ingo Molnar
2005-01-20  5:49                 ` Horrible regression with -CURRENT from "Don't busy-lock-loop in preemptable spinlocks" patch Grant Grundler
2005-01-17  7:38 ` [PATCH] __get_cpu_var should use __smp_processor_id() not smp_processor_id() Chris Wedgwood
2005-01-17 14:40   ` Ingo Molnar
2005-01-17 18:53     ` Chris Wedgwood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050120160839.GA13067@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=benh@kernel.crashing.org \
    --cc=cw@f00f.org \
    --cc=dsw@gelato.unsw.edu.au \
    --cc=hch@infradead.org \
    --cc=jbarnes@sgi.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=peterc@gelato.unsw.edu.au \
    --cc=tony.luck@intel.com \
    --cc=torvalds@osdl.org \
    --cc=wli@holomorphy.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).