introduce sys_membarrier(): process-wide memory barrier (v6)

RFC PATCH In review Mathieu Desnoyers 2 participants 4 messages Jan 15, 2010 – Jan 16, 2010 x86

Share & tools

Mathieu Desnoyers

author

2010-01-15 18:29 UTC #0 lore raw

RFC PATCH 1/1 introduce sys_membarrier(): process-wide memory barrier (v6)

Here is an implementation of a new system call, sys_membarrier(), which executes a memory barrier on all threads of the current process.

It aims at greatly simplifying and enhancing the current signal-based liburcu userspace RCU synchronize_rcu() implementation. (found at http://lttng.org/urcu)

Changelog since v1:

- Only perform the IPI in CONFIG_SMP.
- Only perform the IPI if the process has more than one thread.
- Only send IPIs to CPUs involved with threads belonging to our process.
- Adaptative IPI scheme (single vs many IPI with threshold).
- Issue smp_mb() at the beginning and end of the system call.

Changelog since v2:
- simply send-to-many to the mm_cpumask. It contains the list of processors we have to IPI to (which use the mm), and this mask is updated atomically.

Changelog since v3a:
- Confirm that each CPU indeed runs the current task's ->mm before sending an IPI. Ensures that we do not disturb RT tasks in the presence of lazy TLB shootdown.
- Document memory barriers needed in switch_mm().
- Surround helper functions with #ifdef CONFIG_SMP.

Changelog since v4:
- Add "int expedited" parameter, use synchronize_sched() in the non-expedited case. Thanks to Lai Jiangshan for making us consider seriously using synchronize_sched() to provide the low-overhead membarrier scheme.
- Check num_online_cpus() == 1, quickly return without doing nothing.

Changelog since v5:
- Plan ahead for extensibility by introducing mandatory/optional masks to the "flags" system call parameter. Past experience with accept4(), signalfd4(), eventfd2(), epoll_create1(), dup3(), pipe2(), and inotify_init1() indicates that this is the kind of thing we want to plan for. Return -EINVAL if the mandatory flags received are unknown.
- Create include/linux/membarrier.h to define these flags.
- Add MEMBARRIER_QUERY optional flag.

Both the signal-based and the sys_membarrier userspace RCU schemes permit us to remove the memory barrier from the userspace RCU rcu_read_lock() and rcu_read_unlock() primitives, thus significantly accelerating them. These memory barriers are replaced by compiler barriers on the read-side, and all matching memory barriers on the write-side are turned into an invokation of a memory barrier on all active threads in the process. By letting the kernel perform this synchronization rather than dumbly sending a signal to every process threads (as we currently do), we diminish the number of unnecessary wake ups and only issue the memory barriers on active threads. Non-running threads do not need to execute such barrier anyway, because these are implied by the scheduler context switches.

To explain the benefit of this scheme, let's introduce two example threads:

Thread A (non-frequent, e.g. executing liburcu synchronize_rcu()) Thread B (frequent, e.g. executing liburcu rcu_read_lock()/rcu_read_unlock())

In a scheme where all smp_mb() in thread A synchronize_rcu() are ordering memory accesses with respect to smp_mb() present in rcu_read_lock/unlock(), we can change all smp_mb() from synchronize_rcu() into calls to sys_membarrier() and all smp_mb() from rcu_read_lock/unlock() into compiler barriers "barrier()".

Before the change, we had, for each smp_mb() pairs:

Thread A                    Thread B
prev mem accesses           prev mem accesses
smp_mb()                    smp_mb()
follow mem accesses         follow mem accesses

After the change, these pairs become:

Thread A                    Thread B
prev mem accesses           prev mem accesses
sys_membarrier()            barrier()
follow mem accesses         follow mem accesses

As we can see, there are two possible scenarios: either Thread B memory accesses do not happen concurrently with Thread A accesses (1), or they do (2).

1) Non-concurrent Thread A vs Thread B accesses:

Thread A Thread B
prev mem accesses
sys_membarrier()
follow mem accesses
prev mem accesses
barrier()
follow mem accesses

In this case, thread B accesses will be weakly ordered. This is OK, because at that point, thread A is not particularly interested in ordering them with respect to its own accesses.

2) Concurrent Thread A vs Thread B accesses

Thread A                    Thread B
prev mem accesses           prev mem accesses
sys_membarrier()            barrier()
follow mem accesses         follow mem accesses

In this case, thread B accesses, which are ensured to be in program order thanks to the compiler barrier, will be "upgraded" to full smp_mb() thanks to the IPIs executing memory barriers on each active system threads. Each non-running process threads are intrinsically serialized by the scheduler.

For my Intel Xeon E5405
(one thread is doing the sys_membarrier, the others are busy looping)

* expedited

10,000,000 sys_membarrier calls:

T=1: 0m20.173s
T=2: 0m20.506s
T=3: 0m22.632s
T=4: 0m24.759s
T=5: 0m26.633s
T=6: 0m29.654s
T=7: 0m30.669s

For a 2-3 microseconds/call.

* non-expedited

1000 sys_membarrier calls:

T=1-7: 0m16.002s

For a 16 milliseconds/call. (~5000-8000 times slower than expedited)

The expected top pattern for the expedited scheme, when using 1 CPU for a thread doing sys_membarrier() in a loop and other 7 threads busy-waiting in user-space on a variable shows that the thread doing sys_membarrier is doing mostly system calls, and other threads are mostly running in user-space. Note that IPI handlers are not taken into account in the cpu time sampling.

Cpu0  :100.0%us,  0.0%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Cpu1  : 99.7%us,  0.0%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.3%hi,  0.0%si,  0.0%st
Cpu2  : 99.3%us,  0.0%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.7%hi,  0.0%si,  0.0%st
Cpu3  :100.0%us,  0.0%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Cpu4  :100.0%us,  0.0%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Cpu5  : 96.0%us,  1.3%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  2.6%si,  0.0%st
Cpu6  :  1.3%us, 98.7%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Cpu7  : 96.1%us,  3.3%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.3%hi,  0.3%si,  0.0%st

Results in liburcu:

Operations in 10s, 6 readers, 2 writers:

(what we previously had)
memory barriers in reader: 973494744 reads, 892368 writes
signal-based scheme: 6289946025 reads, 1251 writes

(what we have now, with dynamic sys_membarrier check, expedited scheme) memory barriers in reader: 907693804 reads, 817793 writes
sys_membarrier scheme: 4316818891 reads, 503790 writes

(dynamic sys_membarrier check, non-expedited scheme) memory barriers in reader: 907693804 reads, 817793 writes
sys_membarrier scheme: 8698725501 reads, 313 writes

So the dynamic sys_membarrier availability check adds some overhead to the read-side, but besides that, with the expedited scheme, we can see that we are close to the read-side performance of the signal-based scheme and also close (5/8) to the performance of the memory-barrier write-side. We have a write-side speedup of 400:1 over the signal-based scheme by using the sys_membarrier system call. This allows a 4.5:1 read-side speedup over the memory barrier scheme.

The non-expedited scheme adds indeed a much lower overhead on the read-side both because we do not send IPIs and because we perform less updates, which in turn generates less cache-line exchanges. The write-side latency becomes even higher than with the signal-based scheme. The advantage of the non-expedited sys_membarrier() scheme over signal-based scheme is that it does not require to wake up all the process threads.

The system call number is only assigned for x86_64 in this RFC patch. Note that switch_mm() memory barrier audit is required for each architecture before assigning a system call number.

Signed-off-by Mathieu Desnoyers <[email protected]>

Acked-by KOSAKI Motohiro <[email protected]>

CC "Paul E. McKenney" <[email protected]>

CC Nicholas Miell <[email protected]>

CC [email protected]

