Related non-RCU updates

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

Signed-off-by Paul E. McKenney <[email protected]>

---
 include/uapi/linux/membarrier.h | 11 +++++++++++
 kernel/membarrier.c             |  7 ++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

include/uapi/linux/membarrier.h +10 -0

index e0b108bd2624..ba36d8a6be61 100644
@@ -40,6 +40,16 @@
  *                          (non-running threads are de facto in such a
  *                          state). This covers threads from all processes
  *                          running on the system. This command returns 0.
+ * @MEMBARRIER_CMD_SHARED_EXPEDITED:  Execute a memory barrier on all
+ *			    running threads, but in an expedited fashion.
+ *                          Upon return from system call, the caller thread
+ *                          is ensured that all running threads have passed
+ *                          through a state where all memory accesses to
+ *                          user-space addresses match program order between
+ *                          entry to and return from the system call
+ *                          (non-running threads are de facto in such a
+ *                          state). This covers threads from all processes
+ *                          running on the system. This command returns 0.
  *
  * Command to be passed to the membarrier system call. The commands need to
  * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to

include/uapi/linux/membarrier.h +0 -0

 };
 
 #endif /* _UAPI_LINUX_MEMBARRIER_H */

kernel/membarrier.c +6 -1

index 9f9284f37f8d..b749c39bb219 100644
@@ -22,7 +22,8 @@
  * Bitmask made from a "or" of all commands within enum membarrier_cmd,
  * except MEMBARRIER_CMD_QUERY.
  */
-#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED)
+#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED |		\
+				 MEMBARRIER_CMD_SHARED_EXPEDITED)
 
 /**
  * sys_membarrier - issue memory barriers on a set of threads

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;
 	default:
 		return -EINVAL;
 	}

Signature · 1 line

2.5.2

Quoted 32 lines · Paul E. McKenney

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

Signed-off-by Paul E. McKenney <[email protected]>

---
 include/uapi/linux/membarrier.h | 11 +++++++++++
 kernel/membarrier.c             |  7 ++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

include/uapi/linux/membarrier.h +10 -0

index e0b108bd2624..ba36d8a6be61 100644
@@ -40,6 +40,16 @@
  *                          (non-running threads are de facto in such a
  *                          state). This covers threads from all processes
  *                          running on the system. This command returns 0.
+ * @MEMBARRIER_CMD_SHARED_EXPEDITED:  Execute a memory barrier on all
+ *			    running threads, but in an expedited fashion.
+ *                          Upon return from system call, the caller thread
+ *                          is ensured that all running threads have passed
+ *                          through a state where all memory accesses to
+ *                          user-space addresses match program order between
+ *                          entry to and return from the system call
+ *                          (non-running threads are de facto in such a
+ *                          state). This covers threads from all processes
+ *                          running on the system. This command returns 0.
  *
  * Command to be passed to the membarrier system call. The commands need to

include/uapi/linux/membarrier.h line 61

Quoted 6 lines

include/uapi/linux/membarrier.h +1 -0

  * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to

@@ -48,6 +58,7 @@
 enum membarrier_cmd {
 	MEMBARRIER_CMD_QUERY = 0,
 	MEMBARRIER_CMD_SHARED = (1 << 0),
+	MEMBARRIER_CMD_SHARED_EXPEDITED = (2 << 0),

Should this better be "(1 << 1)" ;-)

Regards,
Boqun

Quoted 31 lines

include/uapi/linux/membarrier.h +0 -0

 };
 
 #endif /* _UAPI_LINUX_MEMBARRIER_H */

kernel/membarrier.c +6 -1

index 9f9284f37f8d..b749c39bb219 100644
@@ -22,7 +22,8 @@
  * Bitmask made from a "or" of all commands within enum membarrier_cmd,
  * except MEMBARRIER_CMD_QUERY.
  */
-#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED)
+#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED |		\
+				 MEMBARRIER_CMD_SHARED_EXPEDITED)
 
 /**
  * sys_membarrier - issue memory barriers on a set of threads

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;
 	default:
 		return -EINVAL;
 	}

Signature · 1 line

2.5.2

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

Signed-off-by Paul E. McKenney <[email protected]>

---
include/uapi/linux/membarrier.h | 11 +++++++++++
kernel/membarrier.c | 7 ++++++-

2 files changed, 17 insertions(+), 1 deletion(-)

include/uapi/linux/membarrier.h +11 -0

index e0b108bd2624..ba36d8a6be61 100644
@@ -40,6 +40,16 @@
 *                          (non-running threads are de facto in such a
 *                          state). This covers threads from all processes
 *                          running on the system. This command returns 0.
+ * @MEMBARRIER_CMD_SHARED_EXPEDITED:  Execute a memory barrier on all
+ *			    running threads, but in an expedited fashion.
+ *                          Upon return from system call, the caller thread
+ *                          is ensured that all running threads have passed
+ *                          through a state where all memory accesses to
+ *                          user-space addresses match program order between
+ *                          entry to and return from the system call
+ *                          (non-running threads are de facto in such a
+ *                          state). This covers threads from all processes
+ *                          running on the system. This command returns 0.
 *
 * Command to be passed to the membarrier system call. The commands need to
 * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to

@@ -48,6 +58,7 @@
 enum membarrier_cmd {
 	MEMBARRIER_CMD_QUERY = 0,
 	MEMBARRIER_CMD_SHARED = (1 << 0),
+	MEMBARRIER_CMD_SHARED_EXPEDITED = (2 << 0),
 };
 
 #endif /* _UAPI_LINUX_MEMBARRIER_H */

kernel/membarrier.c +6 -2

index 9f9284f37f8d..b749c39bb219 100644
@@ -22,7 +22,8 @@
 * Bitmask made from a "or" of all commands within enum membarrier_cmd,
 * except MEMBARRIER_CMD_QUERY.
 */
-#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED)
+#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED |		\
+				 MEMBARRIER_CMD_SHARED_EXPEDITED)
 
 /**
 * sys_membarrier - issue memory barriers on a set of threads

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;
 	default:
 		return -EINVAL;
 	}
--

2.5.2

Quoted 5 lines · ----- On Jul 24, 2017, at 5:58 PM, Paul E. McKenney [email protected]…

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

Is this now possible because the synchronize_sched_expedited() implementation does not require to send IPIs to all CPUS ? I suspect that using tree srcu now solves this somehow, but can you tell us a bit more about why it is now OK to expose this to user-space ?

The commit message here does not explain why it is OK real-time wise to expose this feature as a system call.

Thanks,

Mathieu

Quoted 63 lines

Signed-off-by Paul E. McKenney <[email protected]>

---
include/uapi/linux/membarrier.h | 11 +++++++++++
kernel/membarrier.c | 7 ++++++-

2 files changed, 17 insertions(+), 1 deletion(-)

include/uapi/linux/membarrier.h +11 -0

index e0b108bd2624..ba36d8a6be61 100644
@@ -40,6 +40,16 @@
 *                          (non-running threads are de facto in such a
 *                          state). This covers threads from all processes
 *                          running on the system. This command returns 0.
+ * @MEMBARRIER_CMD_SHARED_EXPEDITED:  Execute a memory barrier on all
+ *			    running threads, but in an expedited fashion.
+ *                          Upon return from system call, the caller thread
+ *                          is ensured that all running threads have passed
+ *                          through a state where all memory accesses to
+ *                          user-space addresses match program order between
+ *                          entry to and return from the system call
+ *                          (non-running threads are de facto in such a
+ *                          state). This covers threads from all processes
+ *                          running on the system. This command returns 0.
 *
 * Command to be passed to the membarrier system call. The commands need to
 * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to

@@ -48,6 +58,7 @@
 enum membarrier_cmd {
 	MEMBARRIER_CMD_QUERY = 0,
 	MEMBARRIER_CMD_SHARED = (1 << 0),
+	MEMBARRIER_CMD_SHARED_EXPEDITED = (2 << 0),
 };
 
 #endif /* _UAPI_LINUX_MEMBARRIER_H */

