All of lore.kernel.org
 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 16:08:39 +0000	[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.

WARNING: multiple messages have this Message-ID (diff)
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:08 UTC|newest]

Thread overview: 87+ 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  1:47       ` Darren Williams
2005-01-18  4:28       ` Darren Williams
2005-01-18  4:28         ` Darren Williams
2005-01-18  7:08         ` Chris Wedgwood
2005-01-18  7:08           ` Chris Wedgwood
2005-01-19  0:14       ` Peter Chubb
2005-01-19  0:14         ` Peter Chubb
2005-01-19  8:04         ` Ingo Molnar
2005-01-19  8:04           ` Ingo Molnar
2005-01-19  9:18           ` Peter Chubb
2005-01-19  9:18             ` Peter Chubb
2005-01-19  9:20             ` Ingo Molnar
2005-01-19  9:20               ` Ingo Molnar
2005-01-19 21:43               ` Paul Mackerras
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  2:34                   ` Chris Wedgwood
2005-01-20  3:01                   ` Andrew Morton
2005-01-20  3:01                     ` Andrew Morton
2005-01-20  3:18                     ` Chris Wedgwood
2005-01-20  3:18                       ` Chris Wedgwood
2005-01-20  3:33                       ` Andrew Morton
2005-01-20  3:33                         ` Andrew Morton
2005-01-20  8:59                       ` Peter Chubb
2005-01-20  8:59                         ` Peter Chubb
2005-01-20 13:04                         ` Ingo Molnar
2005-01-20 13:04                           ` Ingo Molnar
2005-01-20 15:51                         ` Linus Torvalds
2005-01-20 15:51                           ` Linus Torvalds
2005-01-20 16:08                           ` Ingo Molnar [this message]
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:11                               ` Ingo Molnar
2005-01-20 16:12                               ` [patch 3/3] spinlock fix #3: type-checking spinlock primitives, x86 Ingo Molnar
2005-01-20 16:12                                 ` Ingo Molnar
2005-01-20 16:14                                 ` [patch] stricter type-checking rwlock " Ingo Molnar
2005-01-20 16:14                                   ` Ingo Molnar
2005-01-20 16:16                                   ` [patch] minor spinlock cleanups Ingo Molnar
2005-01-20 16:16                                     ` Ingo Molnar
2005-01-20 16:31                             ` [patch 1/3] spinlock fix #1, *_can_lock() primitives Linus Torvalds
2005-01-20 16:31                               ` Linus Torvalds
2005-01-20 16:40                               ` Ingo Molnar
2005-01-20 16:40                                 ` Ingo Molnar
2005-01-20 17:48                                 ` Linus Torvalds
2005-01-20 17:48                                   ` Linus Torvalds
2005-01-20 17:53                                   ` Ingo Molnar
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:22                                       ` Ingo Molnar
2005-01-20 18:25                                       ` [patch, BK-curr] rename 'lock' to 'slock' in asm-i386/spinlock.h Ingo Molnar
2005-01-20 18:25                                         ` Ingo Molnar
2005-01-20 23:45                                       ` [patch, BK-curr] nonintrusive spin-polling loop in kernel/spinlock.c Linus Torvalds
2005-01-20 23:45                                         ` Linus Torvalds
2005-01-20 16:44                               ` [patch 1/3] spinlock fix #1, *_can_lock() primitives Ingo Molnar
2005-01-20 16:44                                 ` Ingo Molnar
2005-01-20 16:59                                 ` Ingo Molnar
2005-01-20 16:59                                   ` Ingo Molnar
2005-01-20 16:47                               ` Ingo Molnar
2005-01-20 16:47                                 ` Ingo Molnar
2005-01-20 16:57                               ` 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:05                         ` Linus Torvalds
2005-01-20 16:20                         ` Ingo Molnar
2005-01-20 16:20                           ` Ingo Molnar
2005-01-20 16:18                   ` Linus Torvalds
2005-01-20 16:18                     ` Linus Torvalds
2005-01-20 16:23                     ` Ingo Molnar
2005-01-20 16:23                       ` Ingo Molnar
2005-01-20 17:30                       ` Linus Torvalds
2005-01-20 17:30                         ` Linus Torvalds
2005-01-20 17:38                         ` Ingo Molnar
2005-01-20 17:38                           ` Ingo Molnar
2005-01-20 16:28                     ` 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-20  5:49                   ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.