Patchwork >Re: [RFC] should VM_BUG_ON(cond) really evaluate cond

login
register
mail settings
Submitter Linus Torvalds
Date Oct. 29, 2011, 5:34 p.m.
Message ID <CA+55aFz=sxRjw6u-ww0P=9hhvWaZP=+QJZ68W+B9WtvQqj9Ogg@mail.gmail.com>
Download mbox | patch
Permalink /patch/122544/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Linus Torvalds - Oct. 29, 2011, 5:34 p.m.
On Fri, Oct 28, 2011 at 7:55 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Comments? I think I'm open to tested patches..

Here's a *untested* patch.

In particular, I worry that I'd need to add a "#include
<linux/compiler.h>" to some header file, although I suspect it gets
included some way regardless.

And when I say "untested", I mean it. I verified that this makes
*some* difference to the generated code, but I didn't actually check
if it really matters, or if it actually compiles and works in general.

Caveat tester,

                                Linus
Stephen Rothwell - Nov. 1, 2011, 4:06 a.m.
Hi Linus,

On Sat, 29 Oct 2011 10:34:30 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> In particular, I worry that I'd need to add a "#include
> <linux/compiler.h>" to some header file, although I suspect it gets
> included some way regardless.

Its that assumption that I usually pull people up on ... See
Documentation/SubmitChecklist Rule 1.  It may work for you, but may well
break on some other config or architecture build.

Patch

 arch/x86/include/asm/bitops.h |    5 +++--
 include/asm-generic/atomic.h  |    2 +-
 include/linux/compiler.h      |    7 +++++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 1775d6e5920e..e3982cb42fe5 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -308,8 +308,9 @@  static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
 
 static __always_inline int constant_test_bit(unsigned int nr, const volatile unsigned long *addr)
 {
-	return ((1UL << (nr % BITS_PER_LONG)) &
-		(addr[nr / BITS_PER_LONG])) != 0;
+	unsigned long *word = (nr / BITS_PER_LONG) + (unsigned long *)addr;
+	unsigned long bit = 1UL << (nr % BITS_PER_LONG);
+	return (bit & ACCESS_AT_MOST_ONCE(*word)) != 0;
 }
 
 static inline int variable_test_bit(int nr, volatile const unsigned long *addr)
diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
index e37963c1df4d..c05e21f7a985 100644
--- a/include/asm-generic/atomic.h
+++ b/include/asm-generic/atomic.h
@@ -39,7 +39,7 @@ 
  * Atomically reads the value of @v.
  */
 #ifndef atomic_read
-#define atomic_read(v)	(*(volatile int *)&(v)->counter)
+#define atomic_read(v)	ACCESS_AT_MOST_ONCE((v)->counter)
 #endif
 
 /**
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 320d6c94ff84..d30ffc685241 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -308,4 +308,11 @@  void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
  */
 #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
 
+/*
+ * Like ACCESS_ONCE, but can be optimized away if nothing uses the value,
+ * and/or merged with previous non-ONCE accesses.
+ */
+#define ACCESS_AT_MOST_ONCE(x) \
+	({ typeof(x) __y; asm("":"=r" (__y):"0" (x)); __y; })
+
 #endif /* __LINUX_COMPILER_H */