kernel/membarrier.c +6 -2

index 9f9284f37f8d..b749c39bb219 100644
@@ -22,7 +22,8 @@
 * Bitmask made from a "or" of all commands within enum membarrier_cmd,
 * except MEMBARRIER_CMD_QUERY.
 */
-#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED)
+#define MEMBARRIER_CMD_BITMASK	(MEMBARRIER_CMD_SHARED |		\
+				 MEMBARRIER_CMD_SHARED_EXPEDITED)
 
 /**
 * sys_membarrier - issue memory barriers on a set of threads

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;
 	default:
 		return -EINVAL;
 	}
--

2.5.2

Signature · 3 lines

Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

Quoted 7 lines · Paul E. McKenney

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

kernel/membarrier.c SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) · line 71

Quoted 7 lines

kernel/membarrier.c +4 -0

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;

So you now give unprivileged userspace the means to IPI the entire machine?

So what do we do when someone goes and does:

	for (;;)
		sys_membarrier(MEMBARRIER_CMD_SHARED_EXPEDITED, 0);

on us?

Quoted 19 lines · Peter Zijlstra

Quoted 7 lines · Paul E. McKenney

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

kernel/membarrier.c SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) · line 71

Quoted 7 lines

kernel/membarrier.c +4 -0

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;

So you now give unprivileged userspace the means to IPI the entire machine?

Quoted 6 lines

So what do we do when someone goes and does:

	for (;;)
		sys_membarrier(MEMBARRIER_CMD_SHARED_EXPEDITED, 0);

on us?

The same thing that happens when they call munmap().

Quoted 25 lines · Paul E. McKenney

Quoted 24 lines · Peter Zijlstra

Quoted 7 lines · Paul E. McKenney

The sys_membarrier() system call has proven too slow for some use cases, which has prompted users to instead rely on TLB shootdown. Although TLB shootdown is much faster, it has the slight disadvantage of not working at all on arm and arm64. This commit therefore adds an expedited option to the sys_membarrier() system call.

kernel/membarrier.c SYSCALL_DEFINE2(membarrier, int, cmd, int, flags) · line 71

Quoted 7 lines

kernel/membarrier.c +4 -0

@@ -64,6 +65,10 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
 		if (num_online_cpus() > 1)
 			synchronize_sched();
 		return 0;
+	case MEMBARRIER_CMD_SHARED_EXPEDITED:
+		if (num_online_cpus() > 1)
+			synchronize_sched_expedited();
+		return 0;

So you now give unprivileged userspace the means to IPI the entire machine?

So what do we do when someone goes and does:

	for (;;)
		sys_membarrier(MEMBARRIER_CMD_SHARED_EXPEDITED, 0);

Quoted 3 lines

Quoted 1 line

on us?

The same thing that happens when they call munmap().

munmap() TLB invalidate is limited to those CPUs that actually ran threads of their process, while this is machine wide.

Quoted 2 lines

munmap() TLB invalidate is limited to those CPUs that actually ran threads of their process, while this is machine wide.

Or those CPUs running threads of any process mapping the underlying file or whatever.

And in either case, this can span the whole machine. Plus there are a number of other ways for users to do on-demand full-system IPIs, including any number of ways to wake up large numbers of CPUs, including from unrelated processes.

But I do plan to add another alternative that is limited to threads of the running process. I will be carrying both versions to enable those who have been bugging me about this to do testing.

Quoted 3 lines · Paul E. McKenney

Quoted 2 lines

munmap() TLB invalidate is limited to those CPUs that actually ran threads of their process, while this is machine wide.

Quoted 2 lines

Or those CPUs running threads of any process mapping the underlying file or whatever.

That doesn't sound right. munmap() of a shared file only invalidates this process's map of it.

Swapping a file page otoh will indeed touch the union of cpumasks over all processes mapping that page.

Quoted 4 lines

And in either case, this can span the whole machine. Plus there are a number of other ways for users to do on-demand full-system IPIs, including any number of ways to wake up large numbers of CPUs, including from unrelated processes.

Which are those? I thought we significantly reduced those with the nohz full work. Most IPI uses now first check if a CPU actually needs the IPI before sending it IIRC.

Quoted 3 lines

But I do plan to add another alternative that is limited to threads of the running process. I will be carrying both versions to enable those who have been bugging me about this to do testing.

Sending IPIs to mm_cpumask() might be better than expedited, but I'm still hesitant. Just because people want it doesn't mean its a good idea. We need to weight this against the potential for abuse.

People want userspace preempt disable, no matter how hard they want it, they're not getting it because its a completely crap idea.

There are a lot of variations, to be sure. For whatever it is worth, the original patch that started this uses mprotect():

https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0

Quoted 3 lines

Which are those? I thought we significantly reduced those with the nohz full work. Most IPI uses now first check if a CPU actually needs the IPI before sending it IIRC.

If the task being awakened is higher priority than the task currently running on a given CPU, that CPU still gets an IPI, right? Or am I completely confused?

Unlike userspace preempt disable, in this case we get the abuse anyway via existing mechanisms, as in they are already being abused. If we provide a mechanism for this purpose, we at least have the potential for handling the abuse, for example:

o "Defanging" sys_membarrier() on systems that are sensitive to
latency. For example, this patch can be defanged by booting with the rcupdate.rcu_normal=1 kernel boot parameter, which causes requests for expedited grace periods to instead use normal grace periods.

o Detecting and responding to abuse. For example, perhaps if there
are more than (say) 50 expedited sys_membarrier()s within a given jiffy, the excess sys_membarrier()s are non-expedited.

o Batching optimizations allow large number of concurrent requests
to be handled with fewer grace periods -- and both normal and expedited grace periods already do exactly this.

This horse is already out, so trying to shut the gate won't be effective.

Quoted 4 lines · Paul E. McKenney

There are a lot of variations, to be sure. For whatever it is worth, the original patch that started this uses mprotect():

https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0

FWIW that will not work on s390 (and maybe others), they don't in fact require IPIs for remote TLB invalidation.

Quoted 7 lines

Quoted 3 lines

Which are those? I thought we significantly reduced those with the nohz full work. Most IPI uses now first check if a CPU actually needs the IPI before sending it IIRC.

If the task being awakened is higher priority than the task currently running on a given CPU, that CPU still gets an IPI, right? Or am I completely confused?

I was thinking of things like on_each_cpu_cond().

Quoted 15 lines

Unlike userspace preempt disable, in this case we get the abuse anyway via existing mechanisms, as in they are already being abused. If we provide a mechanism for this purpose, we at least have the potential for handling the abuse, for example:

o "Defanging" sys_membarrier() on systems that are sensitive to
latency. For example, this patch can be defanged by booting with the rcupdate.rcu_normal=1 kernel boot parameter, which causes requests for expedited grace periods to instead use normal grace periods.

o Detecting and responding to abuse. For example, perhaps if there
are more than (say) 50 expedited sys_membarrier()s within a given jiffy, the excess sys_membarrier()s are non-expedited.

Quoted 5 lines

o Batching optimizations allow large number of concurrent requests
to be handled with fewer grace periods -- and both normal and expedited grace periods already do exactly this.

This horse is already out, so trying to shut the gate won't be effective.

Well, I'm not sure there is an easy means of doing machine wide IPIs for !root out there. This would be a first.

Something along the lines of:

void dummy(void *arg)

{
	/* IPIs are assumed to be serializing */
}

void ipi_mm(struct mm_struct *mm)

{
	cpumask_var_t cpus;
	int cpu;

	zalloc_cpumask_var(&cpus, GFP_KERNEL);

	for_each_cpu(cpu, mm_cpumask(mm)) {
		struct task_struct *p;

		/*
		 * If the current task of @cpu isn't of this @mm, then
		 * it needs a context switch to become one, which will
		 * provide the ordering we require.
		 */
		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == mm)
			__cpumask_set_cpu(cpu, cpus);
		rcu_read_unlock();
	}

	on_each_cpu_mask(cpus, dummy, NULL, 1);
}

Would appear to be minimally invasive and only shoot at CPUs we're currently running our process on, which greatly reduces the impact.

Quoted 5 lines · Peter Zijlstra

Quoted 3 lines · Paul E. McKenney

There are a lot of variations, to be sure. For whatever it is worth, the original patch that started this uses mprotect():

Quoted 4 lines

Quoted 1 line

https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0

FWIW that will not work on s390 (and maybe others), they don't in fact require IPIs for remote TLB invalidation.

Nor will it for ARM. Nor (I think) for PowerPC. But that is in fact what people are doing right now in real life. Hence my renewed interest in sys_membarrier().

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

Quoted 35 lines

Well, I'm not sure there is an easy means of doing machine wide IPIs for !root out there. This would be a first.

Something along the lines of:

void dummy(void *arg)

{
	/* IPIs are assumed to be serializing */
}

void ipi_mm(struct mm_struct *mm)

{
	cpumask_var_t cpus;
	int cpu;

	zalloc_cpumask_var(&cpus, GFP_KERNEL);

	for_each_cpu(cpu, mm_cpumask(mm)) {
		struct task_struct *p;

		/*
		 * If the current task of @cpu isn't of this @mm, then
		 * it needs a context switch to become one, which will
		 * provide the ordering we require.
		 */
		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == mm)
			__cpumask_set_cpu(cpu, cpus);
		rcu_read_unlock();
	}

	on_each_cpu_mask(cpus, dummy, NULL, 1);
}

Quoted 2 lines

Would appear to be minimally invasive and only shoot at CPUs we're currently running our process on, which greatly reduces the impact.

I am good with this approach as well, and I do very much like that it avoids IPIing CPUs that aren't running our process (at least in the common case). But don't we also need added memory ordering? It is sort of OK to IPI a CPU that just now switched away from our process, but not so good to miss IPIing a CPU that switched to our process just a little before sys_membarrier().

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Discussion here:

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Thoughts?

Quoted 11 lines · Paul E. McKenney

Quoted 9 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There are a lot of variations, to be sure. For whatever it is worth, the original patch that started this uses mprotect():

https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0

FWIW that will not work on s390 (and maybe others), they don't in fact require IPIs for remote TLB invalidation.

Quoted 3 lines

Nor will it for ARM. Nor (I think) for PowerPC. But that is in fact what people are doing right now in real life. Hence my renewed interest in sys_membarrier().

People always do crazy stuff, but what surprised me is that such s patch got merged in urcu even though its known broken for a number of architectures.

Quoted 2 lines · Paul E. McKenney

Quoted 1 line · Peter Zijlstra

On Tue, Jul 25, 2017 at 12:36:12PM -0700, Paul E. McKenney wrote:

Quoted 4 lines

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

RT tasks are not readily available to !root, and the user might have been constrained to a subset of available CPUs.

Quoted 38 lines

Quoted 35 lines

Well, I'm not sure there is an easy means of doing machine wide IPIs for !root out there. This would be a first.

Something along the lines of:

void dummy(void *arg)

{
	/* IPIs are assumed to be serializing */
}

void ipi_mm(struct mm_struct *mm)

{
	cpumask_var_t cpus;
	int cpu;

	zalloc_cpumask_var(&cpus, GFP_KERNEL);

	for_each_cpu(cpu, mm_cpumask(mm)) {
		struct task_struct *p;

		/*
		 * If the current task of @cpu isn't of this @mm, then
		 * it needs a context switch to become one, which will
		 * provide the ordering we require.
		 */
		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == mm)
			__cpumask_set_cpu(cpu, cpus);
		rcu_read_unlock();
	}

	on_each_cpu_mask(cpus, dummy, NULL, 1);
}

Quoted 2 lines

Would appear to be minimally invasive and only shoot at CPUs we're currently running our process on, which greatly reduces the impact.

Quoted 6 lines

I am good with this approach as well, and I do very much like that it avoids IPIing CPUs that aren't running our process (at least in the common case). But don't we also need added memory ordering? It is sort of OK to IPI a CPU that just now switched away from our process, but not so good to miss IPIing a CPU that switched to our process just a little before sys_membarrier().

My thinking was that if we observe '!= mm' that CPU will have to do a context switch in order to make it true. That context switch will provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone needs to spell it out for me :-)

Quoted 8 lines

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Quoted 6 lines

Discussion here:

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

TL;DR.. that's far too much to trawl through.

Quoted 4 lines

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

Quoted 4 lines

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Thoughts?

Yes, there are architectures that only accumulate bits in mm_cpumask(), with the additional check to see if the remote task belongs to our MM this should be a non-issue.

Quoted 3 lines · ----- On Jul 25, 2017, at 5:55 PM, Peter Zijlstra [email protected] wrote:

Quoted 2 lines · Paul E. McKenney

Quoted 1 line · Peter Zijlstra

On Tue, Jul 25, 2017 at 12:36:12PM -0700, Paul E. McKenney wrote:

[...]

Quoted 81 lines

Quoted 4 lines

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

RT tasks are not readily available to !root, and the user might have been constrained to a subset of available CPUs.

Quoted 38 lines

Quoted 35 lines

Well, I'm not sure there is an easy means of doing machine wide IPIs for !root out there. This would be a first.

Something along the lines of:

void dummy(void *arg)

{
	/* IPIs are assumed to be serializing */
}

void ipi_mm(struct mm_struct *mm)

{
	cpumask_var_t cpus;
	int cpu;

	zalloc_cpumask_var(&cpus, GFP_KERNEL);

	for_each_cpu(cpu, mm_cpumask(mm)) {
		struct task_struct *p;

		/*
		 * If the current task of @cpu isn't of this @mm, then
		 * it needs a context switch to become one, which will
		 * provide the ordering we require.
		 */
		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == mm)
			__cpumask_set_cpu(cpu, cpus);
		rcu_read_unlock();
	}

	on_each_cpu_mask(cpus, dummy, NULL, 1);
}

Quoted 2 lines

Would appear to be minimally invasive and only shoot at CPUs we're currently running our process on, which greatly reduces the impact.

Quoted 6 lines

I am good with this approach as well, and I do very much like that it avoids IPIing CPUs that aren't running our process (at least in the common case). But don't we also need added memory ordering? It is sort of OK to IPI a CPU that just now switched away from our process, but not so good to miss IPIing a CPU that switched to our process just a little before sys_membarrier().

My thinking was that if we observe '!= mm' that CPU will have to do a context switch in order to make it true. That context switch will provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone needs to spell it out for me :-)

Quoted 8 lines

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Quoted 6 lines

Discussion here:

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

TL;DR.. that's far too much to trawl through.

Quoted 4 lines

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

Quoted 8 lines

Quoted 4 lines

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Thoughts?

Yes, there are architectures that only accumulate bits in mm_cpumask(), with the additional check to see if the remote task belongs to our MM this should be a non-issue.

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

The whole private_expedited is only palatable because we can only hinder our own threads (much).

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Quoted 13 lines · ----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney…

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Thoughts ?

Quoted 33 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney [email protected]

Quoted 13 lines · wrote:

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Quoted 1 line

Thoughts ?

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

But who is pushing for multiple-process sys_membarrier()? Everyone I have talked to is OK with it being local to the current process.

Quoted 34 lines · ----- On Jul 26, 2017, at 2:30 PM, Paul E. McKenney [email protected]…

Quoted 33 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney [email protected]

Quoted 13 lines · wrote:

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Quoted 7 lines

Quoted 1 line

Thoughts ?

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

But who is pushing for multiple-process sys_membarrier()? Everyone I have talked to is OK with it being local to the current process.

I guess I'm probably the guilty one intending to do weird stuff in userspace ;)

