Skip to contentSkip to conversation

Get rid of lockless_dereference()

PATCH New Will Deacon 2 participants 11 messages Oct 24, 2017

Subsystems (19)
lore
Share & tools
lore
#0 lore raw

v2 cover Get rid of lockless_dereference()

Hi Ingo, Paul,

Here's another spin of the patches removing lockless_dereference(), taking into account Ingo's review comments.

Thanks,

Will

--->8

Will Deacon (5):
linux/compiler.h: Split into compiler.h and compiler_types.h locking/barriers: Add implicit smp_read_barrier_depends() to

    READ_ONCE()
  locking/barriers: Convert users of lockless_dereference() to
    READ_ONCE()

locking/barriers: Kill lockless_dereference()
alpha: atomics: Add smp_read_barrier_depends() to release/relaxed
atomics

 Documentation/memory-barriers.txt                  |  12 -
 .../translations/ko_KR/memory-barriers.txt         |  12 -
 arch/alpha/include/asm/atomic.h                    |  13 +
 arch/arm/include/asm/ptrace.h                      |   3 +-
 arch/sparc/include/asm/ptrace.h                    |   1 +
 arch/um/include/shared/init.h                      |   2 +-
 arch/x86/events/core.c                             |   2 +-
 arch/x86/include/asm/mmu_context.h                 |   4 +-
 arch/x86/kernel/ldt.c                              |   2 +-
 drivers/md/dm-mpath.c                              |  20 +-
 fs/dcache.c                                        |   4 +-
 fs/overlayfs/ovl_entry.h                           |   2 +-
 fs/overlayfs/readdir.c                             |   2 +-
 include/linux/compiler-clang.h                     |   2 +-
 include/linux/compiler-gcc.h                       |   2 +-
 include/linux/compiler-intel.h                     |   2 +-
 include/linux/compiler.h                           | 286 +--------------------
 include/linux/compiler_types.h                     | 274 ++++++++++++++++++++
 include/linux/linkage.h                            |   2 +-
 include/linux/rculist.h                            |   4 +-
 include/linux/rcupdate.h                           |   4 +-
 include/uapi/linux/stddef.h                        |   2 +-
 kernel/events/core.c                               |   4 +-
 kernel/seccomp.c                                   |   2 +-
 kernel/task_work.c                                 |   2 +-
 mm/slab.h                                          |   2 +-
 scripts/headers_install.sh                         |   2 +-
 27 files changed, 327 insertions(+), 342 deletions(-)
 create mode 100644 include/linux/compiler_types.h
Signature · 1 line
2.1.4
#1 lore raw

PATCH v2 1/5 linux/compiler.h: Split into compiler.h and compiler_types.h

linux/compiler.h is included indirectly by linux/types.h via uapi/linux/types.h -> uapi/linux/posix_types.h -> linux/stddef.h -> uapi/linux/stddef.h and is needed to provide a proper definition of offsetof.

Unfortunately, compiler.h requires a definition of smp_read_barrier_depends() for defining lockless_dereference() and soon for defining READ_ONCE(), which means that all
users of READ_ONCE() will need to include asm/barrier.h to avoid splats such as:

In file included from include/uapi/linux/stddef.h:1:0,

                    from include/linux/stddef.h:4,
                    from arch/h8300/kernel/asm-offsets.c:11:

include/linux/list.h: In function 'list_empty':

Quoted 1 line
Quoted 1 line

include/linux/compiler.h:343:2: error: implicit declaration of function 'smp_read_barrier_depends' [-Werror=implicit-function-declaration]

     smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
     ^

A better alternative is to include asm/barrier.h in linux/compiler.h, but this requires a type definition for "bool" on some architectures (e.g. x86), which is defined later by linux/types.h. Type "bool" is also used directly in linux/compiler.h, so the whole thing is pretty fragile.

This patch splits compiler.h in two: compiler_types.h contains type annotations, definitions and the compiler-specific parts, whereas compiler.h #includes compiler-types.h and additionally defines macros such as {READ,WRITE.ACCESS}_ONCE().

uapi/linux/stddef.h and linux/linkage.h are then moved over to include linux/compiler_types.h, which fixes the build for h8 and blackfin.

Cc Ingo Molnar <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 arch/arm/include/asm/ptrace.h   |   3 +-
 arch/sparc/include/asm/ptrace.h |   1 +
 arch/um/include/shared/init.h   |   2 +-
 include/linux/compiler-clang.h  |   2 +-
 include/linux/compiler-gcc.h    |   2 +-
 include/linux/compiler-intel.h  |   2 +-
 include/linux/compiler.h        | 265 +-------------------------------------
 include/linux/compiler_types.h  | 274 ++++++++++++++++++++++++++++++++++++++++
 include/linux/linkage.h         |   2 +-
 include/uapi/linux/stddef.h     |   2 +-
 scripts/headers_install.sh      |   2 +-
 11 files changed, 286 insertions(+), 271 deletions(-)
 create mode 100644 include/linux/compiler_types.h
arch/arm/include/asm/ptrace.h +1 -2
index e9c9a117bd25..c7cdbb43ae7c 100644
@@ -126,8 +126,7 @@ extern unsigned long profile_pc(struct pt_regs *regs);
 /*
  * kprobe-based event tracer support
  */
-#include <linux/stddef.h>
-#include <linux/types.h>
+#include <linux/compiler.h>
 #define MAX_REG_OFFSET (offsetof(struct pt_regs, ARM_ORIG_r0))
 
 extern int regs_query_register_offset(const char *name);
arch/sparc/include/asm/ptrace.h +1 -0
index d73428e4333c..b383484edcd3 100644
@@ -6,6 +6,7 @@
 #if defined(__sparc__) && defined(__arch64__)
 #ifndef __ASSEMBLY__
 
+#include <linux/compiler.h>
 #include <linux/threads.h>
 #include <asm/switch_to.h>
 
arch/um/include/shared/init.h +1 -1
index 233e2593eee0..094e96ce653b 100644
@@ -40,7 +40,7 @@
 typedef int (*initcall_t)(void);
 typedef void (*exitcall_t)(void);
 
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 
 /* These are for everybody (although not all archs will actually
    discard it in modules) */
include/linux/compiler-clang.h +1 -1
index de179993e039..5947a3e6c0e6 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-clang.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler-gcc.h +1 -1
index 16d41de92ee3..ce8e965646ef 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler-intel.h +1 -1
index d4c71132d07f..e438ac89c692 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-intel.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler.h +3 -262

287 lines not on this page · raw message

include/linux/compiler_types.h new file +274 -0

274 lines not on this page · raw message

include/linux/linkage.h +1 -1
index a6a42dd02466..ebd61b80fed4 100644
@@ -1,7 +1,7 @@
 #ifndef _LINUX_LINKAGE_H
 #define _LINUX_LINKAGE_H
 
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 #include <linux/stringify.h>
 #include <linux/export.h>
 #include <asm/linkage.h>
include/uapi/linux/stddef.h +1 -1
index 621fa8ac4425..d1f7cb732dfc 100644
@@ -1,4 +1,4 @@
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 
 #ifndef __always_inline
 #define __always_inline inline
scripts/headers_install.sh +1 -1
index fdebd66f8fc1..63b8cc26456a 100755
@@ -33,7 +33,7 @@ do
 	sed -r \
 		-e 's/([ \t(])(__user|__force|__iomem)[ \t]/\1/g' \
 		-e 's/__attribute_const__([ \t]|$)/\1/g' \
-		-e 's@^#include <linux/compiler.h>@@' \
+		-e 's@^#include <linux/compiler(|_types).h>@@' \
 		-e 's/(^|[^a-zA-Z0-9])__packed([^a-zA-Z0-9_]|$)/\1__attribute__((packed))\2/g' \
 		-e 's/(^|[ \t(])(inline|asm|volatile)([ \t(]|$)/\1__\2__\3/g' \
 		-e 's@#(ifndef|define|endif[ \t]*/[*])[ \t]*_UAPI@#\1 @' \
Signature · 1 line
2.1.4
#2 lore raw

[tip:locking/core] linux/compiler.h: Split into compiler.h and compiler_types.h

linux/compiler.h: Split into compiler.h and compiler_types.h
Commit-ID:  d15155824c5014803d91b829736d249c500bdda6
Gitweb:     https://git.kernel.org/tip/d15155824c5014803d91b829736d249c500bdda6
Author:     Will Deacon <[email protected]>
AuthorDate: Tue, 24 Oct 2017 11:22:46 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Oct 2017 13:17:32 +0200

linux/compiler.h: Split into compiler.h and compiler_types.h

linux/compiler.h is included indirectly by linux/types.h via uapi/linux/types.h -> uapi/linux/posix_types.h -> linux/stddef.h -> uapi/linux/stddef.h and is needed to provide a proper definition of offsetof.

Unfortunately, compiler.h requires a definition of smp_read_barrier_depends() for defining lockless_dereference() and soon for defining READ_ONCE(), which means that all
users of READ_ONCE() will need to include asm/barrier.h to avoid splats such as:

In file included from include/uapi/linux/stddef.h:1:0,

                    from include/linux/stddef.h:4,
                    from arch/h8300/kernel/asm-offsets.c:11:

include/linux/list.h: In function 'list_empty':

Quoted 1 line
Quoted 1 line

include/linux/compiler.h:343:2: error: implicit declaration of function 'smp_read_barrier_depends' [-Werror=implicit-function-declaration]

     smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
     ^

A better alternative is to include asm/barrier.h in linux/compiler.h, but this requires a type definition for "bool" on some architectures (e.g. x86), which is defined later by linux/types.h. Type "bool" is also used directly in linux/compiler.h, so the whole thing is pretty fragile.

This patch splits compiler.h in two: compiler_types.h contains type annotations, definitions and the compiler-specific parts, whereas compiler.h #includes compiler-types.h and additionally defines macros such as {READ,WRITE.ACCESS}_ONCE().

uapi/linux/stddef.h and linux/linkage.h are then moved over to include linux/compiler_types.h, which fixes the build for h8 and blackfin.

---
 arch/arm/include/asm/ptrace.h   |   3 +-
 arch/sparc/include/asm/ptrace.h |   1 +
 arch/um/include/shared/init.h   |   2 +-
 include/linux/compiler-clang.h  |   2 +-
 include/linux/compiler-gcc.h    |   2 +-
 include/linux/compiler-intel.h  |   2 +-
 include/linux/compiler.h        | 265 +-------------------------------------
 include/linux/compiler_types.h  | 274 ++++++++++++++++++++++++++++++++++++++++
 include/linux/linkage.h         |   2 +-
 include/uapi/linux/stddef.h     |   2 +-
 scripts/headers_install.sh      |   2 +-
 11 files changed, 286 insertions(+), 271 deletions(-)
arch/arm/include/asm/ptrace.h +1 -2
index e9c9a11..c7cdbb4 100644
@@ -126,8 +126,7 @@ extern unsigned long profile_pc(struct pt_regs *regs);
 /*
  * kprobe-based event tracer support
  */
-#include <linux/stddef.h>
-#include <linux/types.h>
+#include <linux/compiler.h>
 #define MAX_REG_OFFSET (offsetof(struct pt_regs, ARM_ORIG_r0))
 
 extern int regs_query_register_offset(const char *name);
arch/sparc/include/asm/ptrace.h +1 -0
index d73428e..b383484 100644
@@ -6,6 +6,7 @@
 #if defined(__sparc__) && defined(__arch64__)
 #ifndef __ASSEMBLY__
 
+#include <linux/compiler.h>
 #include <linux/threads.h>
 #include <asm/switch_to.h>
 
arch/um/include/shared/init.h +1 -1
index 233e259..094e96c 100644
@@ -40,7 +40,7 @@
 typedef int (*initcall_t)(void);
 typedef void (*exitcall_t)(void);
 
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 
 /* These are for everybody (although not all archs will actually
    discard it in modules) */
include/linux/compiler-clang.h +1 -1
index de17999..5947a3e 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-clang.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler-gcc.h +1 -1
index 16d41de..ce8e965 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler-intel.h +1 -1
index d4c7113..e438ac89 100644
@@ -1,4 +1,4 @@
-#ifndef __LINUX_COMPILER_H
+#ifndef __LINUX_COMPILER_TYPES_H
 #error "Please don't include <linux/compiler-intel.h> directly, include <linux/compiler.h> instead."
 #endif
 
include/linux/compiler.h +3 -262

287 lines not on this page · raw message

include/linux/compiler_types.h new file +274 -0

274 lines not on this page · raw message

include/linux/linkage.h +1 -1
index a6a42dd..ebd61b8 100644
@@ -1,7 +1,7 @@
 #ifndef _LINUX_LINKAGE_H
 #define _LINUX_LINKAGE_H
 
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 #include <linux/stringify.h>
 #include <linux/export.h>
 #include <asm/linkage.h>
include/uapi/linux/stddef.h +1 -1
index 621fa8a..d1f7cb7 100644
@@ -1,4 +1,4 @@
-#include <linux/compiler.h>
+#include <linux/compiler_types.h>
 
 #ifndef __always_inline
 #define __always_inline inline
scripts/headers_install.sh +1 -1
index fdebd66..63b8cc2 100755
@@ -33,7 +33,7 @@ do
 	sed -r \
 		-e 's/([ \t(])(__user|__force|__iomem)[ \t]/\1/g' \
 		-e 's/__attribute_const__([ \t]|$)/\1/g' \
-		-e 's@^#include <linux/compiler.h>@@' \
+		-e 's@^#include <linux/compiler(|_types).h>@@' \
 		-e 's/(^|[^a-zA-Z0-9])__packed([^a-zA-Z0-9_]|$)/\1__attribute__((packed))\2/g' \
 		-e 's/(^|[ \t(])(inline|asm|volatile)([ \t(]|$)/\1__\2__\3/g' \
 		-e 's@#(ifndef|define|endif[ \t]*/[*])[ \t]*_UAPI@#\1 @' \
#3 lore raw

PATCH v2 2/5 locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()

In reply to Will Deacon

In preparation for the removal of lockless_dereference(), which is the same as READ_ONCE() on all architectures other than Alpha, add an implicit smp_read_barrier_depends() to READ_ONCE() so that it can be used to head dependency chains on all architectures.

Cc Ingo Molnar <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 include/linux/compiler.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/compiler.h +1 -0
index 08083186e54f..7d7b77da9716 100644
@@ -242,6 +242,7 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 		__read_once_size(&(x), __u.__c, sizeof(x));		\
 	else								\
 		__read_once_size_nocheck(&(x), __u.__c, sizeof(x));	\
+	smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
 	__u.__val;							\
 })
 #define READ_ONCE(x) __READ_ONCE(x, 1)
Signature · 1 line
2.1.4
#4 lore raw

[tip:locking/core] locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()

locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()
Commit-ID:  76ebbe78f7390aee075a7f3768af197ded1bdfbb
Gitweb:     https://git.kernel.org/tip/76ebbe78f7390aee075a7f3768af197ded1bdfbb
Author:     Will Deacon <[email protected]>
AuthorDate: Tue, 24 Oct 2017 11:22:47 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Oct 2017 13:17:32 +0200

locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()

In preparation for the removal of lockless_dereference(), which is the same as READ_ONCE() on all architectures other than Alpha, add an implicit smp_read_barrier_depends() to READ_ONCE() so that it can be used to head dependency chains on all architectures.

---
 include/linux/compiler.h | 1 +
 1 file changed, 1 insertion(+)
include/linux/compiler.h +1 -0
index 0808318..7d7b77d 100644
@@ -242,6 +242,7 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 		__read_once_size(&(x), __u.__c, sizeof(x));		\
 	else								\
 		__read_once_size_nocheck(&(x), __u.__c, sizeof(x));	\
+	smp_read_barrier_depends(); /* Enforce dependency ordering from x */ \
 	__u.__val;							\
 })
 #define READ_ONCE(x) __READ_ONCE(x, 1)
#5 lore raw

PATCH v2 3/5 locking/barriers: Convert users of lockless_dereference() to READ_ONCE()

In reply to Will Deacon

READ_ONCE() now has an implicit smp_read_barrier_depends() call, so it can be used instead of lockless_dereference() without any change in semantics.

Cc Ingo Molnar <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 arch/x86/events/core.c             |  2 +-
 arch/x86/include/asm/mmu_context.h |  4 ++--
 arch/x86/kernel/ldt.c              |  2 +-
 drivers/md/dm-mpath.c              | 20 ++++++++++----------
 fs/dcache.c                        |  4 ++--
 fs/overlayfs/ovl_entry.h           |  2 +-
 fs/overlayfs/readdir.c             |  2 +-
 include/linux/rculist.h            |  4 ++--
 include/linux/rcupdate.h           |  4 ++--
 kernel/events/core.c               |  4 ++--
 kernel/seccomp.c                   |  2 +-
 kernel/task_work.c                 |  2 +-
 mm/slab.h                          |  2 +-
 13 files changed, 27 insertions(+), 27 deletions(-)
arch/x86/events/core.c +1 -1
index 80534d3c2480..589af1eec7c1 100644
@@ -2371,7 +2371,7 @@ static unsigned long get_segment_base(unsigned int segment)
 		struct ldt_struct *ldt;
 
 		/* IRQs are off, so this synchronizes with smp_store_release */
-		ldt = lockless_dereference(current->active_mm->context.ldt);
+		ldt = READ_ONCE(current->active_mm->context.ldt);
 		if (!ldt || idx >= ldt->nr_entries)
 			return 0;
 
arch/x86/include/asm/mmu_context.h +2 -2
index 3c856a15b98e..efc530642f7d 100644
@@ -72,8 +72,8 @@ static inline void load_mm_ldt(struct mm_struct *mm)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 	struct ldt_struct *ldt;
 
-	/* lockless_dereference synchronizes with smp_store_release */
-	ldt = lockless_dereference(mm->context.ldt);
+	/* READ_ONCE synchronizes with smp_store_release */
+	ldt = READ_ONCE(mm->context.ldt);
 
 	/*
 	 * Any change to mm->context.ldt is followed by an IPI to all
arch/x86/kernel/ldt.c +1 -1
index f0e64db18ac8..0a21390642c4 100644
@@ -101,7 +101,7 @@ static void finalize_ldt_struct(struct ldt_struct *ldt)
 static void install_ldt(struct mm_struct *current_mm,
 			struct ldt_struct *ldt)
 {
-	/* Synchronizes with lockless_dereference in load_mm_ldt. */
+	/* Synchronizes with READ_ONCE in load_mm_ldt. */
 	smp_store_release(&current_mm->context.ldt, ldt);
 
 	/* Activate the LDT for all CPUs using current_mm. */
drivers/md/dm-mpath.c +10 -10
index 11f273d2f018..3f88c9d32f7e 100644
@@ -366,7 +366,7 @@ static struct pgpath *choose_path_in_pg(struct multipath *m,
 
 	pgpath = path_to_pgpath(path);
 
-	if (unlikely(lockless_dereference(m->current_pg) != pg)) {
+	if (unlikely(READ_ONCE(m->current_pg) != pg)) {
 		/* Only update current_pgpath if pg changed */
 		spin_lock_irqsave(&m->lock, flags);
 		m->current_pgpath = pgpath;
@@ -390,7 +390,7 @@ static struct pgpath *choose_pgpath(struct multipath *m, size_t nr_bytes)
 	}
 
 	/* Were we instructed to switch PG? */
-	if (lockless_dereference(m->next_pg)) {
+	if (READ_ONCE(m->next_pg)) {
 		spin_lock_irqsave(&m->lock, flags);
 		pg = m->next_pg;
 		if (!pg) {
@@ -406,7 +406,7 @@ static struct pgpath *choose_pgpath(struct multipath *m, size_t nr_bytes)
 
 	/* Don't change PG until it has no remaining paths */
 check_current_pg:
-	pg = lockless_dereference(m->current_pg);
+	pg = READ_ONCE(m->current_pg);
 	if (pg) {
 		pgpath = choose_path_in_pg(m, pg, nr_bytes);
 		if (!IS_ERR_OR_NULL(pgpath))
@@ -473,7 +473,7 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq,
 	struct request *clone;
 
 	/* Do we need to select a new pgpath? */
-	pgpath = lockless_dereference(m->current_pgpath);
+	pgpath = READ_ONCE(m->current_pgpath);
 	if (!pgpath || !test_bit(MPATHF_QUEUE_IO, &m->flags))
 		pgpath = choose_pgpath(m, nr_bytes);
 
@@ -535,7 +535,7 @@ static int __multipath_map_bio(struct multipath *m, struct bio *bio, struct dm_m
 	bool queue_io;
 
 	/* Do we need to select a new pgpath? */
-	pgpath = lockless_dereference(m->current_pgpath);
+	pgpath = READ_ONCE(m->current_pgpath);
 	queue_io = test_bit(MPATHF_QUEUE_IO, &m->flags);
 	if (!pgpath || !queue_io)
 		pgpath = choose_pgpath(m, nr_bytes);
@@ -1804,7 +1804,7 @@ static int multipath_prepare_ioctl(struct dm_target *ti,
 	struct pgpath *current_pgpath;
 	int r;
 
-	current_pgpath = lockless_dereference(m->current_pgpath);
+	current_pgpath = READ_ONCE(m->current_pgpath);
 	if (!current_pgpath)
 		current_pgpath = choose_pgpath(m, 0);
 
@@ -1826,7 +1826,7 @@ static int multipath_prepare_ioctl(struct dm_target *ti,
 	}
 
 	if (r == -ENOTCONN) {
-		if (!lockless_dereference(m->current_pg)) {
+		if (!READ_ONCE(m->current_pg)) {
 			/* Path status changed, redo selection */
 			(void) choose_pgpath(m, 0);
 		}
@@ -1895,9 +1895,9 @@ static int multipath_busy(struct dm_target *ti)
 		return (m->queue_mode != DM_TYPE_MQ_REQUEST_BASED);
 
 	/* Guess which priority_group will be used at next mapping time */
-	pg = lockless_dereference(m->current_pg);
-	next_pg = lockless_dereference(m->next_pg);
-	if (unlikely(!lockless_dereference(m->current_pgpath) && next_pg))
+	pg = READ_ONCE(m->current_pg);
+	next_pg = READ_ONCE(m->next_pg);
+	if (unlikely(!READ_ONCE(m->current_pgpath) && next_pg))
 		pg = next_pg;
 
 	if (!pg) {
fs/dcache.c +2 -2
index f90141387f01..34c852af215c 100644
@@ -231,7 +231,7 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 {
 	/*
 	 * Be careful about RCU walk racing with rename:
-	 * use 'lockless_dereference' to fetch the name pointer.
+	 * use 'READ_ONCE' to fetch the name pointer.
 	 *
 	 * NOTE! Even if a rename will mean that the length
 	 * was not loaded atomically, we don't care. The
@@ -245,7 +245,7 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 	 * early because the data cannot match (there can
 	 * be no NUL in the ct/tcount data)
 	 */
-	const unsigned char *cs = lockless_dereference(dentry->d_name.name);
+	const unsigned char *cs = READ_ONCE(dentry->d_name.name);
 
 	return dentry_string_cmp(cs, ct, tcount);
 }
fs/overlayfs/ovl_entry.h +1 -1
index 25d9b5adcd42..36b49bd09264 100644
@@ -77,5 +77,5 @@ static inline struct ovl_inode *OVL_I(struct inode *inode)
 
 static inline struct dentry *ovl_upperdentry_dereference(struct ovl_inode *oi)
 {
-	return lockless_dereference(oi->__upperdentry);
+	return READ_ONCE(oi->__upperdentry);
 }
fs/overlayfs/readdir.c +1 -1
index 0f85ee9c3268..c67a7703296b 100644
@@ -754,7 +754,7 @@ static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end,
 	if (!od->is_upper && OVL_TYPE_UPPER(ovl_path_type(dentry))) {
 		struct inode *inode = file_inode(file);
 
-		realfile = lockless_dereference(od->upperfile);
+		realfile = READ_ONCE(od->upperfile);
 		if (!realfile) {
 			struct path upperpath;
 
include/linux/rculist.h +2 -2
index 2bea1d5e9930..5ed091c064b2 100644
@@ -274,7 +274,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
  * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
  */
 #define list_entry_rcu(ptr, type, member) \
-	container_of(lockless_dereference(ptr), type, member)
+	container_of(READ_ONCE(ptr), type, member)
 
 /*
  * Where are list_empty_rcu() and list_first_entry_rcu()?
@@ -367,7 +367,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
  * example is when items are added to the list, but never deleted.
  */
 #define list_entry_lockless(ptr, type, member) \
-	container_of((typeof(ptr))lockless_dereference(ptr), type, member)
+	container_of((typeof(ptr))READ_ONCE(ptr), type, member)
 
 /**
  * list_for_each_entry_lockless - iterate over rcu list of given type
include/linux/rcupdate.h +2 -2
index 1a9f70d44af9..a6ddc42f87a5 100644
@@ -346,7 +346,7 @@ static inline void rcu_preempt_sleep_check(void) { }
 #define __rcu_dereference_check(p, c, space) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(*p) *________p1 = (typeof(*p) *__force)lockless_dereference(p); \
+	typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
 	rcu_dereference_sparse(p, space); \
 	((typeof(*p) __force __kernel *)(________p1)); \
@@ -360,7 +360,7 @@ static inline void rcu_preempt_sleep_check(void) { }
 #define rcu_dereference_raw(p) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(p) ________p1 = lockless_dereference(p); \
+	typeof(p) ________p1 = READ_ONCE(p); \
 	((typeof(*p) __force __kernel *)(________p1)); \
 })
 
kernel/events/core.c +2 -2
index 9d93db81fa36..824a583079a1 100644
@@ -4231,7 +4231,7 @@ static void perf_remove_from_owner(struct perf_event *event)
 	 * indeed free this event, otherwise we need to serialize on
 	 * owner->perf_event_mutex.
 	 */
-	owner = lockless_dereference(event->owner);
+	owner = READ_ONCE(event->owner);
 	if (owner) {
 		/*
 		 * Since delayed_put_task_struct() also drops the last
@@ -4328,7 +4328,7 @@ int perf_event_release_kernel(struct perf_event *event)
 		 * Cannot change, child events are not migrated, see the
 		 * comment with perf_event_ctx_lock_nested().
 		 */
-		ctx = lockless_dereference(child->ctx);
+		ctx = READ_ONCE(child->ctx);
 		/*
 		 * Since child_mutex nests inside ctx::mutex, we must jump
 		 * through hoops. We start by grabbing a reference on the ctx.
kernel/seccomp.c +1 -1
index 0ae832e13b97..8ac79355915b 100644
@@ -189,7 +189,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
 	u32 ret = SECCOMP_RET_ALLOW;
 	/* Make sure cross-thread synced filter points somewhere sane. */
 	struct seccomp_filter *f =
-			lockless_dereference(current->seccomp.filter);
+			READ_ONCE(current->seccomp.filter);
 
 	/* Ensure unexpected behavior doesn't result in failing open. */
 	if (unlikely(WARN_ON(f == NULL)))
kernel/task_work.c +1 -1
index 836a72a66fba..9a9f262fc53d 100644
@@ -67,7 +67,7 @@ task_work_cancel(struct task_struct *task, task_work_func_t func)
 	 * we raced with task_work_run(), *pprev == NULL/exited.
 	 */
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
-	while ((work = lockless_dereference(*pprev))) {
+	while ((work = READ_ONCE(*pprev))) {
 		if (work->func != func)
 			pprev = &work->next;
 		else if (cmpxchg(pprev, work, work->next) == work)
mm/slab.h +1 -1
index 073362816acc..8894f811a89d 100644
@@ -258,7 +258,7 @@ cache_from_memcg_idx(struct kmem_cache *s, int idx)
 	 * memcg_caches issues a write barrier to match this (see
 	 * memcg_create_kmem_cache()).
 	 */
-	cachep = lockless_dereference(arr->entries[idx]);
+	cachep = READ_ONCE(arr->entries[idx]);
 	rcu_read_unlock();
 
 	return cachep;
Signature · 1 line
2.1.4
#6 lore raw

[tip:locking/core] locking/barriers: Convert users of lockless_dereference() to READ_ONCE()

locking/barriers: Convert users of lockless_dereference() to READ_ONCE()
Commit-ID:  506458efaf153c1ea480591c5602a5a3ba5a3b76
Gitweb:     https://git.kernel.org/tip/506458efaf153c1ea480591c5602a5a3ba5a3b76
Author:     Will Deacon <[email protected]>
AuthorDate: Tue, 24 Oct 2017 11:22:48 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Oct 2017 13:17:33 +0200

locking/barriers: Convert users of lockless_dereference() to READ_ONCE()

READ_ONCE() now has an implicit smp_read_barrier_depends() call, so it can be used instead of lockless_dereference() without any change in semantics.

---
 arch/x86/events/core.c             |  2 +-
 arch/x86/include/asm/mmu_context.h |  4 ++--
 arch/x86/kernel/ldt.c              |  2 +-
 drivers/md/dm-mpath.c              | 20 ++++++++++----------
 fs/dcache.c                        |  4 ++--
 fs/overlayfs/ovl_entry.h           |  2 +-
 fs/overlayfs/readdir.c             |  2 +-
 include/linux/rculist.h            |  4 ++--
 include/linux/rcupdate.h           |  4 ++--
 kernel/events/core.c               |  4 ++--
 kernel/seccomp.c                   |  2 +-
 kernel/task_work.c                 |  2 +-
 mm/slab.h                          |  2 +-
 13 files changed, 27 insertions(+), 27 deletions(-)
arch/x86/events/core.c +1 -1
index 80534d3..589af1e 100644
@@ -2371,7 +2371,7 @@ static unsigned long get_segment_base(unsigned int segment)
 		struct ldt_struct *ldt;
 
 		/* IRQs are off, so this synchronizes with smp_store_release */
-		ldt = lockless_dereference(current->active_mm->context.ldt);
+		ldt = READ_ONCE(current->active_mm->context.ldt);
 		if (!ldt || idx >= ldt->nr_entries)
 			return 0;
 
arch/x86/include/asm/mmu_context.h +2 -2
index 3c856a1..efc5306 100644
@@ -72,8 +72,8 @@ static inline void load_mm_ldt(struct mm_struct *mm)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 	struct ldt_struct *ldt;
 
-	/* lockless_dereference synchronizes with smp_store_release */
-	ldt = lockless_dereference(mm->context.ldt);
+	/* READ_ONCE synchronizes with smp_store_release */
+	ldt = READ_ONCE(mm->context.ldt);
 
 	/*
 	 * Any change to mm->context.ldt is followed by an IPI to all
arch/x86/kernel/ldt.c +1 -1
index f0e64db..0a21390 100644
@@ -101,7 +101,7 @@ static void finalize_ldt_struct(struct ldt_struct *ldt)
 static void install_ldt(struct mm_struct *current_mm,
 			struct ldt_struct *ldt)
 {
-	/* Synchronizes with lockless_dereference in load_mm_ldt. */
+	/* Synchronizes with READ_ONCE in load_mm_ldt. */
 	smp_store_release(&current_mm->context.ldt, ldt);
 
 	/* Activate the LDT for all CPUs using current_mm. */
drivers/md/dm-mpath.c +10 -10
index 11f273d..3f88c9d 100644
@@ -366,7 +366,7 @@ static struct pgpath *choose_path_in_pg(struct multipath *m,
 
 	pgpath = path_to_pgpath(path);
 
-	if (unlikely(lockless_dereference(m->current_pg) != pg)) {
+	if (unlikely(READ_ONCE(m->current_pg) != pg)) {
 		/* Only update current_pgpath if pg changed */
 		spin_lock_irqsave(&m->lock, flags);
 		m->current_pgpath = pgpath;
@@ -390,7 +390,7 @@ static struct pgpath *choose_pgpath(struct multipath *m, size_t nr_bytes)
 	}
 
 	/* Were we instructed to switch PG? */
-	if (lockless_dereference(m->next_pg)) {
+	if (READ_ONCE(m->next_pg)) {
 		spin_lock_irqsave(&m->lock, flags);
 		pg = m->next_pg;
 		if (!pg) {
@@ -406,7 +406,7 @@ static struct pgpath *choose_pgpath(struct multipath *m, size_t nr_bytes)
 
 	/* Don't change PG until it has no remaining paths */
 check_current_pg:
-	pg = lockless_dereference(m->current_pg);
+	pg = READ_ONCE(m->current_pg);
 	if (pg) {
 		pgpath = choose_path_in_pg(m, pg, nr_bytes);
 		if (!IS_ERR_OR_NULL(pgpath))
@@ -473,7 +473,7 @@ static int multipath_clone_and_map(struct dm_target *ti, struct request *rq,
 	struct request *clone;
 
 	/* Do we need to select a new pgpath? */
-	pgpath = lockless_dereference(m->current_pgpath);
+	pgpath = READ_ONCE(m->current_pgpath);
 	if (!pgpath || !test_bit(MPATHF_QUEUE_IO, &m->flags))
 		pgpath = choose_pgpath(m, nr_bytes);
 
@@ -535,7 +535,7 @@ static int __multipath_map_bio(struct multipath *m, struct bio *bio, struct dm_m
 	bool queue_io;
 
 	/* Do we need to select a new pgpath? */
-	pgpath = lockless_dereference(m->current_pgpath);
+	pgpath = READ_ONCE(m->current_pgpath);
 	queue_io = test_bit(MPATHF_QUEUE_IO, &m->flags);
 	if (!pgpath || !queue_io)
 		pgpath = choose_pgpath(m, nr_bytes);
@@ -1804,7 +1804,7 @@ static int multipath_prepare_ioctl(struct dm_target *ti,
 	struct pgpath *current_pgpath;
 	int r;
 
-	current_pgpath = lockless_dereference(m->current_pgpath);
+	current_pgpath = READ_ONCE(m->current_pgpath);
 	if (!current_pgpath)
 		current_pgpath = choose_pgpath(m, 0);
 
@@ -1826,7 +1826,7 @@ static int multipath_prepare_ioctl(struct dm_target *ti,
 	}
 
 	if (r == -ENOTCONN) {
-		if (!lockless_dereference(m->current_pg)) {
+		if (!READ_ONCE(m->current_pg)) {
 			/* Path status changed, redo selection */
 			(void) choose_pgpath(m, 0);
 		}
@@ -1895,9 +1895,9 @@ static int multipath_busy(struct dm_target *ti)
 		return (m->queue_mode != DM_TYPE_MQ_REQUEST_BASED);
 
 	/* Guess which priority_group will be used at next mapping time */
-	pg = lockless_dereference(m->current_pg);
-	next_pg = lockless_dereference(m->next_pg);
-	if (unlikely(!lockless_dereference(m->current_pgpath) && next_pg))
+	pg = READ_ONCE(m->current_pg);
+	next_pg = READ_ONCE(m->next_pg);
+	if (unlikely(!READ_ONCE(m->current_pgpath) && next_pg))
 		pg = next_pg;
 
 	if (!pg) {
fs/dcache.c +2 -2
index f901413..34c852a 100644
@@ -231,7 +231,7 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 {
 	/*
 	 * Be careful about RCU walk racing with rename:
-	 * use 'lockless_dereference' to fetch the name pointer.
+	 * use 'READ_ONCE' to fetch the name pointer.
 	 *
 	 * NOTE! Even if a rename will mean that the length
 	 * was not loaded atomically, we don't care. The
@@ -245,7 +245,7 @@ static inline int dentry_cmp(const struct dentry *dentry, const unsigned char *c
 	 * early because the data cannot match (there can
 	 * be no NUL in the ct/tcount data)
 	 */
-	const unsigned char *cs = lockless_dereference(dentry->d_name.name);
+	const unsigned char *cs = READ_ONCE(dentry->d_name.name);
 
 	return dentry_string_cmp(cs, ct, tcount);
 }
fs/overlayfs/ovl_entry.h +1 -1
index 25d9b5a..36b49bd 100644
@@ -77,5 +77,5 @@ static inline struct ovl_inode *OVL_I(struct inode *inode)
 
 static inline struct dentry *ovl_upperdentry_dereference(struct ovl_inode *oi)
 {
-	return lockless_dereference(oi->__upperdentry);
+	return READ_ONCE(oi->__upperdentry);
 }
fs/overlayfs/readdir.c +1 -1
index 0f85ee9..c67a770 100644
@@ -754,7 +754,7 @@ static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end,
 	if (!od->is_upper && OVL_TYPE_UPPER(ovl_path_type(dentry))) {
 		struct inode *inode = file_inode(file);
 
-		realfile = lockless_dereference(od->upperfile);
+		realfile = READ_ONCE(od->upperfile);
 		if (!realfile) {
 			struct path upperpath;
 
include/linux/rculist.h +2 -2
index 2bea1d5..5ed091c 100644
@@ -274,7 +274,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
  * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
  */
 #define list_entry_rcu(ptr, type, member) \
-	container_of(lockless_dereference(ptr), type, member)
+	container_of(READ_ONCE(ptr), type, member)
 
 /*
  * Where are list_empty_rcu() and list_first_entry_rcu()?
@@ -367,7 +367,7 @@ static inline void list_splice_tail_init_rcu(struct list_head *list,
  * example is when items are added to the list, but never deleted.
  */
 #define list_entry_lockless(ptr, type, member) \
-	container_of((typeof(ptr))lockless_dereference(ptr), type, member)
+	container_of((typeof(ptr))READ_ONCE(ptr), type, member)
 
 /**
  * list_for_each_entry_lockless - iterate over rcu list of given type
include/linux/rcupdate.h +2 -2
index 1a9f70d..a6ddc42 100644
@@ -346,7 +346,7 @@ static inline void rcu_preempt_sleep_check(void) { }
 #define __rcu_dereference_check(p, c, space) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(*p) *________p1 = (typeof(*p) *__force)lockless_dereference(p); \
+	typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
 	rcu_dereference_sparse(p, space); \
 	((typeof(*p) __force __kernel *)(________p1)); \
@@ -360,7 +360,7 @@ static inline void rcu_preempt_sleep_check(void) { }
 #define rcu_dereference_raw(p) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(p) ________p1 = lockless_dereference(p); \
+	typeof(p) ________p1 = READ_ONCE(p); \
 	((typeof(*p) __force __kernel *)(________p1)); \
 })
 
kernel/events/core.c +2 -2
index 9d93db8..824a583 100644
@@ -4231,7 +4231,7 @@ static void perf_remove_from_owner(struct perf_event *event)
 	 * indeed free this event, otherwise we need to serialize on
 	 * owner->perf_event_mutex.
 	 */
-	owner = lockless_dereference(event->owner);
+	owner = READ_ONCE(event->owner);
 	if (owner) {
 		/*
 		 * Since delayed_put_task_struct() also drops the last
@@ -4328,7 +4328,7 @@ again:
 		 * Cannot change, child events are not migrated, see the
 		 * comment with perf_event_ctx_lock_nested().
 		 */
-		ctx = lockless_dereference(child->ctx);
+		ctx = READ_ONCE(child->ctx);
 		/*
 		 * Since child_mutex nests inside ctx::mutex, we must jump
 		 * through hoops. We start by grabbing a reference on the ctx.
kernel/seccomp.c +1 -1
index 0ae832e..8ac7935 100644
@@ -189,7 +189,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
 	u32 ret = SECCOMP_RET_ALLOW;
 	/* Make sure cross-thread synced filter points somewhere sane. */
 	struct seccomp_filter *f =
-			lockless_dereference(current->seccomp.filter);
+			READ_ONCE(current->seccomp.filter);
 
 	/* Ensure unexpected behavior doesn't result in failing open. */
 	if (unlikely(WARN_ON(f == NULL)))
kernel/task_work.c +1 -1
index 836a72a..9a9f262 100644
@@ -67,7 +67,7 @@ task_work_cancel(struct task_struct *task, task_work_func_t func)
 	 * we raced with task_work_run(), *pprev == NULL/exited.
 	 */
 	raw_spin_lock_irqsave(&task->pi_lock, flags);
-	while ((work = lockless_dereference(*pprev))) {
+	while ((work = READ_ONCE(*pprev))) {
 		if (work->func != func)
 			pprev = &work->next;
 		else if (cmpxchg(pprev, work, work->next) == work)
mm/slab.h +1 -1
index 0733628..8894f81 100644
@@ -258,7 +258,7 @@ cache_from_memcg_idx(struct kmem_cache *s, int idx)
 	 * memcg_caches issues a write barrier to match this (see
 	 * memcg_create_kmem_cache()).
 	 */
-	cachep = lockless_dereference(arr->entries[idx]);
+	cachep = READ_ONCE(arr->entries[idx]);
 	rcu_read_unlock();
 
 	return cachep;
#7 lore raw

PATCH v2 4/5 locking/barriers: Kill lockless_dereference()

In reply to Will Deacon

lockless_dereference() is a nice idea, but it gained little traction in kernel code since its introduction three years ago. This is partly because it's a pain to type, but also because using READ_ONCE() instead has worked correctly on all architectures apart from Alpha, which is a fully supported but somewhat niche architecture these days.

Now that READ_ONCE() has been upgraded to contain an implicit smp_read_barrier_depends() and the few callers of lockless_dereference() have been converted, we can remove lockless_dereference() altogether.

Cc Ingo Molnar <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 Documentation/memory-barriers.txt                    | 12 ------------
 Documentation/translations/ko_KR/memory-barriers.txt | 12 ------------
 include/linux/compiler.h                             | 20 --------------------
 3 files changed, 44 deletions(-)
Documentation/memory-barriers.txt +0 -12
index b759a60624fd..470a682f3fa4 100644
@@ -1886,18 +1886,6 @@ There are some more advanced barrier functions:
      See Documentation/atomic_{t,bitops}.txt for more information.
 
 
- (*) lockless_dereference();
-
-     This can be thought of as a pointer-fetch wrapper around the
-     smp_read_barrier_depends() data-dependency barrier.
-
-     This is also similar to rcu_dereference(), but in cases where
-     object lifetime is handled by some mechanism other than RCU, for
-     example, when the objects removed only when the system goes down.
-     In addition, lockless_dereference() is used in some data structures
-     that can be used both with and without RCU.
-
-
  (*) dma_wmb();
  (*) dma_rmb();
 
Documentation/translations/ko_KR/memory-barriers.txt +0 -12
index a7a813258013..ec3b46e27b7a 100644
@@ -1858,18 +1858,6 @@ Mandatory 배리어들은 SMP 시스템에서도 UP 시스템에서도 SMP 효
      참고하세요.
 
 
- (*) lockless_dereference();
-
-     이 함수는 smp_read_barrier_depends() 데이터 의존성 배리어를 사용하는
-     포인터 읽어오기 래퍼(wrapper) 함수로 생각될 수 있습니다.
-
-     객체의 라이프타임이 RCU 외의 메커니즘으로 관리된다는 점을 제외하면
-     rcu_dereference() 와도 유사한데, 예를 들면 객체가 시스템이 꺼질 때에만
-     제거되는 경우 등입니다.  또한, lockless_dereference() 은 RCU 와 함께
-     사용될수도, RCU 없이 사용될 수도 있는 일부 데이터 구조에 사용되고
-     있습니다.
-
-
  (*) dma_wmb();
  (*) dma_rmb();
 
include/linux/compiler.h +0 -20
index 7d7b77da9716..5a1cab48442c 100644
@@ -346,24 +346,4 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 	(volatile typeof(x) *)&(x); })
 #define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
 
-/**
- * lockless_dereference() - safely load a pointer for later dereference
- * @p: The pointer to load
- *
- * 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 variable ___typecheck_p validates that @p is
- * indeed a pointer type by using a pointer to typeof(*p) as the type.
- * Taking a pointer to typeof(*p) again is needed in case p is void *.
- */
-#define lockless_dereference(p) \
-({ \
-	typeof(p) _________p1 = READ_ONCE(p); \
-	typeof(*(p)) *___typecheck_p __maybe_unused; \
-	smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
-	(_________p1); \
-})
-
 #endif /* __LINUX_COMPILER_H */
Signature · 1 line
2.1.4
#8 lore raw

[tip:locking/core] locking/barriers: Kill lockless_dereference()

locking/barriers: Kill lockless_dereference()
Commit-ID:  59ecbbe7b31cd2d86ff9a9f461a00f7e7533aedc
Gitweb:     https://git.kernel.org/tip/59ecbbe7b31cd2d86ff9a9f461a00f7e7533aedc
Author:     Will Deacon <[email protected]>
AuthorDate: Tue, 24 Oct 2017 11:22:49 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Oct 2017 13:17:33 +0200

locking/barriers: Kill lockless_dereference()

lockless_dereference() is a nice idea, but it gained little traction in kernel code since its introduction three years ago. This is partly because it's a pain to type, but also because using READ_ONCE() instead has worked correctly on all architectures apart from Alpha, which is a fully supported but somewhat niche architecture these days.

Now that READ_ONCE() has been upgraded to contain an implicit smp_read_barrier_depends() and the few callers of lockless_dereference() have been converted, we can remove lockless_dereference() altogether.

---
 Documentation/memory-barriers.txt                    | 12 ------------
 Documentation/translations/ko_KR/memory-barriers.txt | 12 ------------
 include/linux/compiler.h                             | 20 --------------------
 3 files changed, 44 deletions(-)
Documentation/memory-barriers.txt +0 -12
index b759a60..470a682 100644
@@ -1886,18 +1886,6 @@ There are some more advanced barrier functions:
      See Documentation/atomic_{t,bitops}.txt for more information.
 
 
- (*) lockless_dereference();
-
-     This can be thought of as a pointer-fetch wrapper around the
-     smp_read_barrier_depends() data-dependency barrier.
-
-     This is also similar to rcu_dereference(), but in cases where
-     object lifetime is handled by some mechanism other than RCU, for
-     example, when the objects removed only when the system goes down.
-     In addition, lockless_dereference() is used in some data structures
-     that can be used both with and without RCU.
-
-
  (*) dma_wmb();
  (*) dma_rmb();
 
Documentation/translations/ko_KR/memory-barriers.txt +0 -12
index a7a8132..ec3b46e 100644
@@ -1858,18 +1858,6 @@ Mandatory 배리어들은 SMP 시스템에서도 UP 시스템에서도 SMP 효
      참고하세요.
 
 
- (*) lockless_dereference();
-
-     이 함수는 smp_read_barrier_depends() 데이터 의존성 배리어를 사용하는
-     포인터 읽어오기 래퍼(wrapper) 함수로 생각될 수 있습니다.
-
-     객체의 라이프타임이 RCU 외의 메커니즘으로 관리된다는 점을 제외하면
-     rcu_dereference() 와도 유사한데, 예를 들면 객체가 시스템이 꺼질 때에만
-     제거되는 경우 등입니다.  또한, lockless_dereference() 은 RCU 와 함께
-     사용될수도, RCU 없이 사용될 수도 있는 일부 데이터 구조에 사용되고
-     있습니다.
-
-
  (*) dma_wmb();
  (*) dma_rmb();
 
include/linux/compiler.h +0 -20
index 7d7b77d..5a1cab4 100644
@@ -346,24 +346,4 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 	(volatile typeof(x) *)&(x); })
 #define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
 
-/**
- * lockless_dereference() - safely load a pointer for later dereference
- * @p: The pointer to load
- *
- * 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 variable ___typecheck_p validates that @p is
- * indeed a pointer type by using a pointer to typeof(*p) as the type.
- * Taking a pointer to typeof(*p) again is needed in case p is void *.
- */
-#define lockless_dereference(p) \
-({ \
-	typeof(p) _________p1 = READ_ONCE(p); \
-	typeof(*(p)) *___typecheck_p __maybe_unused; \
-	smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
-	(_________p1); \
-})
-
 #endif /* __LINUX_COMPILER_H */
#9 lore raw

PATCH v2 5/5 alpha: atomics: Add smp_read_barrier_depends() to release/relaxed atomics

In reply to Will Deacon

As part of the fight against smp_read_barrier_depends(), we require dependency ordering to be preserved when a dependency is headed by a load performed using an atomic operation.

This patch adds smp_read_barrier_depends() to the _release and _relaxed atomics on alpha, which otherwise lack anything to enforce dependency ordering.

---
 arch/alpha/include/asm/atomic.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/alpha/include/asm/atomic.h +13 -0
index 498933a7df97..16961a3f45ba 100644
@@ -13,6 +13,15 @@
  * than regular operations.
  */
 
+/*
+ * To ensure dependency ordering is preserved for the _relaxed and
+ * _release atomics, an smp_read_barrier_depends() is unconditionally
+ * inserted into the _relaxed variants, which are used to build the
+ * barriered versions. To avoid redundant back-to-back fences, we can
+ * define the _acquire and _fence versions explicitly.
+ */
+#define __atomic_op_acquire(op, args...)	op##_relaxed(args)
+#define __atomic_op_fence			__atomic_op_release
 
 #define ATOMIC_INIT(i)		{ (i) }
 #define ATOMIC64_INIT(i)	{ (i) }
@@ -60,6 +69,7 @@ static inline int atomic_##op##_return_relaxed(int i, atomic_t *v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -77,6 +87,7 @@ static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -111,6 +122,7 @@ static __inline__ long atomic64_##op##_return_relaxed(long i, atomic64_t * v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -128,6 +140,7 @@ static __inline__ long atomic64_fetch_##op##_relaxed(long i, atomic64_t * v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
Signature · 1 line
2.1.4
#10 lore raw

[tip:locking/core] locking/atomics/alpha: Add smp_read_barrier_depends() to _release()/_relaxed() atomics

locking/atomics/alpha: Add smp_read_barrier_depends() to _release()/_relaxed() atomics
Commit-ID:  5a8897cc7631fa544d079c443800f4420d1b173f
Gitweb:     https://git.kernel.org/tip/5a8897cc7631fa544d079c443800f4420d1b173f
Author:     Will Deacon <[email protected]>
AuthorDate: Tue, 24 Oct 2017 11:22:50 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 Oct 2017 13:17:34 +0200

locking/atomics/alpha: Add smp_read_barrier_depends() to _release()/_relaxed() atomics

As part of the fight against smp_read_barrier_depends(), we require dependency ordering to be preserved when a dependency is headed by a load performed using an atomic operation.

This patch adds smp_read_barrier_depends() to the _release() and _relaxed() atomics on alpha, which otherwise lack anything to enforce dependency ordering.

---
 arch/alpha/include/asm/atomic.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/alpha/include/asm/atomic.h +13 -0
index 498933a..16961a3 100644
@@ -13,6 +13,15 @@
  * than regular operations.
  */
 
+/*
+ * To ensure dependency ordering is preserved for the _relaxed and
+ * _release atomics, an smp_read_barrier_depends() is unconditionally
+ * inserted into the _relaxed variants, which are used to build the
+ * barriered versions. To avoid redundant back-to-back fences, we can
+ * define the _acquire and _fence versions explicitly.
+ */
+#define __atomic_op_acquire(op, args...)	op##_relaxed(args)
+#define __atomic_op_fence			__atomic_op_release
 
 #define ATOMIC_INIT(i)		{ (i) }
 #define ATOMIC64_INIT(i)	{ (i) }
@@ -60,6 +69,7 @@ static inline int atomic_##op##_return_relaxed(int i, atomic_t *v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -77,6 +87,7 @@ static inline int atomic_fetch_##op##_relaxed(int i, atomic_t *v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -111,6 +122,7 @@ static __inline__ long atomic64_##op##_return_relaxed(long i, atomic64_t * v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }
 
@@ -128,6 +140,7 @@ static __inline__ long atomic64_fetch_##op##_relaxed(long i, atomic64_t * v)	\
 	".previous"							\
 	:"=&r" (temp), "=m" (v->counter), "=&r" (result)		\
 	:"Ir" (i), "m" (v->counter) : "memory");			\
+	smp_read_barrier_depends();					\
 	return result;							\
 }