---
 arch/x86/include/asm/mmu_context.h |   18 +++-
 arch/x86/include/asm/unistd_64.h   |    2 
 include/linux/Kbuild               |    1 
 include/linux/membarrier.h         |   25 +++++
 kernel/sched.c                     |  160 +++++++++++++++++++++++++++++++++++++
 5 files changed, 204 insertions(+), 2 deletions(-)

arch/x86/include/asm/unistd_64.h +2 -0

Index: linux-2.6-lttng/arch/x86/include/asm/unistd_64.h
===================================================================
@@ -661,6 +661,8 @@ __SYSCALL(__NR_pwritev, sys_pwritev)
 __SYSCALL(__NR_rt_tgsigqueueinfo, sys_rt_tgsigqueueinfo)
 #define __NR_perf_event_open			298
 __SYSCALL(__NR_perf_event_open, sys_perf_event_open)
+#define __NR_membarrier				299
+__SYSCALL(__NR_membarrier, sys_membarrier)
 
 #ifndef __NO_STUBS
 #define __ARCH_WANT_OLD_READDIR

kernel/sched.c +160 -0

172 lines not on this page · raw message

arch/x86/include/asm/mmu_context.h +16 -2

Index: linux-2.6-lttng/arch/x86/include/asm/mmu_context.h
===================================================================
@@ -36,6 +36,11 @@ static inline void switch_mm(struct mm_s
 	unsigned cpu = smp_processor_id();
 
 	if (likely(prev != next)) {
+		/*
+		 * smp_mb() between user-space thread execution and
+		 * mm_cpumask clear is required by sys_membarrier().
+		 */
+		smp_mb__before_clear_bit();
 		/* stop flush ipis for the previous mm */
 		cpumask_clear_cpu(cpu, mm_cpumask(prev));
 #ifdef CONFIG_SMP

@@ -43,7 +48,11 @@ static inline void switch_mm(struct mm_s
 		percpu_write(cpu_tlbstate.active_mm, next);
 #endif
 		cpumask_set_cpu(cpu, mm_cpumask(next));
-
+		/*
+		 * smp_mb() between mm_cpumask set and user-space thread
+		 * execution is required by sys_membarrier(). Implied by
+		 * load_cr3.
+		 */
 		/* Re-load page tables */
 		load_cr3(next->pgd);

@@ -59,9 +68,14 @@ static inline void switch_mm(struct mm_s
 		BUG_ON(percpu_read(cpu_tlbstate.active_mm) != next);
 
 		if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next))) {
-			/* We were in lazy tlb mode and leave_mm disabled
+			/*
+			 * We were in lazy tlb mode and leave_mm disabled
 			 * tlb flush IPI delivery. We must reload CR3
 			 * to make sure to use no freed page tables.
+			 *
+			 * smp_mb() between mm_cpumask set and user-space
+			 * thread execution is required by sys_membarrier().
+			 * Implied by load_cr3.
 			 */
 			load_cr3(next->pgd);
 			load_LDT_nolock(&next->context);

include/linux/membarrier.h new file +25 -0

Index: linux-2.6-lttng/include/linux/membarrier.h
===================================================================
@@ -0,0 +1,25 @@
+#ifndef _LINUX_MEMBARRIER_H
+#define _LINUX_MEMBARRIER_H
+
+/* First argument to membarrier syscall */
+
+/*
+ * Mandatory flags to the membarrier system call that the kernel must
+ * understand are in the low 16 bits.
+ */
+#define MEMBARRIER_MANDATORY_MASK	0x0000FFFF	/* Mandatory flags */
+
+/*
+ * Optional hints that the kernel can ignore are in the high 16 bits.
+ */
+#define MEMBARRIER_OPTIONAL_MASK	0xFFFF0000	/* Optional hints */
+
+/* Expedited: adds some overhead, fast execution (few microseconds) */
+#define MEMBARRIER_EXPEDITED		(1 << 0)
+/* Delayed: Low overhead, but slow execution (few milliseconds) */
+#define MEMBARRIER_DELAYED		(1 << 1)
+
+/* Query flag support, without performing synchronization */
+#define MEMBARRIER_QUERY		(1 << 16)
+
+#endif

include/linux/Kbuild +1 -0

Index: linux-2.6-lttng/include/linux/Kbuild
===================================================================
@@ -110,6 +110,7 @@ header-y += magic.h
 header-y += major.h
 header-y += map_to_7segment.h
 header-y += matroxfb.h
+header-y += membarrier.h
 header-y += meye.h
 header-y += minix_fs.h
 header-y += mmtimer.h

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Steven Rostedt

Reviewed-by on v1 1/1

2010-01-15 19:58 UTC #1 lore raw

Quoted 11 lines · Mathieu Desnoyers

kernel/sched.c +11 -0

+SYSCALL_DEFINE1(membarrier, unsigned int, flags)
+{
+#ifdef CONFIG_SMP
+	cpumask_var_t tmpmask;
+	struct mm_struct *mm;
+	int cpu;
+
+	/*
+	 * Expect _only_ one of expedited or delayed flags.
+	 * Don't care about optional mask for now.
+	 */

kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10945

Quoted 10 lines

kernel/sched.c +10 -0

+	switch(flags & MEMBARRIER_MANDATORY_MASK) {
+	case MEMBARRIER_EXPEDITED:
+	case MEMBARRIER_DELAYED:
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (unlikely(flags & MEMBARRIER_QUERY
+		     || thread_group_empty(current)
+		     || (num_online_cpus() == 1)))

I really hate the overuse of unlikely in the kernel. Unless it is 99.9% false, it should not be used. My branch analyzer shows lots of areas in the kernel that need this fixed.

For the check of MEMBARRIER_QUERY and thread_group_empty(), sure that could be 99.9% true. But for num_online_cpus() == 1, I can see that always being true. How may boxes do you have that are uniprocessor that run without CONFIG_SMP=y? Every distro I know enables this, only embedded boards seem not to. So you just made the 100% true condition unlikely on those boards. Grant it, this may not be such a big deal, but I like to shed light on the overuse of unlikely. I don't think this unlikely is worth it.

Quoted 4 lines

kernel/sched.c +4 -0

+		return 0;
+	if (flags & MEMBARRIER_DELAYED) {
+		synchronize_sched();
+		return 0;

kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10959

Quoted 10 lines

kernel/sched.c +10 -0

+	}
+	/*
+	 * Memory barrier on the caller thread _before_ sending first
+	 * IPI. Matches memory barriers around mm_cpumask modification in
+	 * switch_mm().
+	 */
+	smp_mb();
+	if (!alloc_cpumask_var(&tmpmask, GFP_NOWAIT)) {
+		membarrier_retry();
+		goto unlock;

This unlock label is misleading, I had to go and see what it unlocked, it which it did not unlock anything. Change it to "out".

Quoted 5 lines

kernel/sched.c +5 -0

+	}
+	cpumask_copy(tmpmask, mm_cpumask(current->mm));
+	preempt_disable();
+	cpumask_clear_cpu(smp_processor_id(), tmpmask);
+	for_each_cpu(cpu, tmpmask) {

kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10974

Quoted 10 lines

kernel/sched.c +10 -0

+		spin_lock_irq(&cpu_rq(cpu)->lock);
+		mm = cpu_curr(cpu)->mm;
+		spin_unlock_irq(&cpu_rq(cpu)->lock);
+		if (current->mm != mm)
+			cpumask_clear_cpu(cpu, tmpmask);
+	}
+	smp_call_function_many(tmpmask, membarrier_ipi, NULL, 1);
+	preempt_enable();
+	free_cpumask_var(tmpmask);
+unlock:

rename to "out:"

Quoted 18 lines

kernel/sched.c +10 -0

+	/*
+	 * Memory barrier on the caller thread _after_ we finished
+	 * waiting for the last IPI. Matches memory barriers around mm_cpumask
+	 * modification in switch_mm().
+	 */
+	smp_mb();
+#endif /* #ifdef CONFIG_SMP */
+	return 0;
+}
+
 #ifndef CONFIG_SMP
 
 int rcu_expedited_torture_stats(char *page)

Index: linux-2.6-lttng/arch/x86/include/asm/mmu_context.h
===================================================================

arch/x86/include/asm/mmu_context.h static inline void switch_mm(struct mm_s · line 41

Quoted 6 lines

arch/x86/include/asm/mmu_context.h +3 -0

@@ -36,6 +36,11 @@ static inline void switch_mm(struct mm_s
 	unsigned cpu = smp_processor_id();
 
 	if (likely(prev != next)) {
+		/*
+		 * smp_mb() between user-space thread execution and
+		 * mm_cpumask clear is required by sys_membarrier().

Of course on x86, this just turns into a barrier();, perhaps the comment could state, that the use of smp_mb__before_clear_bit() is more of an example for other archs since x86 only requires a barrier().

Quoted 70 lines

arch/x86/include/asm/mmu_context.h +13 -2

+		 */
+		smp_mb__before_clear_bit();
 		/* stop flush ipis for the previous mm */
 		cpumask_clear_cpu(cpu, mm_cpumask(prev));
 #ifdef CONFIG_SMP

@@ -43,7 +48,11 @@ static inline void switch_mm(struct mm_s
 		percpu_write(cpu_tlbstate.active_mm, next);
 #endif
 		cpumask_set_cpu(cpu, mm_cpumask(next));
-
+		/*
+		 * smp_mb() between mm_cpumask set and user-space thread
+		 * execution is required by sys_membarrier(). Implied by
+		 * load_cr3.
+		 */
 		/* Re-load page tables */
 		load_cr3(next->pgd);

@@ -59,9 +68,14 @@ static inline void switch_mm(struct mm_s
 		BUG_ON(percpu_read(cpu_tlbstate.active_mm) != next);
 
 		if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next))) {
-			/* We were in lazy tlb mode and leave_mm disabled
+			/*
+			 * We were in lazy tlb mode and leave_mm disabled
 			 * tlb flush IPI delivery. We must reload CR3
 			 * to make sure to use no freed page tables.
+			 *
+			 * smp_mb() between mm_cpumask set and user-space
+			 * thread execution is required by sys_membarrier().
+			 * Implied by load_cr3.
 			 */
 			load_cr3(next->pgd);
 			load_LDT_nolock(&next->context);

Index: linux-2.6-lttng/include/linux/membarrier.h
===================================================================

include/linux/membarrier.h new file +25 -0

@@ -0,0 +1,25 @@
+#ifndef _LINUX_MEMBARRIER_H
+#define _LINUX_MEMBARRIER_H
+
+/* First argument to membarrier syscall */
+
+/*
+ * Mandatory flags to the membarrier system call that the kernel must
+ * understand are in the low 16 bits.
+ */
+#define MEMBARRIER_MANDATORY_MASK	0x0000FFFF	/* Mandatory flags */
+
+/*
+ * Optional hints that the kernel can ignore are in the high 16 bits.
+ */
+#define MEMBARRIER_OPTIONAL_MASK	0xFFFF0000	/* Optional hints */
+
+/* Expedited: adds some overhead, fast execution (few microseconds) */
+#define MEMBARRIER_EXPEDITED		(1 << 0)
+/* Delayed: Low overhead, but slow execution (few milliseconds) */
+#define MEMBARRIER_DELAYED		(1 << 1)
+
+/* Query flag support, without performing synchronization */
+#define MEMBARRIER_QUERY		(1 << 16)
+
+#endif

Index: linux-2.6-lttng/include/linux/Kbuild
===================================================================

include/linux/Kbuild header-y += magic.h · line 116

Quoted 7 lines

include/linux/Kbuild +1 -0

@@ -110,6 +110,7 @@ header-y += magic.h
 header-y += major.h
 header-y += map_to_7segment.h
 header-y += matroxfb.h
+header-y += membarrier.h
 header-y += meye.h
 header-y += minix_fs.h
 header-y += mmtimer.h

The rest:

Reviewed-by Steven Rostedt <[email protected]>

-- Steve

Mathieu Desnoyers

author

2010-01-15 21:13 UTC #2 lore raw

Quoted 28 lines · * Steven Rostedt ([email protected]) wrote:

Quoted 11 lines · Mathieu Desnoyers
kernel/sched.c +11 -0
+SYSCALL_DEFINE1(membarrier, unsigned int, flags)
+{
+#ifdef CONFIG_SMP
+	cpumask_var_t tmpmask;
+	struct mm_struct *mm;
+	int cpu;
+
+	/*
+	 * Expect _only_ one of expedited or delayed flags.
+	 * Don't care about optional mask for now.
+	 */
kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10945
Quoted 10 lines
kernel/sched.c +10 -0
+	switch(flags & MEMBARRIER_MANDATORY_MASK) {
+	case MEMBARRIER_EXPEDITED:
+	case MEMBARRIER_DELAYED:
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (unlikely(flags & MEMBARRIER_QUERY
+		     || thread_group_empty(current)
+		     || (num_online_cpus() == 1)))
I really hate the overuse of unlikely in the kernel. Unless it is 99.9% false, it should not be used. My branch analyzer shows lots of areas in the kernel that need this fixed.

Quoted 8 lines

For the check of MEMBARRIER_QUERY and thread_group_empty(), sure that could be 99.9% true. But for num_online_cpus() == 1, I can see that always being true. How may boxes do you have that are uniprocessor that run without CONFIG_SMP=y? Every distro I know enables this, only embedded boards seem not to. So you just made the 100% true condition unlikely on those boards. Grant it, this may not be such a big deal, but I like to shed light on the overuse of unlikely. I don't think this unlikely is worth it.

Good point. So I'll remove the (num_online_cpus() == 1) from unlikely then, it will become:

        if (unlikely(flags & MEMBARRIER_QUERY
                     || thread_group_empty(current))
                     || num_online_cpus() == 1)

(removed extra () around (num_online_cpus() == 1) too)

Quoted 11 lines

kernel/sched.c +9 -0

+		return 0;
+	if (flags & MEMBARRIER_DELAYED) {
+		synchronize_sched();
+		return 0;
+	}
+	/*
+	 * Memory barrier on the caller thread _before_ sending first
+	 * IPI. Matches memory barriers around mm_cpumask modification in
+	 * switch_mm().

Quoted 9 lines

kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10959
Quoted 5 lines
kernel/sched.c +5 -0
+	 */
+	smp_mb();
+	if (!alloc_cpumask_var(&tmpmask, GFP_NOWAIT)) {
+		membarrier_retry();
+		goto unlock;
This unlock label is misleading, I had to go and see what it unlocked, it which it did not unlock anything. Change it to "out".

Quoted 9 lines

kernel/sched.c +9 -0

+	}
+	cpumask_copy(tmpmask, mm_cpumask(current->mm));
+	preempt_disable();
+	cpumask_clear_cpu(smp_processor_id(), tmpmask);
+	for_each_cpu(cpu, tmpmask) {
+		spin_lock_irq(&cpu_rq(cpu)->lock);
+		mm = cpu_curr(cpu)->mm;
+		spin_unlock_irq(&cpu_rq(cpu)->lock);
+		if (current->mm != mm)

Quoted 8 lines

kernel/sched.c struct cgroup_subsys cpuacct_subsys = { · line 10974
Quoted 6 lines
kernel/sched.c +6 -0
+			cpumask_clear_cpu(cpu, tmpmask);
+	}
+	smp_call_function_many(tmpmask, membarrier_ipi, NULL, 1);
+	preempt_enable();
+	free_cpumask_var(tmpmask);
+unlock:
rename to "out:"

Quoted 21 lines

kernel/sched.c +10 -0

+	/*
+	 * Memory barrier on the caller thread _after_ we finished
+	 * waiting for the last IPI. Matches memory barriers around mm_cpumask
+	 * modification in switch_mm().
+	 */
+	smp_mb();
+#endif /* #ifdef CONFIG_SMP */
+	return 0;
+}
+
 #ifndef CONFIG_SMP
 
 int rcu_expedited_torture_stats(char *page)

Index: linux-2.6-lttng/arch/x86/include/asm/mmu_context.h
===================================================================

arch/x86/include/asm/mmu_context.h +0 -0

@@ -36,6 +36,11 @@ static inline void switch_mm(struct mm_s
 	unsigned cpu = smp_processor_id();

Quoted 8 lines

arch/x86/include/asm/mmu_context.h static inline void switch_mm(struct mm_s · line 41
Quoted 4 lines
arch/x86/include/asm/mmu_context.h +3 -0
 	if (likely(prev != next)) {
+		/*
+		 * smp_mb() between user-space thread execution and
+		 * mm_cpumask clear is required by sys_membarrier().
Of course on x86, this just turns into a barrier();, perhaps the comment could state, that the use of smp_mb__before_clear_bit() is more of an example for other archs since x86 only requires a barrier().

Does something like this work ?

/*
* smp_mb() between user-space thread execution and
* mm_cpumask clear is required by sys_membarrier().
* smp_mb__before_clear_bit() turns into a barrier() on x86. It
* is left here to document that this barrier is needed, as an
* example for other architectures.
*/
smp_mb__before_clear_bit();

Quoted 81 lines

Quoted 71 lines

arch/x86/include/asm/mmu_context.h +13 -2

+		 */
+		smp_mb__before_clear_bit();
 		/* stop flush ipis for the previous mm */
 		cpumask_clear_cpu(cpu, mm_cpumask(prev));
 #ifdef CONFIG_SMP

@@ -43,7 +48,11 @@ static inline void switch_mm(struct mm_s
 		percpu_write(cpu_tlbstate.active_mm, next);
 #endif
 		cpumask_set_cpu(cpu, mm_cpumask(next));
-
+		/*
+		 * smp_mb() between mm_cpumask set and user-space thread
+		 * execution is required by sys_membarrier(). Implied by
+		 * load_cr3.
+		 */
 		/* Re-load page tables */
 		load_cr3(next->pgd);

@@ -59,9 +68,14 @@ static inline void switch_mm(struct mm_s
 		BUG_ON(percpu_read(cpu_tlbstate.active_mm) != next);
 
 		if (!cpumask_test_and_set_cpu(cpu, mm_cpumask(next))) {
-			/* We were in lazy tlb mode and leave_mm disabled
+			/*
+			 * We were in lazy tlb mode and leave_mm disabled
 			 * tlb flush IPI delivery. We must reload CR3
 			 * to make sure to use no freed page tables.
+			 *
+			 * smp_mb() between mm_cpumask set and user-space
+			 * thread execution is required by sys_membarrier().
+			 * Implied by load_cr3.
 			 */
 			load_cr3(next->pgd);
 			load_LDT_nolock(&next->context);

Index: linux-2.6-lttng/include/linux/membarrier.h
===================================================================

include/linux/membarrier.h new file +25 -0

@@ -0,0 +1,25 @@
+#ifndef _LINUX_MEMBARRIER_H
+#define _LINUX_MEMBARRIER_H
+
+/* First argument to membarrier syscall */
+
+/*
+ * Mandatory flags to the membarrier system call that the kernel must
+ * understand are in the low 16 bits.
+ */
+#define MEMBARRIER_MANDATORY_MASK	0x0000FFFF	/* Mandatory flags */
+
+/*
+ * Optional hints that the kernel can ignore are in the high 16 bits.
+ */
+#define MEMBARRIER_OPTIONAL_MASK	0xFFFF0000	/* Optional hints */
+
+/* Expedited: adds some overhead, fast execution (few microseconds) */
+#define MEMBARRIER_EXPEDITED		(1 << 0)
+/* Delayed: Low overhead, but slow execution (few milliseconds) */
+#define MEMBARRIER_DELAYED		(1 << 1)
+
+/* Query flag support, without performing synchronization */
+#define MEMBARRIER_QUERY		(1 << 16)
+
+#endif

Index: linux-2.6-lttng/include/linux/Kbuild
===================================================================

include/linux/Kbuild header-y += magic.h · line 116

Quoted 7 lines

include/linux/Kbuild +1 -0

@@ -110,6 +110,7 @@ header-y += magic.h
 header-y += major.h
 header-y += map_to_7segment.h
 header-y += matroxfb.h
+header-y += membarrier.h
 header-y += meye.h
 header-y += minix_fs.h
 header-y += mmtimer.h

The rest:

Quoted 1 line

Reviewed-by Steven Rostedt <[email protected]>

Thanks!

Mathieu

Quoted 4 lines

-- Steve

Signature · 2 lines

Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Steven Rostedt

2010-01-16 02:40 UTC #3 lore raw

Quoted 11 lines · Mathieu Desnoyers

Quoted 8 lines · * Steven Rostedt ([email protected]) wrote:
For the check of MEMBARRIER_QUERY and thread_group_empty(), sure that could be 99.9% true. But for num_online_cpus() == 1, I can see that always being true. How may boxes do you have that are uniprocessor that run without CONFIG_SMP=y? Every distro I know enables this, only embedded boards seem not to. So you just made the 100% true condition unlikely on those boards. Grant it, this may not be such a big deal, but I like to shed light on the overuse of unlikely. I don't think this unlikely is worth it.

Quoted 8 lines

Good point. So I'll remove the (num_online_cpus() == 1) from unlikely then, it will become:
        if (unlikely(flags & MEMBARRIER_QUERY
                     || thread_group_empty(current))
                     || num_online_cpus() == 1)
(removed extra () around (num_online_cpus() == 1) too)

I wonder if that unlikely even matters. It's not really that critical of a path. If anything it just documents what would be unlikely.

Honestly, I'd recommend getting rid of that unlikely altogether. But I'm fine with this version with or without it.

Quoted 19 lines

Quoted 8 lines
Quoted 8 lines
int rcu_expedited_torture_stats(char *page)
Index: linux-2.6-lttng/arch/x86/include/asm/mmu_context.h
===================================================================
arch/x86/include/asm/mmu_context.h +0 -0
@@ -36,6 +36,11 @@ static inline void switch_mm(struct mm_s
 	unsigned cpu = smp_processor_id();
 
Quoted 8 lines
arch/x86/include/asm/mmu_context.h static inline void switch_mm(struct mm_s · line 41
Quoted 4 lines
arch/x86/include/asm/mmu_context.h +3 -0
 	if (likely(prev != next)) {
+		/*
+		 * smp_mb() between user-space thread execution and
+		 * mm_cpumask clear is required by sys_membarrier().
Of course on x86, this just turns into a barrier();, perhaps the comment could state, that the use of smp_mb__before_clear_bit() is more of an example for other archs since x86 only requires a barrier().
Does something like this work ?

Quoted 9 lines

/*
* smp_mb() between user-space thread execution and
* mm_cpumask clear is required by sys_membarrier().
* smp_mb__before_clear_bit() turns into a barrier() on x86. It
* is left here to document that this barrier is needed, as an
* example for other architectures.
*/
smp_mb__before_clear_bit();

That's fine with me.

-- Steve