Here are my two use-cases:

* a new multi-process liburcu flavor, useful if e.g. a set of processes are responsible for updating a shared memory data structure, and a separate set of processes read that data structure. The readers can be killed without ill effect on the other processes. The synchronization could be done by one multi-process liburcu flavor per reader process "group".

* lttng-ust user-space ring buffers (shared across processes).

Both rely on a shared memory mapping for communication between processes, and I would like to be able to issue a sys_membarrier targeting all CPUs that may currently touch the shared memory mapping.

I don't really need a system-wide effect, but I would like to be able to target a shared memory mapping and efficiently do an expedited sys_membarrier on all cpus involved.

With lttng-ust, the shared buffers can spawn across 1000+ processes, so asking each process to issue sys_membarrier would add lots of unneeded overhead, because this would issue lots of needless memory barriers.

Thoughts ?

Quoted 64 lines · Mathieu Desnoyers

Quoted 34 lines · ----- On Jul 26, 2017, at 2:30 PM, Paul E. McKenney [email protected]…

Quoted 33 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney [email protected]

Quoted 13 lines · wrote:

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Quoted 7 lines

Quoted 1 line

Thoughts ?

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

But who is pushing for multiple-process sys_membarrier()? Everyone I have talked to is OK with it being local to the current process.

I guess I'm probably the guilty one intending to do weird stuff in userspace ;)

Here are my two use-cases:

* a new multi-process liburcu flavor, useful if e.g. a set of processes are responsible for updating a shared memory data structure, and a separate set of processes read that data structure. The readers can be killed without ill effect on the other processes. The synchronization could be done by one multi-process liburcu flavor per reader process "group".

* lttng-ust user-space ring buffers (shared across processes).

Both rely on a shared memory mapping for communication between processes, and I would like to be able to issue a sys_membarrier targeting all CPUs that may currently touch the shared memory mapping.

I don't really need a system-wide effect, but I would like to be able to target a shared memory mapping and efficiently do an expedited sys_membarrier on all cpus involved.

Quoted 5 lines

With lttng-ust, the shared buffers can spawn across 1000+ processes, so asking each process to issue sys_membarrier would add lots of unneeded overhead, because this would issue lots of needless memory barriers.

Thoughts ?

Dealing explicitly with 1000+ processes sounds like no picnic. It instead sounds like a job for synchronize_sched_expedited(). ;-)

Quoted 70 lines · Paul E. McKenney

Quoted 69 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 2:30 PM, Paul E. McKenney [email protected]

Quoted 34 lines · wrote:

Quoted 33 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney [email protected]

Quoted 13 lines · wrote:

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Quoted 7 lines

Quoted 1 line

Thoughts ?

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

But who is pushing for multiple-process sys_membarrier()? Everyone I have talked to is OK with it being local to the current process.

I guess I'm probably the guilty one intending to do weird stuff in userspace ;)

Here are my two use-cases:

* a new multi-process liburcu flavor, useful if e.g. a set of processes are responsible for updating a shared memory data structure, and a separate set of processes read that data structure. The readers can be killed without ill effect on the other processes. The synchronization could be done by one multi-process liburcu flavor per reader process "group".

* lttng-ust user-space ring buffers (shared across processes).

Both rely on a shared memory mapping for communication between processes, and I would like to be able to issue a sys_membarrier targeting all CPUs that may currently touch the shared memory mapping.

I don't really need a system-wide effect, but I would like to be able to target a shared memory mapping and efficiently do an expedited sys_membarrier on all cpus involved.

With lttng-ust, the shared buffers can spawn across 1000+ processes, so asking each process to issue sys_membarrier would add lots of unneeded overhead, because this would issue lots of needless memory barriers.

Quoted 4 lines

Quoted 1 line

Thoughts ?

Dealing explicitly with 1000+ processes sounds like no picnic. It instead sounds like a job for synchronize_sched_expedited(). ;-)

Actually...

Mathieu, does your use case require unprivileged access to sys_membarrier()?

Thanx, Paul

Quoted 76 lines · ----- On Jul 26, 2017, at 9:45 PM, Paul E. McKenney [email protected]…

Quoted 74 lines · Paul E. McKenney

Quoted 65 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 2:30 PM, Paul E. McKenney [email protected]

Quoted 34 lines · wrote:

Quoted 33 lines · Mathieu Desnoyers

----- On Jul 26, 2017, at 11:42 AM, Paul E. McKenney [email protected]

Quoted 13 lines · wrote:

Quoted 12 lines · Peter Zijlstra

Quoted 10 lines · Mathieu Desnoyers

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag for expedited process-local effect. This differs from the "SHARED" flag, since the SHARED flag affects threads accessing memory mappings shared across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior by iterating on all memory mappings mapped into the current process, and build a cpumask based on the union of all mm masks encountered ? Then we could send the IPI to all cpus belonging to that cpumask. Or am I missing something obvious ?

Quoted 6 lines

Quoted 3 lines

I would readily object to such a beast. You far too quickly end up having to IPI everybody because of some stupid shared map or something (yes I know, normal DSOs are mapped private).

Agreed, we should keep things simple to start with. The user can always invoke sys_membarrier() from each process.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Quoted 7 lines

Quoted 1 line

Thoughts ?

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

But who is pushing for multiple-process sys_membarrier()? Everyone I have talked to is OK with it being local to the current process.

I guess I'm probably the guilty one intending to do weird stuff in userspace ;)

Here are my two use-cases:

* a new multi-process liburcu flavor, useful if e.g. a set of processes are responsible for updating a shared memory data structure, and a separate set of processes read that data structure. The readers can be killed without ill effect on the other processes. The synchronization could be done by one multi-process liburcu flavor per reader process "group".

* lttng-ust user-space ring buffers (shared across processes).

Both rely on a shared memory mapping for communication between processes, and I would like to be able to issue a sys_membarrier targeting all CPUs that may currently touch the shared memory mapping.

I don't really need a system-wide effect, but I would like to be able to target a shared memory mapping and efficiently do an expedited sys_membarrier on all cpus involved.

Quoted 5 lines

With lttng-ust, the shared buffers can spawn across 1000+ processes, so asking each process to issue sys_membarrier would add lots of unneeded overhead, because this would issue lots of needless memory barriers.

Thoughts ?

Dealing explicitly with 1000+ processes sounds like no picnic. It instead sounds like a job for synchronize_sched_expedited(). ;-)

Quoted 3 lines

Actually...

Mathieu, does your use case require unprivileged access to sys_membarrier()?

Unfortunately, yes, it does require sys_membarrier to be used from non-root both for lttng-ust and liburcu multi-process. And as Peter pointed out, stuff like containers complicates things even for the root case.

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

Quoted 2 lines · Paul E. McKenney

The patch I posted reverts to synchronize_sched() in kernels booted with rcupdate.rcu_normal=1. ;-)

So boot parameters are no solution and are only slightly better than compile time switches.

What if you have a machine that runs workloads that want both options? partitioning and containers are somewhat popular these days and system wide tunables don't work for them.

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Thoughts ?

Another crazy idea is using madvise() for this. The new MADV_MEMBAR could revoke PROT_WRITE and PROT_READ for all extant PTEs. Then the tasks attempting access will fault and the fault handler can figure out if it still needs to issue a MB or not before reinstating the PTE.

That is fully contained to the tasks actually having that map, and doesn't perturb anybody else.

Quoted 5 lines · Mathieu Desnoyers

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

You forgot to ask yourself how you could abuse this.. just spawn more threads.

Per-thread limits are nearly useless, because spawning new threads is cheap.

Quoted 5 lines

A RT system that really cares about not having userspace sending IPIs to all cpus could set the ulimit value to 0, which would always use synchronize_sched().

Thoughts ?

So I really don't like SHARED_EXPEDITED, and your use-cases (from later emails) makes me think sys_membarrier() should have a pointer argument to identify the shared mapping.

