2020-08-06 17:09:28

by Peter Oskolkov

[permalink] [raw]
Subject: [PATCH 1/2 v2] rseq/membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU

This patchset is based on Google-internal RSEQ
work done by Paul Turner and Andrew Hunter.

When working with per-CPU RSEQ-based memory allocations,
it is sometimes important to make sure that a global
memory location is no longer accessed from RSEQ critical
sections. For example, there can be two per-CPU lists,
one is "active" and accessed per-CPU, while another one
is inactive and worked on asynchronously "off CPU" (e.g.
garbage collection is performed). Then at some point
the two lists are swapped, and a fast RCU-like mechanism
is required to make sure that the previously active
list is no longer accessed.

This patch introduces such a mechanism: in short,
membarrier() syscall issues an IPI to a CPU, restarting
a potentially active RSEQ critical section on the CPU.

v1->v2:
- removed the ability to IPI all CPUs in a single sycall;
- use task->mm rather than task->group_leader to identify
tasks belonging to the same process.

The second patch in the patchset adds a selftest
of this feature.

Signed-off-by: Peter Oskolkov <[email protected]>
---
include/uapi/linux/membarrier.h | 5 ++++
kernel/sched/membarrier.c | 43 +++++++++++++++++++++++++++++++--
2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/membarrier.h b/include/uapi/linux/membarrier.h
index 5891d7614c8c..ce4628ea17fa 100644
--- a/include/uapi/linux/membarrier.h
+++ b/include/uapi/linux/membarrier.h
@@ -114,6 +114,10 @@
* If this command is not implemented by an
* architecture, -EINVAL is returned.
* Returns 0 on success.
+ * @MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU:
+ * If a thread belonging to the current process
+ * is currently in an RSEQ critical section on the
+ * CPU identified by flags parameter, restart it.
* @MEMBARRIER_CMD_SHARED:
* Alias to MEMBARRIER_CMD_GLOBAL. Provided for
* header backward compatibility.
@@ -131,6 +135,7 @@ enum membarrier_cmd {
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 << 4),
MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 5),
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 6),
+ MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU = (1 << 7),

/* Alias for header backward compatibility. */
MEMBARRIER_CMD_SHARED = MEMBARRIER_CMD_GLOBAL,
diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
index 168479a7d61b..1cdfde23696c 100644
--- a/kernel/sched/membarrier.c
+++ b/kernel/sched/membarrier.c
@@ -18,11 +18,19 @@
#define MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK 0
#endif

+#ifdef CONFIG_RSEQ
+#define MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU_BITMASK \
+ MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU
+#else
+#define MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU_BITMASK 0
+#endif
+
#define MEMBARRIER_CMD_BITMASK \
(MEMBARRIER_CMD_GLOBAL | MEMBARRIER_CMD_GLOBAL_EXPEDITED \
| MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED \
| MEMBARRIER_CMD_PRIVATE_EXPEDITED \
| MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED \
+ | MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU_BITMASK \
| MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK)

static void ipi_mb(void *info)
@@ -308,10 +316,39 @@ static int membarrier_register_private_expedited(int flags)
return 0;
}

+#ifdef CONFIG_RSEQ
+static void membarrier_rseq_ipi(void *arg)
+{
+ if (current->mm != arg) /* Not our process. */
+ return;
+ if (!current->rseq) /* RSEQ not set up for the current task/thread. */
+ return;
+
+ rseq_preempt(current);
+}
+#endif
+
+static int membarrier_private_restart_rseq_on_cpu(int cpu_id)
+{
+#ifdef CONFIG_RSEQ
+ /* syscalls are not allowed inside rseq critical sections. */
+ if (cpu_id == raw_smp_processor_id())
+ return 0;
+
+ return smp_call_function_single(cpu_id, membarrier_rseq_ipi,
+ current->mm, true);
+#else
+ return 0;
+#endif
+}
+
/**
* sys_membarrier - issue memory barriers on a set of threads
* @cmd: Takes command values defined in enum membarrier_cmd.
- * @flags: Currently needs to be 0. For future extensions.
+ * @flags: Currently needs to be 0 for all commands other than
+ * MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU: in the latter
+ * case it indicates the CPU on which to interrupt (= restart)
+ * the RSEQ critical section.
*
* If this system call is not implemented, -ENOSYS is returned. If the
* command specified does not exist, not available on the running
@@ -339,7 +376,7 @@ static int membarrier_register_private_expedited(int flags)
*/
SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
{
- if (unlikely(flags))
+ if (unlikely(flags && cmd != MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU))
return -EINVAL;
switch (cmd) {
case MEMBARRIER_CMD_QUERY:
@@ -369,6 +406,8 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE:
return membarrier_register_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
+ case MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU:
+ return membarrier_private_restart_rseq_on_cpu(flags);
default:
return -EINVAL;
}
--
2.28.0.163.g6104cc2f0b6-goog


2020-08-07 13:39:31

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] rseq/membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU

On Thu, Aug 06, 2020 at 10:05:43AM -0700, Peter Oskolkov wrote:
> +#ifdef CONFIG_RSEQ
> +static void membarrier_rseq_ipi(void *arg)
> +{
> + if (current->mm != arg) /* Not our process. */
> + return;
> + if (!current->rseq) /* RSEQ not set up for the current task/thread. */
> + return;
> +
> + rseq_preempt(current);
> +}
> +#endif
> +
> +static int membarrier_private_restart_rseq_on_cpu(int cpu_id)
> +{
> +#ifdef CONFIG_RSEQ
> + /* syscalls are not allowed inside rseq critical sections. */
> + if (cpu_id == raw_smp_processor_id())
> + return 0;
> +
> + return smp_call_function_single(cpu_id, membarrier_rseq_ipi,
> + current->mm, true);
> +#else
> + return 0;
> +#endif
> +}

I'm thinking even this is a problem, we can end up sending IPIs to CPUs
outside out partition (they might be NOHZ_FULL) and that's a no-no too.

Something like so perhaps... that really limits it to CPUs that match
our mm.

diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 6be66f52a2ad..bee5e98e6774 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -356,6 +356,7 @@ enum {

enum {
MEMBARRIER_FLAG_SYNC_CORE = (1U << 0),
+ MEMBARRIER_FLAG_RSEQ = (1U << 1),
};

#ifdef CONFIG_ARCH_HAS_MEMBARRIER_CALLBACKS
diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
index 168479a7d61b..4d9b22c2f5e2 100644
--- a/kernel/sched/membarrier.c
+++ b/kernel/sched/membarrier.c
@@ -27,6 +27,11 @@

static void ipi_mb(void *info)
{
+ int *flags = info;
+
+ if (flags && (*flags & MEMBARRIER_FLAG_RSEQ))
+ rseq_preempt(current);
+
smp_mb(); /* IPIs should be serializing but paranoid. */
}

@@ -129,11 +134,11 @@ static int membarrier_global_expedited(void)
return 0;
}

-static int membarrier_private_expedited(int flags)
+static int membarrier_private_expedited(int flags, int cpu_id)
{
- int cpu;
- cpumask_var_t tmpmask;
struct mm_struct *mm = current->mm;
+ cpumask_var_t tmpmask;
+ int cpu;

if (flags & MEMBARRIER_FLAG_SYNC_CORE) {
if (!IS_ENABLED(CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE))
@@ -174,6 +179,10 @@ static int membarrier_private_expedited(int flags)
*/
if (cpu == raw_smp_processor_id())
continue;
+
+ if (cpu_id >= 0 && cpu != cpu_id)
+ continue;
+
p = rcu_dereference(cpu_rq(cpu)->curr);
if (p && p->mm == mm)
__cpumask_set_cpu(cpu, tmpmask);
@@ -181,7 +190,7 @@ static int membarrier_private_expedited(int flags)
rcu_read_unlock();

preempt_disable();
- smp_call_function_many(tmpmask, ipi_mb, NULL, 1);
+ smp_call_function_many(tmpmask, ipi_mb, &flags, 1);
preempt_enable();

free_cpumask_var(tmpmask);
@@ -362,11 +371,13 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
case MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED:
return membarrier_register_global_expedited();
case MEMBARRIER_CMD_PRIVATE_EXPEDITED:
- return membarrier_private_expedited(0);
+ return membarrier_private_expedited(0, -1);
case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED:
return membarrier_register_private_expedited(0);
case MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE:
- return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
+ return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE, -1);
+ case MEMBERRIER_CMD_PRIVATE_EXPEDITED_RSEQ:
+ return membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ, flags);
case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE:
return membarrier_register_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
default:

2020-08-07 17:52:06

by Peter Oskolkov

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] rseq/membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU

On Fri, Aug 7, 2020 at 6:38 AM <[email protected]> wrote:
>

[...]

> I'm thinking even this is a problem, we can end up sending IPIs to CPUs
> outside out partition (they might be NOHZ_FULL) and that's a no-no too.
>
> Something like so perhaps... that really limits it to CPUs that match
> our mm.

Thanks for the suggestion - I'll prepare a v3 based on your and
Mathieu's feedback.

>
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 6be66f52a2ad..bee5e98e6774 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -356,6 +356,7 @@ enum {
>
> enum {
> MEMBARRIER_FLAG_SYNC_CORE = (1U << 0),
> + MEMBARRIER_FLAG_RSEQ = (1U << 1),
> };
>
> #ifdef CONFIG_ARCH_HAS_MEMBARRIER_CALLBACKS
> diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c
> index 168479a7d61b..4d9b22c2f5e2 100644
> --- a/kernel/sched/membarrier.c
> +++ b/kernel/sched/membarrier.c
> @@ -27,6 +27,11 @@
>
> static void ipi_mb(void *info)
> {
> + int *flags = info;
> +
> + if (flags && (*flags & MEMBARRIER_FLAG_RSEQ))
> + rseq_preempt(current);
> +
> smp_mb(); /* IPIs should be serializing but paranoid. */
> }
>
> @@ -129,11 +134,11 @@ static int membarrier_global_expedited(void)
> return 0;
> }
>
> -static int membarrier_private_expedited(int flags)
> +static int membarrier_private_expedited(int flags, int cpu_id)
> {
> - int cpu;
> - cpumask_var_t tmpmask;
> struct mm_struct *mm = current->mm;
> + cpumask_var_t tmpmask;
> + int cpu;
>
> if (flags & MEMBARRIER_FLAG_SYNC_CORE) {
> if (!IS_ENABLED(CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE))
> @@ -174,6 +179,10 @@ static int membarrier_private_expedited(int flags)
> */
> if (cpu == raw_smp_processor_id())
> continue;
> +
> + if (cpu_id >= 0 && cpu != cpu_id)
> + continue;
> +
> p = rcu_dereference(cpu_rq(cpu)->curr);
> if (p && p->mm == mm)
> __cpumask_set_cpu(cpu, tmpmask);
> @@ -181,7 +190,7 @@ static int membarrier_private_expedited(int flags)
> rcu_read_unlock();
>
> preempt_disable();
> - smp_call_function_many(tmpmask, ipi_mb, NULL, 1);
> + smp_call_function_many(tmpmask, ipi_mb, &flags, 1);
> preempt_enable();
>
> free_cpumask_var(tmpmask);
> @@ -362,11 +371,13 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
> case MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED:
> return membarrier_register_global_expedited();
> case MEMBARRIER_CMD_PRIVATE_EXPEDITED:
> - return membarrier_private_expedited(0);
> + return membarrier_private_expedited(0, -1);
> case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED:
> return membarrier_register_private_expedited(0);
> case MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE:
> - return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
> + return membarrier_private_expedited(MEMBARRIER_FLAG_SYNC_CORE, -1);
> + case MEMBERRIER_CMD_PRIVATE_EXPEDITED_RSEQ:
> + return membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ, flags);
> case MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE:
> return membarrier_register_private_expedited(MEMBARRIER_FLAG_SYNC_CORE);
> default: