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

Message ID 1507573730-8083-1-git-send-email-mark.rutland@arm.com
Headers show
Series
  • Preparatory work to kill off ACCESS_ONCE()
Related show

Message

Mark Rutland Oct. 9, 2017, 6:28 p.m.
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].

Thanks,
Mark.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git core/access-once
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/commit/?h=core/access-once

----
// Convert trivial ACCESS_ONCE() uses to equivalent READ_ONCE() and
// WRITE_ONCE()

virtual patch

@ depends on patch @
expression E1, E2;
@@

- ACCESS_ONCE(E1) = E2
+ WRITE_ONCE(E1, E2)

@ depends on patch @
expression E;
@@

- ACCESS_ONCE(E)
+ READ_ONCE(E)
*** BLURB HERE ***
----

Mark Rutland (13):
  dm integrity: kill off ACCESS_ONCE()
  EDAC, altera: kill off ACCESS_ONCE()
  firmware/ivc: kill off ACCESS_ONCE()
  fs: dcache: kill off ACCESS_ONCE()
  fs: ncpfs: kill off ACCESS_ONCE()
  media: dvb_ringbuffer: kill off ACCESS_ONCE()
  net: netlink/netfilter: kill off ACCESS_ONCE()
  net/ipv4/tcp_input.c: kill off ACCESS_ONCE()
  net: average: kill off ACCESS_ONCE()
  samples: mic/mpssd/mpssd.c: kill off ACCESS_ONCE()
  selftests/powerpc: kill off ACCESS_ONCE()
  workqueue: kill off ACCESS_ONCE()
  rcutorture: formal: prepare for ACCESS_ONCE() removal

 drivers/edac/altera_edac.c                         | 10 ++++-----
 drivers/firmware/tegra/ivc.c                       | 24 +++++++++++-----------
 drivers/md/dm-integrity.c                          | 15 +++++++-------
 drivers/media/dvb-core/dvb_ringbuffer.c            |  8 ++++----
 fs/dcache.c                                        | 18 ++++++++--------
 fs/ncpfs/dir.c                                     |  9 --------
 include/linux/average.h                            | 10 ++++++---
 include/linux/dcache.h                             |  4 ++--
 include/linux/genetlink.h                          |  2 +-
 include/linux/netfilter/nfnetlink.h                |  2 +-
 include/linux/rtnetlink.h                          |  2 +-
 include/net/netfilter/nf_tables.h                  |  4 ++--
 kernel/workqueue.c                                 |  4 ++--
 net/ipv4/tcp_input.c                               |  6 +++---
 net/netfilter/ipvs/ip_vs_sync.c                    |  2 +-
 net/netfilter/nfnetlink_queue.c                    |  4 ++--
 samples/mic/mpssd/mpssd.c                          |  6 +++---
 tools/testing/selftests/powerpc/dscr/dscr.h        |  2 +-
 .../selftests/powerpc/dscr/dscr_default_test.c     |  2 +-
 .../rcutorture/formal/srcu-cbmc/src/barriers.h     |  7 ++++---
 20 files changed, 69 insertions(+), 72 deletions(-)

-- 
1.9.1

Comments

Paul E. McKenney Oct. 9, 2017, 8:02 p.m. | #1
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@linux.vnet.ibm.com>
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@linux.vnet.ibm.com>


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();