But even then, iterating the rmap for something that has 1000+ maps isn't going to be nice or fast, even in kernel space.

Another crazy idea is using madvise() for this. The new MADV_MEMBAR could revoke PROT_WRITE and PROT_READ for all extant PTEs. Then the tasks attempting access will fault and the fault handler can figure out if it still needs to issue a MB or not before reinstating the PTE.

Quoted 5 lines · Mathieu Desnoyers

Another alternative for a MEMBARRIER_CMD_SHARED_EXPEDITED would be rate-limiting per thread. For instance, we could add a new "ulimit" that would bound the number of expedited membarrier per thread that can be done per millisecond, and switch to synchronize_sched() whenever a thread goes beyond that limit for the rest of the time-slot.

You forgot to ask yourself how you could abuse this.. just spawn more threads.

Per-thread limits are nearly useless, because spawning new threads is cheap.

Quoted 11 lines · Paul E. McKenney

Quoted 9 lines · Peter Zijlstra

Quoted 4 lines · Paul E. McKenney

There are a lot of variations, to be sure. For whatever it is worth, the original patch that started this uses mprotect():

https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0

FWIW that will not work on s390 (and maybe others), they don't in fact require IPIs for remote TLB invalidation.

Quoted 3 lines

Nor will it for ARM. Nor (I think) for PowerPC. But that is in fact what people are doing right now in real life. Hence my renewed interest in sys_membarrier().

People always do crazy stuff, but what surprised me is that such s patch got merged in urcu even though its known broken for a number of architectures.

Quoted 3 lines

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

RT tasks are not readily available to !root, and the user might have been constrained to a subset of available CPUs.

Quoted 38 lines

Quoted 35 lines

Well, I'm not sure there is an easy means of doing machine wide IPIs for !root out there. This would be a first.

Something along the lines of:

void dummy(void *arg)

{
	/* IPIs are assumed to be serializing */
}

void ipi_mm(struct mm_struct *mm)

{
	cpumask_var_t cpus;
	int cpu;

	zalloc_cpumask_var(&cpus, GFP_KERNEL);

	for_each_cpu(cpu, mm_cpumask(mm)) {
		struct task_struct *p;

		/*
		 * If the current task of @cpu isn't of this @mm, then
		 * it needs a context switch to become one, which will
		 * provide the ordering we require.
		 */
		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == mm)
			__cpumask_set_cpu(cpu, cpus);
		rcu_read_unlock();
	}

	on_each_cpu_mask(cpus, dummy, NULL, 1);
}

Quoted 2 lines

Would appear to be minimally invasive and only shoot at CPUs we're currently running our process on, which greatly reduces the impact.

Quoted 6 lines

I am good with this approach as well, and I do very much like that it avoids IPIing CPUs that aren't running our process (at least in the common case). But don't we also need added memory ordering? It is sort of OK to IPI a CPU that just now switched away from our process, but not so good to miss IPIing a CPU that switched to our process just a little before sys_membarrier().

My thinking was that if we observe '!= mm' that CPU will have to do a context switch in order to make it true. That context switch will provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone needs to spell it out for me :-)

Quoted 8 lines

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Quoted 6 lines

Discussion here:

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

TL;DR.. that's far too much to trawl through.

Quoted 4 lines

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

Quoted 4 lines

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Thoughts?

Yes, there are architectures that only accumulate bits in mm_cpumask(), with the additional check to see if the remote task belongs to our MM this should be a non-issue.

Quoted 3 lines · Peter Zijlstra

People always do crazy stuff, but what surprised me is that such s patch got merged in urcu even though its known broken for a number of architectures.

It did not get merged into urcu. It is instead used directly by a number of people for a number of concurrent algorithms.

Quoted 6 lines

Quoted 3 lines

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

RT tasks are not readily available to !root, and the user might have been constrained to a subset of available CPUs.

So non-idle non-nohz CPUs never get IPIed for wakeups of SCHED_OTHER threads?

Quoted 6 lines

My thinking was that if we observe '!= mm' that CPU will have to do a context switch in order to make it true. That context switch will provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone needs to spell it out for me :-)

This would be the https://marc.info/?l=linux-kernel&m=126349766324224&w=2
URL below.

Which might or might not still be applicable.

Quoted 10 lines

Quoted 10 lines

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Discussion here:

Quoted 6 lines

Quoted 4 lines

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

TL;DR.. that's far too much to trawl through.

So we re-derive it from first principles instead? ;-)

Quoted 8 lines · Paul E. McKenney

Quoted 3 lines · Peter Zijlstra

People always do crazy stuff, but what surprised me is that such s patch got merged in urcu even though its known broken for a number of architectures.

It did not get merged into urcu. It is instead used directly by a number of people for a number of concurrent algorithms.

Yah, Mathieu also already pointed that out. It seems I really cannot deal with github well -- that website always terminally confuses me.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

But it would not be hard for userspace code to force IPIs by repeatedly awakening higher-priority threads that sleep immediately after being awakened, right?

RT tasks are not readily available to !root, and the user might have been constrained to a subset of available CPUs.

Quoted 2 lines

So non-idle non-nohz CPUs never get IPIed for wakeups of SCHED_OTHER threads?

Sure, but SCHED_OTHER auto throttles in that if there's anything else to run, you get to wait. So you can't generate an IPI storm with it. Also, again, we can be limited to a subset of CPUs.

Quoted 11 lines

Quoted 6 lines

My thinking was that if we observe '!= mm' that CPU will have to do a context switch in order to make it true. That context switch will provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone needs to spell it out for me :-)

This would be the https://marc.info/?l=linux-kernel&m=126349766324224&w=2
URL below.

Which might or might not still be applicable.

I think we actually have those two smp_mb()'s around the rq->curr assignment.

we have smp_mb__before_spinlock(), which per the argument here:

https://lkml.kernel.org/r/[email protected]

is actually a full MB, irrespective of that weird smp_wmb() definition we have now. And we have switch_mm() on the other side.

Quoted 15 lines

Quoted 15 lines

Quoted 14 lines

I was intending to base this on the last few versions of a 2010 patch, but maybe things have changed:

	https://marc.info/?l=linux-kernel&m=126358017229620&w=2
	https://marc.info/?l=linux-kernel&m=126436996014016&w=2
	https://marc.info/?l=linux-kernel&m=126601479802978&w=2
	https://marc.info/?l=linux-kernel&m=126970692903302&w=2

Discussion here:

	https://marc.info/?l=linux-kernel&m=126349766324224&w=2

The discussion led to acquiring the runqueue locks, as there was otherwise a need to add code to the scheduler fastpaths.

Quoted 3 lines

Quoted 1 line

TL;DR.. that's far too much to trawl through.

So we re-derive it from first principles instead? ;-)

Yep, that's what I usually do anyway, who knows what kind of crazy our younger selves were up to ;-)

Quoted 5 lines · Peter Zijlstra

On Tue, Jul 25, 2017 at 04:59:36PM -0700, Paul E. McKenney wrote:

Sure, but SCHED_OTHER auto throttles in that if there's anything else to run, you get to wait. So you can't generate an IPI storm with it. Also, again, we can be limited to a subset of CPUs.

OK, what is its auto-throttle policy? One round of IPIs per jiffy or some such?

Does this auto-throttling also apply if the user is running a CPU-bound SCHED_BATCH or SCHED_IDLE task on each CPU, and periodically waking up one of a large group of SCHED_OTHER tasks, where the SCHED_OTHER tasks immediately sleep upon being awakened?

OK, and the rq->curr assignment is in common code, correct? Does this allow the IPI-only-requesting-process approach to live entirely within common code?

The 2010 email thread ended up with sys_membarrier() acquiring the runqueue lock for each CPU,

because doing otherwise meant adding code to the scheduler fastpath.

Don't we still need to do this?

https://marc.info/?l=linux-kernel&m=126341138408407&w=2
https://marc.info/?l=linux-kernel&m=126349766324224&w=2

Quoted 7 lines · Paul E. McKenney

Quoted 5 lines · Peter Zijlstra

On Tue, Jul 25, 2017 at 04:59:36PM -0700, Paul E. McKenney wrote:

Sure, but SCHED_OTHER auto throttles in that if there's anything else to run, you get to wait. So you can't generate an IPI storm with it. Also, again, we can be limited to a subset of CPUs.

Quoted 2 lines

OK, what is its auto-throttle policy? One round of IPIs per jiffy or some such?

No. Its called wakeup latency :-) Your SCHED_OTHER task will not get to insta-run all the time. If there are other tasks already running, we'll not IPI unless it should preempt.

If its idle, nobody cares..

Quoted 4 lines

Does this auto-throttling also apply if the user is running a CPU-bound SCHED_BATCH or SCHED_IDLE task on each CPU, and periodically waking up one of a large group of SCHED_OTHER tasks, where the SCHED_OTHER tasks immediately sleep upon being awakened?

SCHED_BATCH is even more likely to suffer wakeup latency since it will never preempt anything.

Quoted 3 lines

OK, and the rq->curr assignment is in common code, correct? Does this allow the IPI-only-requesting-process approach to live entirely within common code?

That is the idea.

Quoted 2 lines

The 2010 email thread ended up with sys_membarrier() acquiring the runqueue lock for each CPU,

Yes, that's something I'm not happy with. Machine wide banging of that lock will be a performance no-no.

Quoted 1 line

because doing otherwise meant adding code to the scheduler fastpath.

And that's obviously another thing I'm not happy with either.

Quoted 4 lines

Don't we still need to do this?

https://marc.info/?l=linux-kernel&m=126341138408407&w=2
https://marc.info/?l=linux-kernel&m=126349766324224&w=2

I don't know.. those seem focussed on mm_cpumask() and we can't use that per Will's email.

So I think we need to think anew on this, start from the ground up.

What is missing for this:

static void ipi_mb(void *info)

{
	smp_mb(); // IPIs should be serializing but paranoid
}

sys_membarrier()

{
	smp_mb(); // because sysenter isn't an unconditional mb

	for_each_online_cpu(cpu) {
		struct task_struct *p;

		rcu_read_lock();
		p = task_rcu_dereference(&cpu_curr(cpu));
		if (p && p->mm == current->mm)
			__set_bit(cpus, cpu);
		rcu_read_unlock();
	}

	on_cpu_cpu_mask(cpus, ipi_mb, NULL, 1); // does local smp_mb() too
}

VS

__schedule()

{
	spin_lock(&rq->lock);
	smp_mb__after_spinlock(); // really full mb implied

	/* lots */

	if (likely(prev != next)) {

		rq->curr = next;

		context_switch() {
			switch_mm();
			switch_to();
			// neither need imply a barrier

			spin_unlock(&rq->lock);
		}
	}
}

So I think we need either switch_mm() or switch_to() to imply a full barrier for this to work, otherwise we get:

CPU0 CPU1

lock rq->lock
mb

rq->curr = A

unlock rq->lock

lock rq->lock
mb

				sys_membarrier()

				mb

				for_each_online_cpu()
				  p = A
				  // no match no IPI

				mb

rq->curr = B

unlock rq->lock

And that's bad, because now CPU0 doesn't have an MB happening _after_ sys_membarrier() if B matches.

So without audit, I only know of PPC and Alpha not having a barrier in either switch_*().

x86 obviously has barriers all over the place, arm has a super duper heavy barrier in switch_to().

One option might be to resurrect spin_unlock_wait(), although to use that here is really ugly too, but it would avoid thrashing the rq->lock.

I think it'd end up having to look like:

	rq = cpu_rq(cpu);
again:
	rcu_read_lock()
	p = task_rcu_dereference(&rq->curr);
	if (p) {
		raw_spin_unlock_wait(&rq->lock);
		q = task_rcu_dereference(&rq->curr);
		if (q != p) {
			rcu_read_unlock();
			goto again;
		}
	}
	...

which is just about as horrible as it looks.

Quoted 5 lines

No. Its called wakeup latency :-) Your SCHED_OTHER task will not get to insta-run all the time. If there are other tasks already running, we'll not IPI unless it should preempt.

If its idle, nobody cares..

So it does IPI immediately sometimes.

Quoted 5 lines

Quoted 4 lines

Does this auto-throttling also apply if the user is running a CPU-bound SCHED_BATCH or SCHED_IDLE task on each CPU, and periodically waking up one of a large group of SCHED_OTHER tasks, where the SCHED_OTHER tasks immediately sleep upon being awakened?

Quoted 2 lines

SCHED_BATCH is even more likely to suffer wakeup latency since it will never preempt anything.

Ahem. In this scenario, SCHED_BATCH is already running on a the CPU in question, and a SCHED_OTHER task is awakened from some other CPU.

Do we IPI in that case.

Quoted 13 lines · Paul E. McKenney

Quoted 5 lines

No. Its called wakeup latency :-) Your SCHED_OTHER task will not get to insta-run all the time. If there are other tasks already running, we'll not IPI unless it should preempt.

If its idle, nobody cares..

So it does IPI immediately sometimes.

Quoted 5 lines

Quoted 4 lines

Does this auto-throttling also apply if the user is running a CPU-bound SCHED_BATCH or SCHED_IDLE task on each CPU, and periodically waking up one of a large group of SCHED_OTHER tasks, where the SCHED_OTHER tasks immediately sleep upon being awakened?

Quoted 7 lines

Quoted 2 lines

SCHED_BATCH is even more likely to suffer wakeup latency since it will never preempt anything.

Ahem. In this scenario, SCHED_BATCH is already running on a the CPU in question, and a SCHED_OTHER task is awakened from some other CPU.

Do we IPI in that case.

So I'm a bit confused as to where you're trying to go with this.

I'm saying that if there are other users of our CPU, we can't significantly disturb them with IPIs.

Yes, we'll sometimes IPI in order to do a preemption on wakeup. But we cannot always win that. There is no wakeup triggered starvation case.

If you create a thread per CPU and have them insta sleep after wakeup, and then keep prodding them to wakeup. They will disturb things less than if they were while(1); loops.

If the machine is otherwise idle, nobody cares.

If there are other tasks on the system, the IPI rate is limited to the wakeup latency of you tasks.

And any of this is limited to the CPUs we're allowed to run in the first place.

So yes, the occasional IPI happens, but if there's other tasks, we can't disturb them more than we could with while(1); tasks.

OTOH, something like:

	while(1)
		synchronize_sched_expedited();

as per your proposed patch, will spray IPIs to all CPUs and at high rates.

Quoted 2 lines

as per your proposed patch, will spray IPIs to all CPUs and at high rates.

OK, I have updated my patch to do throttling.

Quoted 4 lines · Paul E. McKenney

Quoted 2 lines

as per your proposed patch, will spray IPIs to all CPUs and at high rates.

OK, I have updated my patch to do throttling.

But not respect cpusets. Which is the other important point.

The scheduler based IPIs are limited to where we are allowed to place tasks (as are the TLB ones for that matter, for the exact same reason).

Hi Paul,

I have a side question out of curiosity:

How does synchronize_sched() work properly for sys_membarrier()?

sys_membarrier() requires every other CPU does a smp_mb() before it returns, and I know synchronize_sched() will wait until all CPUs running a kernel thread do a context-switch, which has a smp_mb(). However, I believe sched flavor RCU treat CPU running a user thread as a quiesent state, so synchronize_sched() could return without that CPU does a context switch.

So why could we use synchronize_sched() for sys_membarrier()?

In particular, could the following happens?

