linux-kernel - [PATCH] percpu: Revert ("percpu: Replace smp_read_barrier_depends() with lockless

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160525201157.GI3354@mtj.duckdns.org>
Date:	Wed, 25 May 2016 16:11:57 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Alexey Dobriyan <adobriyan@...il.com>, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Pranith Kumar <bobby.prani@...il.com>, kernel-team@...com
Subject: [PATCH] percpu: Revert ("percpu: Replace smp_read_barrier_depends()
 with lockless_dereference()")

lockless_dereference() is planned to grow a sanity check to ensure
that the input parameter is a pointer.  __ref_is_percpu() passes in an
unsinged long value which is a combination of a pointer and a flag.
While it can be casted to a pointer lvalue, the casting looks messy
and it's a special case anyway.  Let's revert back to open-coding
READ_ONCE() and explicit barrier.

This doesn't cause any functional changes.

Signed-off-by: Tejun Heo <tj@...nel.org>
Link: http://lkml.kernel.org/g/20160522185040.GA23664@p183.telecom.by
Cc: Pranith Kumar <bobby.prani@...il.com>
Cc: Alexey Dobriyan <adobriyan@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>
---
So, something like this.  Please feel free to include in the series.

Thanks.

 include/linux/percpu-refcount.h |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
index 84f542d..1c7eec0 100644
--- a/include/linux/percpu-refcount.h
+++ b/include/linux/percpu-refcount.h
@@ -136,14 +136,12 @@ static inline bool __ref_is_percpu(struct percpu_ref *ref,
 	 * used as a pointer.  If the compiler generates a separate fetch
 	 * when using it as a pointer, __PERCPU_REF_ATOMIC may be set in
 	 * between contaminating the pointer value, meaning that
-	 * ACCESS_ONCE() is required when fetching it.
-	 *
-	 * Also, we need a data dependency barrier to be paired with
-	 * smp_store_release() in __percpu_ref_switch_to_percpu().
-	 *
-	 * Use lockless deref which contains both.
+	 * READ_ONCE() is required when fetching it.
 	 */
-	percpu_ptr = lockless_dereference(ref->percpu_count_ptr);
+	percpu_ptr = READ_ONCE(ref->percpu_count_ptr);
+
+	/* paired with smp_store_release() in __percpu_ref_switch_to_percpu() */
+	smp_read_barrier_depends();
 
 	/*
 	 * Theoretically, the following could test just ATOMIC; however,