Re: [PATCH 00/13] Preparatory work to kill off ACCESS_ONCE()

From: Paul E. McKenney
Date: Mon Oct 09 2017 - 16:02:36 EST


On Mon, Oct 09, 2017 at 07:28:37PM +0100, Mark Rutland wrote:
> Hi all,
>
> There's a general want to kill off ACCESS_ONCE(), which is required to kill off
> smp_read_barrier_depends(), and to support debug features which require
> instrumenting reads and writes separately.
>
> Thanks to preparatory work by a number of people, it's largely possible to
> script this with the Coccinelle patch below. However, this breaks a handful of
> cases, and renders some comments stale.
>
> This series fixes up said cases, and comments. Where fixups have been made,
> I've converted the entire file for consistency. The remaining code can be
> converted by Coccinelle script, allowing for the subsequent removal of
> ACCESS_ONCE().
>
> I've pushed this series, complete with an example treewide conversion of
> v4.14-rc4 to my core/access-once branch [1,2].

And this all that is left, aside from definitions and comments associated
with those definitions. Cool!

Feel free to pull this into your preparation series.

Thanx, Paul

------------------------------------------------------------------------

commit ed940234966f4857e23dd5f16aa8f200fc85dac6
Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Date: Mon Oct 9 12:59:18 2017 -0700

treewide: Kill off remaining ACCESS_ONCE()

This commit removes instances of ACCESS_ONCE() not located by Mark Rutland's
coccinelle script, for example, those in complex macro definitions and
those in comments. Removing ACCESS_ONCE() in favor of READ_ONCE() and
WRITE_ONCE() provides tools such as KTSAN the read/write distinction that
they need to better detect concurrency bugs. In addition, removing
ACCESS_ONCE() is one necessary step towards removing special cases for
DEC Alpha from the Linux-kernel memory model.

Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

diff --git a/Documentation/filesystems/path-lookup.md b/Documentation/filesystems/path-lookup.md
index 1b39e084a2b2..50ea32a51626 100644
--- a/Documentation/filesystems/path-lookup.md
+++ b/Documentation/filesystems/path-lookup.md
@@ -826,9 +826,9 @@ If the filesystem may need to revalidate dcache entries, then
*is* passed the dentry but does not have access to the `inode` or the
`seq` number from the `nameidata`, so it needs to be extra careful
when accessing fields in the dentry. This "extra care" typically
-involves using `ACCESS_ONCE()` or the newer [`READ_ONCE()`] to access
-fields, and verifying the result is not NULL before using it. This
-pattern can be see in `nfs_lookup_revalidate()`.
+involves using [`READ_ONCE()`] to access fields, and verifying the
+result is not NULL before using it. This pattern can be see in
+`nfs_lookup_revalidate()`.

A pair of patterns
------------------
diff --git a/mm/memory.c b/mm/memory.c
index a728bed16c20..cae514e7dcfc 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3891,9 +3891,9 @@ static int handle_pte_fault(struct vm_fault *vmf)
/*
* some architectures can have larger ptes than wordsize,
* e.g.ppc44x-defconfig has CONFIG_PTE_64BIT=y and
- * CONFIG_32BIT=y, so READ_ONCE or ACCESS_ONCE cannot guarantee
- * atomic accesses. The code below just needs a consistent
- * view for the ifs and we later double check anyway with the
+ * CONFIG_32BIT=y, so READ_ONCE cannot guarantee atomic
+ * accesses. The code below just needs a consistent view
+ * for the ifs and we later double check anyway with the
* ptl lock held. So here a barrier will do.
*/
barrier();