CPU 0: CPU 1:
========================= ==========================
<in user space> <in user space>
{read Y}(reordered) <------------------------------+
store Y; |
read X; --------------------------------------+ |
sys_membarrier(): <timer interrupt> | |
synchronize_sched(); update_process_times(user): //user == true | |
rcu_check_callbacks(usr): | |
if (user || ..) { | |
rcu_sched_qs() | |
... | |
<report quesient state in softirq> | |

					<return to user space>                        |    |
					read Y; --------------------------------------+----+
	store X;			                                              |
					{read X}(reordered) <-------------------------+

I assume the timer interrupt handler, which interrupts a user space and reports a quiesent state for sched flavor RCU, may not have a smp_mb() in some code path.

I may miss something subtle, but it just not very obvious how synchronize_sched() will guarantee a remote CPU running in userspace to do a smp_mb() before it returns, this is at least not in RCU requirements, right?

Regards,
Boqun

Quoted 17 lines · Boqun Feng

Hi Paul,

I have a side question out of curiosity:

How does synchronize_sched() work properly for sys_membarrier()?

sys_membarrier() requires every other CPU does a smp_mb() before it returns, and I know synchronize_sched() will wait until all CPUs running a kernel thread do a context-switch, which has a smp_mb(). However, I believe sched flavor RCU treat CPU running a user thread as a quiesent state, so synchronize_sched() could return without that CPU does a context switch.

So why could we use synchronize_sched() for sys_membarrier()?

In particular, could the following happens?

Quoted 13 lines

CPU 0: CPU 1:
========================= ==========================
<in user space> <in user space>
{read Y}(reordered) <------------------------------+
store Y; |
read X; --------------------------------------+ |
sys_membarrier(): <timer interrupt> | |
synchronize_sched(); update_process_times(user): //user == true | |
rcu_check_callbacks(usr): | |
if (user || ..) { | |
rcu_sched_qs() | |
... | |
<report quesient state in softirq> | |

The reporting of the quiescent state will acquire the leaf rcu_node structure's lock, with an smp_mb__after_unlock_lock(), which will one way or another be a full memory barrier. So the reorderings cannot happen.

Unless I am missing something subtle. ;-)

Thanx, Paul

Quoted 16 lines

					<return to user space>                        |    |
					read Y; --------------------------------------+----+
	store X;			                                              |
					{read X}(reordered) <-------------------------+

I assume the timer interrupt handler, which interrupts a user space and reports a quiesent state for sched flavor RCU, may not have a smp_mb() in some code path.

I may miss something subtle, but it just not very obvious how synchronize_sched() will guarantee a remote CPU running in userspace to do a smp_mb() before it returns, this is at least not in RCU requirements, right?

Regards,
Boqun

Quoted 32 lines · Paul E. McKenney

Quoted 17 lines · Boqun Feng

Hi Paul,

I have a side question out of curiosity:

How does synchronize_sched() work properly for sys_membarrier()?

sys_membarrier() requires every other CPU does a smp_mb() before it returns, and I know synchronize_sched() will wait until all CPUs running a kernel thread do a context-switch, which has a smp_mb(). However, I believe sched flavor RCU treat CPU running a user thread as a quiesent state, so synchronize_sched() could return without that CPU does a context switch.

So why could we use synchronize_sched() for sys_membarrier()?

In particular, could the following happens?

Quoted 13 lines

CPU 0: CPU 1:
========================= ==========================
<in user space> <in user space>
{read Y}(reordered) <------------------------------+
store Y; |
read X; --------------------------------------+ |
sys_membarrier(): <timer interrupt> | |
synchronize_sched(); update_process_times(user): //user == true | |
rcu_check_callbacks(usr): | |
if (user || ..) { | |
rcu_sched_qs() | |
... | |
<report quesient state in softirq> | |

Quoted 7 lines

The reporting of the quiescent state will acquire the leaf rcu_node structure's lock, with an smp_mb__after_unlock_lock(), which will one way or another be a full memory barrier. So the reorderings cannot happen.

Unless I am missing something subtle. ;-)

Well, smp_mb__after_unlock_lock() in ARM64 is a no-op, and ARM64's lock doesn't provide a smp_mb().

So my point is more like: synchronize_sched() happens to be a sys_membarrier() because of some implementation detail, and if some day we come up with a much cheaper way to implement sched flavor RCU(hopefully!), synchronize_sched() may be not good for the job. So at least, we'd better document this somewhere?

Regards,
Boqun

Quoted 20 lines

Thanx, Paul

Quoted 16 lines

					<return to user space>                        |    |
					read Y; --------------------------------------+----+
	store X;			                                              |
					{read X}(reordered) <-------------------------+

I assume the timer interrupt handler, which interrupts a user space and reports a quiesent state for sched flavor RCU, may not have a smp_mb() in some code path.

I may miss something subtle, but it just not very obvious how synchronize_sched() will guarantee a remote CPU running in userspace to do a smp_mb() before it returns, this is at least not in RCU requirements, right?

Regards,
Boqun

Quoted 44 lines · Boqun Feng

Quoted 32 lines · Paul E. McKenney

Quoted 17 lines · Boqun Feng

Hi Paul,

I have a side question out of curiosity:

How does synchronize_sched() work properly for sys_membarrier()?

sys_membarrier() requires every other CPU does a smp_mb() before it returns, and I know synchronize_sched() will wait until all CPUs running a kernel thread do a context-switch, which has a smp_mb(). However, I believe sched flavor RCU treat CPU running a user thread as a quiesent state, so synchronize_sched() could return without that CPU does a context switch.

So why could we use synchronize_sched() for sys_membarrier()?

In particular, could the following happens?

Quoted 13 lines

CPU 0: CPU 1:
========================= ==========================
<in user space> <in user space>
{read Y}(reordered) <------------------------------+
store Y; |
read X; --------------------------------------+ |
sys_membarrier(): <timer interrupt> | |
synchronize_sched(); update_process_times(user): //user == true | |
rcu_check_callbacks(usr): | |
if (user || ..) { | |
rcu_sched_qs() | |
... | |
<report quesient state in softirq> | |

Quoted 7 lines

The reporting of the quiescent state will acquire the leaf rcu_node structure's lock, with an smp_mb__after_unlock_lock(), which will one way or another be a full memory barrier. So the reorderings cannot happen.

Unless I am missing something subtle. ;-)

Well, smp_mb__after_unlock_lock() in ARM64 is a no-op, and ARM64's lock doesn't provide a smp_mb().

Quoted 5 lines

So my point is more like: synchronize_sched() happens to be a sys_membarrier() because of some implementation detail, and if some day we come up with a much cheaper way to implement sched flavor RCU(hopefully!), synchronize_sched() may be not good for the job. So at least, we'd better document this somewhere?

Last I heard, ARM's unlock/lock acted as a full barrier. Will?

Quoted 9 lines

Quoted 8 lines

The reporting of the quiescent state will acquire the leaf rcu_node structure's lock, with an smp_mb__after_unlock_lock(), which will one way or another be a full memory barrier. So the reorderings cannot happen.

Unless I am missing something subtle. ;-)

Quoted 8 lines

Well, smp_mb__after_unlock_lock() in ARM64 is a no-op, and ARM64's lock doesn't provide a smp_mb().

So my point is more like: synchronize_sched() happens to be a sys_membarrier() because of some implementation detail, and if some day we come up with a much cheaper way to implement sched flavor RCU(hopefully!), synchronize_sched() may be not good for the job. So at least, we'd better document this somewhere?

Last I heard, ARM's unlock/lock acted as a full barrier. Will?

Please see the synchronize_sched() comment header for the documentation you are asking for. And the "Memory-Barrier Guarantees" section of Documentation/RCU/Design/Requirements/Requirements.html.

Thanx, Paul

Quoted 23 lines

Regards,
Boqun

Quoted 20 lines

Thanx, Paul

Quoted 16 lines

					<return to user space>                        |    |
					read Y; --------------------------------------+----+
	store X;			                                              |
					{read X}(reordered) <-------------------------+

