Hi Ingo, Peter, Thomas,
Here is the updated membarrier patch series following review from
Andy.
It would be appreciated of those can go through the scheduler tree for
the 4.16 merge window.
Patch "x86: Implement sync_core_before_usermode" got a reviewed-by from
Thomas, but I removed the tag following KPTI-related optimizations based
on the discussion with Andy afterwards.
Highlights:
"powerpc: membarrier: Skip memory barrier in switch_mm()" takes care of
a TODO that was left in the private expedited implementation when merged
in 4.14: an extra memory barrier was added on context switch on powerpc.
Ensure that the barrier is only performed when scheduling between
different processes, only for threads belonging to processes that have
registered their intent to use the private expedited command.
"membarrier: provide GLOBAL_EXPEDITED command" adds new commands to
membarrier for registration and use of membarrier across processes
communicating through shared memory mappings. The non-expedited command
has proven to be really too slow (taking 10ms and more to complete) for
real-world use. The expedited version completes in a matter of
microseconds. This patch renames the pre-existing MEMBARRIER_CMD_SHARED
to MEMBARRIER_CMD_GLOBAL for consistency, keeping the old enum label
as an alias for backward compatibility.
"membarrier: Provide core serializing command" provides core
serialization for JIT reclaim. We received positive feedback from
Android developers that the proposed ABI fits their use-case.
Only x86 32/64 and arm 64 implement this command so far. This is
opt-in per architecture.
The other patches add selftests and documentation.
Thanks,
Mathieu
Mathieu Desnoyers (11):
membarrier: selftest: Test private expedited cmd (v2)
powerpc: membarrier: Skip memory barrier in switch_mm() (v7)
membarrier: Document scheduler barrier requirements (v5)
membarrier: provide GLOBAL_EXPEDITED command (v3)
membarrier: selftest: Test global expedited cmd (v2)
Introduce sync_core_before_usermode (v2)
x86: Implement sync_core_before_usermode (v3)
membarrier: Provide core serializing command (v2)
membarrier: x86: Provide core serializing command (v4)
membarrier: arm64: Provide core serializing command
membarrier: selftest: Test private expedited sync core cmd
MAINTAINERS | 1 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/entry.S | 4 +
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/membarrier.h | 27 +++
arch/powerpc/mm/mmu_context.c | 7 +
arch/x86/Kconfig | 2 +
arch/x86/entry/entry_32.S | 5 +
arch/x86/entry/entry_64.S | 4 +
arch/x86/include/asm/sync_core.h | 28 +++
arch/x86/mm/tlb.c | 6 +
include/linux/sched/mm.h | 40 +++-
include/linux/sync_core.h | 21 ++
include/uapi/linux/membarrier.h | 74 ++++++-
init/Kconfig | 9 +
kernel/sched/core.c | 53 +++--
kernel/sched/membarrier.c | 177 +++++++++++++--
.../testing/selftests/membarrier/membarrier_test.c | 237 +++++++++++++++++++--
18 files changed, 629 insertions(+), 68 deletions(-)
create mode 100644 arch/powerpc/include/asm/membarrier.h
create mode 100644 arch/x86/include/asm/sync_core.h
create mode 100644 include/linux/sync_core.h
--
2.11.0
Test the new MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE and
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE commands.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: Shuah Khan <redacted>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <redacted>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Will Deacon <redacted>
CC: Andy Lutomirski <luto@kernel.org>
CC: Alice Ferrazzi <redacted>
CC: Paul Elder <redacted>
CC: linux-kselftest@vger.kernel.org
CC: linux-arch@vger.kernel.org
---
.../testing/selftests/membarrier/membarrier_test.c | 73 ++++++++++++++++++++++
1 file changed, 73 insertions(+)
Allow PowerPC to skip the full memory barrier in switch_mm(), and
only issue the barrier when scheduling into a task belonging to a
process that has registered to use expedited private.
Threads targeting the same VM but which belong to different thread
groups is a tricky case. It has a few consequences:
It turns out that we cannot rely on get_nr_threads(p) to count the
number of threads using a VM. We can use
(atomic_read(&mm->mm_users) == 1 && get_nr_threads(p) == 1)
instead to skip the synchronize_sched() for cases where the VM only has
a single user, and that user only has a single thread.
It also turns out that we cannot use for_each_thread() to set
thread flags in all threads using a VM, as it only iterates on the
thread group.
Therefore, test the membarrier state variable directly rather than
relying on thread flags. This means
membarrier_register_private_expedited() needs to set the
MEMBARRIER_STATE_PRIVATE_EXPEDITED flag, issue synchronize_sched(), and
only then set MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY which allows
private expedited membarrier commands to succeed.
membarrier_arch_switch_mm() now tests for the
MEMBARRIER_STATE_PRIVATE_EXPEDITED flag.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Peter Zijlstra <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
CC: Will Deacon <redacted>
CC: Andy Lutomirski <redacted>
CC: Ingo Molnar <redacted>
CC: Alexander Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
CC: Nicholas Piggin <redacted>
CC: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
CC: linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Use test_ti_thread_flag(next, ...) instead of test_thread_flag() in
powerpc membarrier_arch_sched_in(), given that we want to specifically
check the next thread state.
- Add missing ARCH_HAS_MEMBARRIER_HOOKS in Kconfig.
- Use task_thread_info() to pass thread_info from task to
*_ti_thread_flag().
Changes since v2:
- Move membarrier_arch_sched_in() call to finish_task_switch().
- Check for NULL t->mm in membarrier_arch_fork().
- Use membarrier_sched_in() in generic code, which invokes the
arch-specific membarrier_arch_sched_in(). This fixes allnoconfig
build on PowerPC.
- Move asm/membarrier.h include under CONFIG_MEMBARRIER, fixing
allnoconfig build on PowerPC.
- Build and runtime tested on PowerPC.
Changes since v3:
- Simply rely on copy_mm() to copy the membarrier_private_expedited mm
field on fork.
- powerpc: test thread flag instead of reading
membarrier_private_expedited in membarrier_arch_fork().
- powerpc: skip memory barrier in membarrier_arch_sched_in() if coming
from kernel thread, since mmdrop() implies a full barrier.
- Set membarrier_private_expedited to 1 only after arch registration
code, thus eliminating a race where concurrent commands could succeed
when they should fail if issued concurrently with process
registration.
- Use READ_ONCE() for membarrier_private_expedited field access in
membarrier_private_expedited. Matches WRITE_ONCE() performed in
process registration.
Changes since v4:
- Move powerpc hook from sched_in() to switch_mm(), based on feedback
from Nicholas Piggin.
Changes since v5:
- Rebase on v4.14-rc6.
- Fold "Fix: membarrier: Handle CLONE_VM + !CLONE_THREAD correctly on
powerpc (v2)"
Changes since v6:
- Rename MEMBARRIER_STATE_SWITCH_MM to MEMBARRIER_STATE_PRIVATE_EXPEDITED.
---
MAINTAINERS | 1 +
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/membarrier.h | 26 ++++++++++++++++++++++++++
arch/powerpc/mm/mmu_context.c | 7 +++++++
include/linux/sched/mm.h | 13 ++++++++++++-
init/Kconfig | 3 +++
kernel/sched/core.c | 10 ----------
kernel/sched/membarrier.c | 8 ++++++++
8 files changed, 58 insertions(+), 11 deletions(-)
create mode 100644 arch/powerpc/include/asm/membarrier.h
Test the new MEMBARRIER_CMD_PRIVATE_EXPEDITED and
MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED commands.
Add checks expecting specific error values on system calls expected to
fail.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: Shuah Khan <redacted>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <redacted>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Will Deacon <redacted>
CC: Andy Lutomirski <luto@kernel.org>
CC: Alice Ferrazzi <redacted>
CC: Paul Elder <redacted>
CC: linux-kselftest@vger.kernel.org
CC: linux-arch@vger.kernel.org
---
Changes since v1:
- return result of ksft_exit_pass from main(), silencing compiler
warning about missing return value.
---
.../testing/selftests/membarrier/membarrier_test.c | 111 ++++++++++++++++++---
1 file changed, 95 insertions(+), 16 deletions(-)
Test the new MEMBARRIER_CMD_GLOBAL_EXPEDITED and
MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED commands.
Adapt to the MEMBARRIER_CMD_SHARED -> MEMBARRIER_CMD_GLOBAL rename.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
Acked-by: Shuah Khan <redacted>
CC: Greg Kroah-Hartman <redacted>
CC: Peter Zijlstra <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
CC: Will Deacon <redacted>
CC: Andy Lutomirski <redacted>
CC: Alice Ferrazzi <redacted>
CC: Paul Elder <redacted>
CC: linux-kselftest-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Rename SHARED to GLOBAL.
---
.../testing/selftests/membarrier/membarrier_test.c | 59 ++++++++++++++++++++--
1 file changed, 54 insertions(+), 5 deletions(-)
@@ -142,7 +176,7 @@ static int test_membarrier(void)status=test_membarrier_flags_fail();if(status)returnstatus;-status=test_membarrier_shared_success();+status=test_membarrier_global_success();if(status)returnstatus;status=test_membarrier_private_expedited_fail();
@@ -154,6 +188,19 @@ static int test_membarrier(void)status=test_membarrier_private_expedited_success();if(status)returnstatus;+/*+*Itisvalidtosendaglobalmembarrierfromanon-registered+*process.+*/+status=test_membarrier_global_expedited_success();+if(status)+returnstatus;+status=test_membarrier_register_global_expedited_success();+if(status)+returnstatus;+status=test_membarrier_global_expedited_success();+if(status)+returnstatus;return0;}
@@ -173,8 +220,10 @@ static int test_membarrier_query(void)}ksft_exit_fail_msg("sys_membarrier() failed\n");}-if(!(ret&MEMBARRIER_CMD_SHARED))+if(!(ret&MEMBARRIER_CMD_GLOBAL)){+ksft_test_result_fail("sys_membarrier() CMD_GLOBAL query failed\n");ksft_exit_fail_msg("sys_membarrier is not supported.\n");+}ksft_test_result_pass("sys_membarrier available\n");return0;
Allow expedited membarrier to be used for data shared between processes
through shared memory.
Processes wishing to receive the membarriers register with
MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED. Those which want to issue
membarrier invoke MEMBARRIER_CMD_GLOBAL_EXPEDITED.
This allows extremely simple kernel-level implementation: we have almost
everything we need with the PRIVATE_EXPEDITED barrier code. All we need
to do is to add a flag in the mm_struct that will be used to check
whether we need to send the IPI to the current thread of each CPU.
There is a slight downside to this approach compared to targeting
specific shared memory users: when performing a membarrier operation,
all registered "global" receivers will get the barrier, even if they
don't share a memory mapping with the sender issuing
MEMBARRIER_CMD_GLOBAL_EXPEDITED.
This registration approach seems to fit the requirement of not
disturbing processes that really deeply care about real-time: they
simply should not register with MEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED.
In order to align the membarrier command names, the "MEMBARRIER_CMD_SHARED"
command is renamed to "MEMBARRIER_CMD_GLOBAL", keeping an alias of
MEMBARRIER_CMD_SHARED to MEMBARRIER_CMD_GLOBAL for UAPI header backward
compatibility.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Peter Zijlstra <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <redacted>
CC: "H. Peter Anvin" <redacted>
CC: Andrea Parri <redacted>
CC: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
---
Changes since v1:
- Add missing preempt disable around smp_call_function_many().
Changes since v2:
- UAPI documentation fix based on Thomas Gleixner's feedback.
- Rename SHARED to GLOBAL.
---
arch/powerpc/include/asm/membarrier.h | 3 +-
include/linux/sched/mm.h | 6 +-
include/uapi/linux/membarrier.h | 42 ++++++++++--
kernel/sched/membarrier.c | 120 +++++++++++++++++++++++++++++++---
4 files changed, 153 insertions(+), 18 deletions(-)
@@ -35,6 +37,73 @@ static void ipi_mb(void *info)smp_mb();/* IPIs should be serializing but paranoid. */}+staticintmembarrier_global_expedited(void)+{+intcpu;+boolfallback=false;+cpumask_var_ttmpmask;++if(num_online_cpus()==1)+return0;++/*+*Matchesmemorybarriersaroundrq->currmodificationin+*scheduler.+*/+smp_mb();/* system call entry is not a mb. */++/*+*Expeditedmembarriercommandsguaranteethattheywon't+*block,hencetheGFP_NOWAITallocationflagandfallback+*implementation.+*/+if(!zalloc_cpumask_var(&tmpmask,GFP_NOWAIT)){+/* Fallback for OOM. */+fallback=true;+}++cpus_read_lock();+for_each_online_cpu(cpu){+structtask_struct*p;++/*+*SkippingthecurrentCPUisOKeventhroughwecanbe+*migratedatanypoint.ThecurrentCPU,atthepoint+*wherewereadraw_smp_processor_id(),isensuredto+*beinprogramorderwithrespecttothecaller+*thread.Therefore,wecanskipthisCPUfromthe+*iteration.+*/+if(cpu==raw_smp_processor_id())+continue;+rcu_read_lock();+p=task_rcu_dereference(&cpu_rq(cpu)->curr);+if(p&&p->mm&&(atomic_read(&p->mm->membarrier_state)&+MEMBARRIER_STATE_GLOBAL_EXPEDITED)){+if(!fallback)+__cpumask_set_cpu(cpu,tmpmask);+else+smp_call_function_single(cpu,ipi_mb,NULL,1);+}+rcu_read_unlock();+}+if(!fallback){+preempt_disable();+smp_call_function_many(tmpmask,ipi_mb,NULL,1);+preempt_enable();+free_cpumask_var(tmpmask);+}+cpus_read_unlock();++/*+*Memorybarrieronthecallerthread_after_wefinished+*waitingforthelastIPI.Matchesmemorybarriersaround+*rq->currmodificationinscheduler.+*/+smp_mb();/* exit from system call is not a mb */+return0;+}+staticintmembarrier_private_expedited(void){intcpu;
@@ -105,7 +174,38 @@ static int membarrier_private_expedited(void)return0;}-staticvoidmembarrier_register_private_expedited(void)+staticintmembarrier_register_global_expedited(void)+{+structtask_struct*p=current;+structmm_struct*mm=p->mm;++if(atomic_read(&mm->membarrier_state)&+MEMBARRIER_STATE_GLOBAL_EXPEDITED_READY)+return0;+atomic_or(MEMBARRIER_STATE_GLOBAL_EXPEDITED,&mm->membarrier_state);+if(atomic_read(&mm->mm_users)==1&&get_nr_threads(p)==1){+/*+*Forsinglemmuser,singlethreadedprocess,wecan+*simplyissueamemorybarrieraftersetting+*MEMBARRIER_STATE_GLOBAL_EXPEDITEDtoguaranteethat+*nomemoryaccessfollowingregistrationisreordered+*beforeregistration.+*/+smp_mb();+}else{+/*+*Formulti-mmuserthreads,weneedtoensureall+*futureschedulerexecutionswillobservethenew+*threadflagstateforthismm.+*/+synchronize_sched();+}+atomic_or(MEMBARRIER_STATE_GLOBAL_EXPEDITED_READY,+&mm->membarrier_state);+return0;+}++staticintmembarrier_register_private_expedited(void){structtask_struct*p=current;structmm_struct*mm=p->mm;
@@ -167,21 +268,24 @@ SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)intcmd_mask=MEMBARRIER_CMD_BITMASK;if(tick_nohz_full_enabled())-cmd_mask&=~MEMBARRIER_CMD_SHARED;+cmd_mask&=~MEMBARRIER_CMD_GLOBAL;returncmd_mask;}-caseMEMBARRIER_CMD_SHARED:-/* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */+caseMEMBARRIER_CMD_GLOBAL:+/* MEMBARRIER_CMD_GLOBAL is not compatible with nohz_full. */if(tick_nohz_full_enabled())return-EINVAL;if(num_online_cpus()>1)synchronize_sched();return0;+caseMEMBARRIER_CMD_GLOBAL_EXPEDITED:+returnmembarrier_global_expedited();+caseMEMBARRIER_CMD_REGISTER_GLOBAL_EXPEDITED:+returnmembarrier_register_global_expedited();caseMEMBARRIER_CMD_PRIVATE_EXPEDITED:returnmembarrier_private_expedited();caseMEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED:-membarrier_register_private_expedited();-return0;+returnmembarrier_register_private_expedited();default:return-EINVAL;}
Document the membarrier requirement on having a full memory barrier in
__schedule() after coming from user-space, before storing to rq->curr.
It is provided by smp_mb__after_spinlock() in __schedule().
Document that membarrier requires a full barrier on transition from
kernel thread to userspace thread. We currently have an implicit barrier
from atomic_dec_and_test() in mmdrop() that ensures this.
The x86 switch_mm_irqs_off() full barrier is currently provided by many
cpumask update operations as well as write_cr3(). Document that
write_cr3() provides this barrier.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <redacted>
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andrea Parri <parri.andrea@gmail.com>
CC: x86@kernel.org
---
Changes since v1:
- Update comments to match reality for code paths which are after
storing to rq->curr, before returning to user-space, based on feedback
from Andrea Parri.
Changes since v2:
- Update changelog (smp_mb__before_spinlock -> smp_mb__after_spinlock).
Based on feedback from Andrea Parri.
Changes since v3:
- Clarify comments following feeback from Peter Zijlstra.
Changes since v4:
- Update comment regarding powerpc barrier.
---
arch/x86/mm/tlb.c | 5 +++++
include/linux/sched/mm.h | 5 +++++
kernel/sched/core.c | 37 ++++++++++++++++++++++++++-----------
3 files changed, 36 insertions(+), 11 deletions(-)
There are two places where core serialization is needed by membarrier:
1) When returning from the membarrier IPI,
2) After scheduler updates curr to a thread with a different mm, before
going back to user-space, since the curr->mm is used by membarrier to
check whether it needs to send an IPI to that CPU.
x86-32 uses iret as return from interrupt, and both iret and sysexit to go
back to user-space. The iret instruction is core serializing, but not
sysexit.
x86-64 uses iret as return from interrupt, which takes care of the IPI.
However, it can return to user-space through either sysretl (compat
code), sysretq, or iret. Given that sysret{l,q} is not core serializing,
we rely instead on write_cr3() performed by switch_mm() to provide core
serialization after changing the current mm, and deal with the special
case of kthread -> uthread (temporarily keeping current mm into
active_mm) by adding a sync_core() in that specific case.
Use the new sync_core_before_usermode() to guarantee this.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Peter Zijlstra <redacted>
CC: Andy Lutomirski <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <redacted>
CC: "H. Peter Anvin" <redacted>
CC: Andrea Parri <redacted>
CC: Russell King <redacted>
CC: Greg Hackmann <redacted>
CC: Will Deacon <redacted>
CC: David Sehr <redacted>
CC: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
CC: linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Use the newly introduced sync_core_before_usermode(). Move all state
handling to generic code.
- Add linux/processor.h include to include/linux/sched/mm.h.
Changes since v2:
- Fix use-after-free in membarrier_mm_sync_core_before_usermode.
Changes since v3:
- Move generic code into separate patch.
---
arch/x86/Kconfig | 1 +
arch/x86/entry/entry_32.S | 5 +++++
arch/x86/entry/entry_64.S | 4 ++++
arch/x86/mm/tlb.c | 7 ++++---
4 files changed, 14 insertions(+), 3 deletions(-)
Introduce an architecture function that ensures the current CPU
issues a core serializing instruction before returning to usermode.
This is needed for the membarrier "sync_core" command.
Architectures defining the sync_core_before_usermode() static inline
need to select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <redacted>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@kernel.org>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andrea Parri <parri.andrea@gmail.com>
CC: Russell King <linux@armlinux.org.uk>
CC: Greg Hackmann <redacted>
CC: Will Deacon <redacted>
CC: David Sehr <redacted>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Arnd Bergmann <arnd@arndb.de>
CC: x86@kernel.org
CC: linux-arch@vger.kernel.org
---
Changes since v1:
- Introduce include/linux/sync_core.h
---
include/linux/sync_core.h | 21 +++++++++++++++++++++
init/Kconfig | 3 +++
2 files changed, 24 insertions(+)
create mode 100644 include/linux/sync_core.h
Ensure that a core serializing instruction is issued before returning to
user-mode. x86 implements return to user-space through sysexit, sysrel,
and sysretq, which are not core serializing.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Thomas Gleixner <redacted>
CC: Peter Zijlstra <redacted>
CC: Andy Lutomirski <redacted>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <avi-VrcmuVmyx1hWk0Htik3J/w@public.gmane.org>
CC: Benjamin Herrenschmidt <redacted>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
CC: Dave Watson <redacted>
CC: Ingo Molnar <redacted>
CC: "H. Peter Anvin" <redacted>
CC: Andrea Parri <redacted>
CC: Russell King <redacted>
CC: Greg Hackmann <redacted>
CC: Will Deacon <redacted>
CC: David Sehr <redacted>
CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Arnd Bergmann <redacted>
CC: x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
CC: linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Fix prototype of sync_core_before_usermode in generic code (missing
return type).
- Add linux/processor.h include to sched/core.c.
- Add ARCH_HAS_SYNC_CORE_BEFORE_USERMODE to init/Kconfig.
- Fix linux/processor.h ifdef to target
CONFIG_ARCH_HAS_SYNC_CORE_BEFORE_USERMODE rather than
ARCH_HAS_SYNC_CORE_BEFORE_USERMODE.
- Move empty static inline in processor.h to generic patch.
Changes since v2:
- Introduce arch/x86/include/asm/sync_core.h
- Don't sync_core when KPTI is enabled, and when invoked from irq and nmi
context.
- Note: v2 was reviewed by Thomas Gleixner, but changes were introduced
since.
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/sync_core.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/x86/include/asm/sync_core.h
Provide core serializing membarrier command to support memory reclaim
by JIT.
Each architecture needs to explicitly opt into that support by
documenting in their architecture code how they provide the core
serializing instructions required when returning from the membarrier
IPI, and after the scheduler has updated the curr->mm pointer (before
going back to user-space). They should then select
ARCH_HAS_MEMBARRIER_SYNC_CORE to enable support for that command on
their architecture.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@kernel.org>
CC: Paul E. McKenney <redacted>
CC: Boqun Feng <redacted>
CC: Andrew Hunter <redacted>
CC: Maged Michael <redacted>
CC: Avi Kivity <redacted>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <redacted>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Dave Watson <redacted>
CC: Thomas Gleixner <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andrea Parri <parri.andrea@gmail.com>
CC: Russell King <linux@armlinux.org.uk>
CC: Greg Hackmann <redacted>
CC: Will Deacon <redacted>
CC: David Sehr <redacted>
CC: linux-arch@vger.kernel.org
---
Changes since v1:
- Include linux/sync_core.h
---
include/linux/sched/mm.h | 18 ++++++++++++++
include/uapi/linux/membarrier.h | 32 ++++++++++++++++++++++++-
init/Kconfig | 3 +++
kernel/sched/core.c | 6 ++++-
kernel/sched/membarrier.c | 53 +++++++++++++++++++++++++++++++----------
5 files changed, 98 insertions(+), 14 deletions(-)
*confused*, when we switch from process A to process B, context_switch()
will not set rq->prev_mm and the above mm will be NULL and we'll not
pass through your_function_names_are_waaay_too_long and we'll not get
cookies.
And if there's anything more complicated going on, the comment/changelog
are not adequate.
if (unlikely(prev_state == TASK_DEAD)) {
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
*confused*, when we switch from process A to process B, context_switch()
will not set rq->prev_mm and the above mm will be NULL and we'll not
pass through your_function_names_are_waaay_too_long and we'll not get
cookies.
And if there's anything more complicated going on, the comment/changelog
are not adequate.
Aaah, its the case where we do not pass through switch_mm(), the partial
comment got to me. I only realized after reading the next patch.
quoted
if (unlikely(prev_state == TASK_DEAD)) {
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
*prev)
* thread, mmdrop()'s implicit full barrier is required by the
* membarrier system call, because the current active_mm can
* become the current mm without going through switch_mm().
+ * membarrier also requires a core serializing instruction
+ * before going back to user-space after storing to rq->curr.
*/
- if (mm)
+ if (mm) {
+ membarrier_mm_sync_core_before_usermode(mm);
mmdrop(mm);
+ }
*confused*, when we switch from process A to process B, context_switch()
will not set rq->prev_mm and the above mm will be NULL and we'll not
pass through your_function_names_are_waaay_too_long and we'll not get
cookies.
And if there's anything more complicated going on, the comment/changelog
are not adequate.
Aaah, its the case where we do not pass through switch_mm(), the partial
comment got to me. I only realized after reading the next patch.
Indeed, if we read the entire comment, it's made clear that this case is for
when switch_mm is not invoked, where the current mm is changed without going
through switch_mm(), when scheduling between uthread->kthread->uthread for
instance.
/*
* When transitioning from a kernel thread to a userspace
* thread, mmdrop()'s implicit full barrier is required by the
* membarrier system call, because the current active_mm can
* become the current mm without going through switch_mm().
* membarrier also requires a core serializing instruction
* before going back to user-space after storing to rq->curr.
*/
Is there something I should improve in the wording of this added
sentence to make it clearer ?
Thanks,
Mathieu
quoted
quoted
if (unlikely(prev_state == TASK_DEAD)) {
if (prev->sched_class->task_dead)
prev->sched_class->task_dead(prev);
From: Peter Zijlstra <peterz@infradead.org> Date: 2018-01-29 19:09:54
On Mon, Jan 29, 2018 at 06:36:05PM +0000, Mathieu Desnoyers wrote:
----- On Jan 29, 2018, at 1:15 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
quoted
Aaah, its the case where we do not pass through switch_mm(), the partial
comment got to me. I only realized after reading the next patch.
Indeed, if we read the entire comment, it's made clear that this case is for
when switch_mm is not invoked, where the current mm is changed without going
through switch_mm(), when scheduling between uthread->kthread->uthread for
instance.
/*
* When transitioning from a kernel thread to a userspace
* thread, mmdrop()'s implicit full barrier is required by the
* membarrier system call, because the current active_mm can
* become the current mm without going through switch_mm().
* membarrier also requires a core serializing instruction
* before going back to user-space after storing to rq->curr.
*/
Is there something I should improve in the wording of this added
sentence to make it clearer ?
Can be improved I think, its got two unqualified "membarrier"s in and
its a bit mixed up. I'm having a major case of the mondays (brain just
won't start today), but maybe something like:
When we switched through a kernel thread, the loop in
membarrier_{private,global}_expedited() can have observed that
kernel thread and not issued an IPI. We will also not pass
through switch_mm(). Membarrier requires a barrier after writing
rq->curr and returning to userspace, so provide them here:
- a full memory barrier for {PRIVATE,GLOBAL}_EXPEDITED
- a sync_core for SYNC_CORE
Also I think changing the changlog to state where we need core-sync
would be good. Currently the x86 patch does that, but not this one,
while this introduces the feature.
----- On Jan 29, 2018, at 2:09 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
On Mon, Jan 29, 2018 at 06:36:05PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Jan 29, 2018, at 1:15 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
quoted
quoted
Aaah, its the case where we do not pass through switch_mm(), the partial
comment got to me. I only realized after reading the next patch.
Indeed, if we read the entire comment, it's made clear that this case is for
when switch_mm is not invoked, where the current mm is changed without going
through switch_mm(), when scheduling between uthread->kthread->uthread for
instance.
/*
* When transitioning from a kernel thread to a userspace
* thread, mmdrop()'s implicit full barrier is required by the
* membarrier system call, because the current active_mm can
* become the current mm without going through switch_mm().
* membarrier also requires a core serializing instruction
* before going back to user-space after storing to rq->curr.
*/
Is there something I should improve in the wording of this added
sentence to make it clearer ?
Can be improved I think, its got two unqualified "membarrier"s in and
its a bit mixed up. I'm having a major case of the mondays (brain just
won't start today), but maybe something like:
When we switched through a kernel thread, the loop in
membarrier_{private,global}_expedited() can have observed that
kernel thread and not issued an IPI. We will also not pass
through switch_mm(). Membarrier requires a barrier after writing
rq->curr and returning to userspace, so provide them here:
- a full memory barrier for {PRIVATE,GLOBAL}_EXPEDITED
- a sync_core for SYNC_CORE
Editing to remove use of "we" and clarify, which ends up as:
/*
* When switching through a kernel thread, the loop in
* membarrier_{private,global}_expedited() may have observed that
* kernel thread and not issued an IPI. It is therefore possible to
* schedule between user->kernel->user threads without passing though
* switch_mm(). Membarrier requires a barrier after storing to
* rq->curr, before returning to userspace, so provide them here:
*
* - a full memory barrier for {PRIVATE,GLOBAL}_EXPEDITED, implicitly
* provided by mmdrop(),
* - a sync_core for SYNC_CORE.
*/
Also I think changing the changlog to state where we need core-sync
would be good. Currently the x86 patch does that, but not this one,
while this introduces the feature.
Planning to add this:
Architectures selecting this feature need to either document that
they issue core serializing instructions when returning to user-space,
or implement their architecture-specific sync_core_before_usermode().
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Hi Ingo, Hi Peter,
Please let me know if you find anything that prevents you from integrating
this patchset into the scheduler tree.
Thanks,
Mathieu
----- On Jan 23, 2018, at 10:57 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
Hi Ingo, Peter, Thomas,
Here is the updated membarrier patch series following review from
Andy.
It would be appreciated of those can go through the scheduler tree for
the 4.16 merge window.
Patch "x86: Implement sync_core_before_usermode" got a reviewed-by from
Thomas, but I removed the tag following KPTI-related optimizations based
on the discussion with Andy afterwards.
Highlights:
"powerpc: membarrier: Skip memory barrier in switch_mm()" takes care of
a TODO that was left in the private expedited implementation when merged
in 4.14: an extra memory barrier was added on context switch on powerpc.
Ensure that the barrier is only performed when scheduling between
different processes, only for threads belonging to processes that have
registered their intent to use the private expedited command.
"membarrier: provide GLOBAL_EXPEDITED command" adds new commands to
membarrier for registration and use of membarrier across processes
communicating through shared memory mappings. The non-expedited command
has proven to be really too slow (taking 10ms and more to complete) for
real-world use. The expedited version completes in a matter of
microseconds. This patch renames the pre-existing MEMBARRIER_CMD_SHARED
to MEMBARRIER_CMD_GLOBAL for consistency, keeping the old enum label
as an alias for backward compatibility.
"membarrier: Provide core serializing command" provides core
serialization for JIT reclaim. We received positive feedback from
Android developers that the proposed ABI fits their use-case.
Only x86 32/64 and arm 64 implement this command so far. This is
opt-in per architecture.
The other patches add selftests and documentation.
Thanks,
Mathieu
Mathieu Desnoyers (11):
membarrier: selftest: Test private expedited cmd (v2)
powerpc: membarrier: Skip memory barrier in switch_mm() (v7)
membarrier: Document scheduler barrier requirements (v5)
membarrier: provide GLOBAL_EXPEDITED command (v3)
membarrier: selftest: Test global expedited cmd (v2)
Introduce sync_core_before_usermode (v2)
x86: Implement sync_core_before_usermode (v3)
membarrier: Provide core serializing command (v2)
membarrier: x86: Provide core serializing command (v4)
membarrier: arm64: Provide core serializing command
membarrier: selftest: Test private expedited sync core cmd
MAINTAINERS | 1 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/entry.S | 4 +
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/membarrier.h | 27 +++
arch/powerpc/mm/mmu_context.c | 7 +
arch/x86/Kconfig | 2 +
arch/x86/entry/entry_32.S | 5 +
arch/x86/entry/entry_64.S | 4 +
arch/x86/include/asm/sync_core.h | 28 +++
arch/x86/mm/tlb.c | 6 +
include/linux/sched/mm.h | 40 +++-
include/linux/sync_core.h | 21 ++
include/uapi/linux/membarrier.h | 74 ++++++-
init/Kconfig | 9 +
kernel/sched/core.c | 53 +++--
kernel/sched/membarrier.c | 177 +++++++++++++--
.../testing/selftests/membarrier/membarrier_test.c | 237 +++++++++++++++++++--
18 files changed, 629 insertions(+), 68 deletions(-)
create mode 100644 arch/powerpc/include/asm/membarrier.h
create mode 100644 arch/x86/include/asm/sync_core.h
create mode 100644 include/linux/sync_core.h
--
2.11.0
Ingo/Peter/Thomas,
can you comment on this? I'd like to get this moving one way or
another, rather than just hold it back forever.
I _think_ all the concerns were resolved, no?
Linus
On Tue, Jan 23, 2018 at 7:57 AM, Mathieu Desnoyers
[off-list ref] wrote:
Hi Ingo, Peter, Thomas,
Here is the updated membarrier patch series following review from
Andy.
It would be appreciated of those can go through the scheduler tree for
the 4.16 merge window.
Ingo/Peter/Thomas,
can you comment on this? I'd like to get this moving one way or
another, rather than just hold it back forever.
I _think_ all the concerns were resolved, no?
Yeah, I wanted to apply these bits later today with some minor edits - unless you
are going to pull it directly.
Thanks,
Ingo
On Mon, Feb 5, 2018 at 8:33 AM, Ingo Molnar [off-list ref] wrote:
Yeah, I wanted to apply these bits later today with some minor edits - unless you
are going to pull it directly.
I have not pulled yet, I wanted to check. But I do want to get it merged.
It gets easier for me if I can just pull.
Mathieu, do you have a git tree? I don't think I've pulled from you,
so in that sense it's more convenient for me if it goes through Ingo.
But if you have a git tree already set up, just holler.
Linus
On Mon, Feb 5, 2018 at 8:33 AM, Ingo Molnar [off-list ref] wrote:
quoted
Yeah, I wanted to apply these bits later today with some minor edits - unless you
are going to pull it directly.
I have not pulled yet, I wanted to check. But I do want to get it merged.
It gets easier for me if I can just pull.
Yes, I'll apply them - and I'd feel better if this went through the scheduler
tree, because there's been significant review feedback both from PeterZ and from
Thomas.
If it passes all testing then I'll send it to you tomorrow at the latest.
Thanks,
Ingo
----- On Feb 5, 2018, at 12:23 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
On Mon, Feb 5, 2018 at 8:33 AM, Ingo Molnar [off-list ref] wrote:
quoted
Yeah, I wanted to apply these bits later today with some minor edits - unless
you
are going to pull it directly.
I have not pulled yet, I wanted to check. But I do want to get it merged.
It gets easier for me if I can just pull.
Mathieu, do you have a git tree? I don't think I've pulled from you,
so in that sense it's more convenient for me if it goes through Ingo.
But if you have a git tree already set up, just holler.
I indeed have a git tree setup. The URL is in the pull request I sent you this
morning.
But I favor letting it go through Ingo's scheduler tree and benefit from the extra
bit of automated testing this could add.
Thanks to you both,
Mathieu
----- On Feb 5, 2018, at 12:23 PM, Linus Torvalds torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org wrote:
quoted
On Mon, Feb 5, 2018 at 8:33 AM, Ingo Molnar [off-list ref] wrote:
quoted
Yeah, I wanted to apply these bits later today with some minor edits - unless
you
are going to pull it directly.
I have not pulled yet, I wanted to check. But I do want to get it merged.
It gets easier for me if I can just pull.
Mathieu, do you have a git tree? I don't think I've pulled from you,
so in that sense it's more convenient for me if it goes through Ingo.
But if you have a git tree already set up, just holler.
I indeed have a git tree setup. The URL is in the pull request I sent you this
morning.
But I favor letting it go through Ingo's scheduler tree and benefit from the extra
bit of automated testing this could add.
Sorry about the delays, I will handle your patches with priority tomorrow!
Thanks,
Ingo