[PATCH] locking/barriers: evaluate the argument to lockless_dereference only once

[PATCH] locking/barriers: evaluate the argument to lockless_dereference only once

[Mateusz Guzik]

The commit 25841ee0e9d2a ("Validate lockless_dereference() is used on a
pointer type") added a second use of the parameter to the macro.

This leads to trouble with consumers which use arguments with side
effects.

Instead, reuse the value which was read the first time.

Signed-off-by: Mateusz Guzik <mguzik@redhat.com>

---
 include/linux/compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index e9c6417..06f27fd 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -551,8 +551,8 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
  */
 #define lockless_dereference(p) \
 ({ \
-	__maybe_unused const void * const _________p2 = p; \
 	typeof(p) _________p1 = READ_ONCE(p); \
+	__maybe_unused const void * const _________p2 = _________p1; \
 	smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
 	(_________p1); \
 })
-- 
1.8.3.1

Re: [PATCH] locking/barriers: evaluate the argument to lockless_dereference only once

[Peter Zijlstra]

On Tue, Jun 07, 2016 at 03:14:55PM +0200, Mateusz Guzik wrote:
> The commit 25841ee0e9d2a ("Validate lockless_dereference() is used on a
> pointer type") added a second use of the parameter to the macro.
> 
> This leads to trouble with consumers which use arguments with side
> effects.

Current tip/locking/core doesn't have that patch anymore IIRC, Ingo
folded your thing and we now have:


commit 331b6d8c7afc2e5b900b9dcd850c265e1ba8d8e7
Author: Peter Zijlstra <peterz@infradead.org>
Date:   Sun May 22 12:48:27 2016 +0200

    locking/barriers: Validate lockless_dereference() is used on a pointer type
    
    Use the type to validate the argument @p is indeed a pointer type.
    
    Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: Alexey Dobriyan <adobriyan@gmail.com>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Link: http://lkml.kernel.org/r/20160522104827.GP3193@twins.programming.kicks-ass.net
    Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 793c0829e3a3..06f27fd9d760 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -545,10 +545,14 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
  * Similar to rcu_dereference(), but for situations where the pointed-to
  * object's lifetime is managed by something other than RCU.  That
  * "something other" might be reference counting or simple immortality.
+ *
+ * The seemingly unused void * variable is to validate @p is indeed a pointer
+ * type. All pointer types silently cast to void *.
  */
 #define lockless_dereference(p) \
 ({ \
 	typeof(p) _________p1 = READ_ONCE(p); \
+	__maybe_unused const void * const _________p2 = _________p1; \
 	smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
 	(_________p1); \
 })