I assume the timer interrupt handler, which interrupts a user space and reports a quiesent state for sched flavor RCU, may not have a smp_mb() in some code path.

I may miss something subtle, but it just not very obvious how synchronize_sched() will guarantee a remote CPU running in userspace to do a smp_mb() before it returns, this is at least not in RCU requirements, right?

Regards,
Boqun

Quoted 18 lines · Paul E. McKenney

Quoted 9 lines

Quoted 8 lines

The reporting of the quiescent state will acquire the leaf rcu_node structure's lock, with an smp_mb__after_unlock_lock(), which will one way or another be a full memory barrier. So the reorderings cannot happen.

Unless I am missing something subtle. ;-)

Quoted 8 lines

Well, smp_mb__after_unlock_lock() in ARM64 is a no-op, and ARM64's lock doesn't provide a smp_mb().

So my point is more like: synchronize_sched() happens to be a sys_membarrier() because of some implementation detail, and if some day we come up with a much cheaper way to implement sched flavor RCU(hopefully!), synchronize_sched() may be not good for the job. So at least, we'd better document this somewhere?

Quoted 6 lines

Last I heard, ARM's unlock/lock acted as a full barrier. Will?

Please see the synchronize_sched() comment header for the documentation you are asking for. And the "Memory-Barrier Guarantees" section of Documentation/RCU/Design/Requirements/Requirements.html.

All those barrier guarantees are subject to a RCU read-side critical section with a synchonize_*(), IIRC, for example:

* On systems with more than one CPU, when synchronize_sched() returns,
* each CPU is guaranteed to have executed a full memory barrier since the
* end of its last RCU-sched read-side critical section whose beginning
* preceded the call to synchronize_sched(). In addition, each CPU having

, which is not the case for a quiesent state without a read-side critical section(i.e. non-context-switch quiesent state for sched Flavor)

I've read those requirements and could not find one to explain why there will be a full barrier emitted in an interrupted user-space program.

Regards,
Boqun

Quoted 27 lines

Thanx, Paul

Quoted 23 lines

Regards,
Boqun

Quoted 20 lines

Thanx, Paul

Quoted 16 lines

					<return to user space>                        |    |
					read Y; --------------------------------------+----+
	store X;			                                              |
					{read X}(reordered) <-------------------------+

I assume the timer interrupt handler, which interrupts a user space and reports a quiesent state for sched flavor RCU, may not have a smp_mb() in some code path.

I may miss something subtle, but it just not very obvious how synchronize_sched() will guarantee a remote CPU running in userspace to do a smp_mb() before it returns, this is at least not in RCU requirements, right?

Regards,
Boqun

Quoted 27 lines

So I think we need either switch_mm() or switch_to() to imply a full barrier for this to work, otherwise we get:

CPU0 CPU1

lock rq->lock
mb

rq->curr = A

unlock rq->lock

lock rq->lock
mb

				sys_membarrier()

				mb

				for_each_online_cpu()
				  p = A
				  // no match no IPI

				mb

rq->curr = B

Quoted 5 lines

unlock rq->lock

And that's bad, because now CPU0 doesn't have an MB happening _after_ sys_membarrier() if B matches.

Yes, this looks somewhat similar to the scenario that Mathieu pointed out back in 2010: https://marc.info/?l=linux-kernel&m=126349766324224&w=2

Quoted 5 lines

So without audit, I only know of PPC and Alpha not having a barrier in either switch_*().

x86 obviously has barriers all over the place, arm has a super duper heavy barrier in switch_to().

Agreed, if we are going to rely on ->mm, we need ordering on assignment to it.

Quoted 27 lines · Paul E. McKenney

Quoted 27 lines

So I think we need either switch_mm() or switch_to() to imply a full barrier for this to work, otherwise we get:

CPU0 CPU1

lock rq->lock
mb

rq->curr = A

unlock rq->lock

lock rq->lock
mb

				sys_membarrier()

				mb

				for_each_online_cpu()
				  p = A
				  // no match no IPI

				mb

rq->curr = B

Quoted 8 lines

Quoted 5 lines

unlock rq->lock

And that's bad, because now CPU0 doesn't have an MB happening _after_ sys_membarrier() if B matches.

Yes, this looks somewhat similar to the scenario that Mathieu pointed out back in 2010: https://marc.info/?l=linux-kernel&m=126349766324224&w=2

Yes. Minus the mm_cpumask() worries.

Quoted 8 lines

Quoted 5 lines

So without audit, I only know of PPC and Alpha not having a barrier in either switch_*().

x86 obviously has barriers all over the place, arm has a super duper heavy barrier in switch_to().

Agreed, if we are going to rely on ->mm, we need ordering on assignment to it.

Right, Boqun provided this reordering to show the problem:

CPU0 CPU1

<in process X>
lock rq->lock
mb

rq->curr = A

unlock rq->lock

<switch to process A>

lock rq->lock
mb
read Y(reordered)<---+

                       |        store to Y
                       |
                       |        sys_membarrier()
                       |
                       |        mb
                       |
                       |        for_each_online_cpu()
                       |          p = A
                       |          // no match no IPI
                       |
                       |        mb
                       |
                       |        store to X
  rq->curr = B         |
                       |
  unlock rq->lock      |
  <switch to B>        |
  read X               |
                       |

read Y --------------+

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Thoughts?

Quoted 7 lines · Paul E. McKenney

Some architectures are less precise than others in tracking which CPUs are running a given process due to ASIDs, though this is thought to be a non-problem:

	https://marc.info/?l=linux-arch&m=126716090413065&w=2
	https://marc.info/?l=linux-arch&m=126716262815202&w=2

Quoted 1 line

Thoughts?

On arm64, we *never* touch mm_cpumask, so it will always be empty. The only thing we could potentially use it for is non-broadcast TLB invalidation if the mm was only active on a single CPU, but when I implemented that we pretty much never hit that case. I was hoping fork()+exec() would trigger it (e.g. scripts), but the scheduler treats both of those as rebalancing points so the temporary ASID before the exec always has two CPUs set in its mask.

This horse is already out, so trying to shut the gate won't be effective.

Quoted 1 line · Paul E. McKenney

This horse is already out, so trying to shut the gate won't be effective.

So I'm not convinced it is. The mprotect() hack isn't portable as we've established and on x86 where it does work, it doesn't (much) perturb tasks not related to our process because we keep a tight mm_cpumask().

And if there are other (unpriv.) means of spraying IPIs around, we should most certainly look at fixing those, not just shrug and make matters worse.

Quoted 6 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

This horse is already out, so trying to shut the gate won't be effective.

So I'm not convinced it is. The mprotect() hack isn't portable as we've established and on x86 where it does work, it doesn't (much) perturb tasks not related to our process because we keep a tight mm_cpumask().

Wrong. People are using it today, portable or not. If we want them to stop using it, we need to give them an alternative. Period.

Quoted 10 lines · Paul E. McKenney

Quoted 6 lines · Peter Zijlstra

Quoted 1 line · Paul E. McKenney

This horse is already out, so trying to shut the gate won't be effective.

So I'm not convinced it is. The mprotect() hack isn't portable as we've established and on x86 where it does work, it doesn't (much) perturb tasks not related to our process because we keep a tight mm_cpumask().

Wrong. People are using it today, portable or not. If we want them to stop using it, we need to give them an alternative. Period.

What's wrong? The mprotect() hack isn't portable, nor does it perturb other users much.

I would much rather they use this than your synchronize_sched_expedited() thing. Its much better behaved. And they'll run into pain the moment they start using ARM,PPC,S390,etc..

From: Wanpeng Li <[email protected]>

 BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181

2017-07-25 5:58 GMT+08:00 Paul E. McKenney <[email protected]>:

Quoted 3 lines

From: Wanpeng Li <[email protected]>

 BUG: using smp_processor_id() in preemptible [00000000] code: 99-trinity/181

What's the meaning of EXP? Btw, the patch is in linus's tree currently.