Hi,
Here is a patchset implementing a cache for the CPU number of the
currently running thread in user-space.
Benchmarks comparing this approach to a getcpu based on system call on
ARM show a 44x speedup. They show a 14x speedup on x86-64 compared to
executing lsl from a vDSO through glibc.
I'm added a man page in the changelog of patch 1/3, which shows an
example usage of this new system call.
This series is based on v4.5-rc5, submitted for Linux 4.6.
Feedback is welcome,
Thanks!
Mathieu
Mathieu Desnoyers (5):
getcpu_cache system call: cache CPU number of running thread
getcpu_cache: ARM resume notifier
getcpu_cache: wire up ARM system call
getcpu_cache: x86 32/64 resume notifier
getcpu_cache: wire up x86 32/64 system call
MAINTAINERS | 7 ++
arch/arm/include/uapi/asm/unistd.h | 1 +
arch/arm/kernel/calls.S | 3 +-
arch/arm/kernel/signal.c | 1 +
arch/x86/entry/common.c | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/exec.c | 1 +
include/linux/sched.h | 36 ++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/getcpu_cache.h | 42 +++++++++
init/Kconfig | 10 ++
kernel/Makefile | 1 +
kernel/fork.c | 4 +
kernel/getcpu_cache.c | 163 +++++++++++++++++++++++++++++++++
kernel/sched/sched.h | 1 +
kernel/sys_ni.c | 3 +
17 files changed, 276 insertions(+), 1 deletion(-)
create mode 100644 include/uapi/linux/getcpu_cache.h
create mode 100644 kernel/getcpu_cache.c
--
2.1.4
Call the getcpu_cache_handle_notify_resume() function on return to
userspace if TIF_NOTIFY_RESUME thread flag is set.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Russell King <redacted>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <redacted>
CC: Thomas Gleixner <redacted>
CC: Paul Turner <redacted>
CC: Andrew Hunter <redacted>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <redacted>
CC: Dave Watson <redacted>
CC: Chris Lameter <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: Ben Maurer <redacted>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <redacted>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: linux-api@vger.kernel.org
---
arch/arm/kernel/signal.c | 1 +
1 file changed, 1 insertion(+)
Expose a new system call allowing threads to register one userspace
memory area where to store the CPU number on which the calling thread is
running. Scheduler migration sets the TIF_NOTIFY_RESUME flag on the
current thread. Upon return to user-space, a notify-resume handler
updates the current CPU value within each registered user-space memory
area. User-space can then read the current CPU number directly from
memory.
This getcpu cache is an improvement over current mechanisms available to
read the current CPU number, which has the following benefits:
- 44x speedup on ARM vs system call through glibc,
- 14x speedup on x86 compared to calling glibc, which calls vdso
executing a "lsl" instruction,
- 11x speedup on x86 compared to inlined "lsl" instruction,
- Unlike vdso approaches, this cached value can be read from an inline
assembly, which makes it a useful building block for restartable
sequences.
- The getcpu cache approach is portable (e.g. ARM), which is not the
case for the lsl-based x86 vdso.
On x86, yet another possible approach would be to use the gs segment
selector to point to user-space per-cpu data. This approach performs
similarly to the getcpu cache, but it has two disadvantages: it is
not portable, and it is incompatible with existing applications already
using the gs segment selector for other purposes.
This approach is inspired by Paul Turner and Andrew Hunter's work
on percpu atomics, which lets the kernel handle restart of critical
sections. [1] [2]
Benchmarking various approaches for reading the current CPU number:
ARMv7 Processor rev 10 (v7l)
Machine model: Wandboard i.MX6 Quad Board
- Baseline (empty loop): 10.1 ns
- Read CPU from getcpu cache: 10.1 ns
- glibc 2.19-0ubuntu6.6 getcpu: 445.6 ns
- getcpu system call: 322.2 ns
x86-64 Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz:
- Baseline (empty loop): 1.0 ns
- Read CPU from getcpu cache: 1.0 ns
- Read using gs segment selector: 1.0 ns
- "lsl" inline assembly: 11.2 ns
- glibc 2.19-0ubuntu6.6 getcpu: 14.3 ns
- getcpu system call: 51.0 ns
[1] https://lwn.net/Articles/650333/
[2] http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
Link: http://lkml.kernel.org/r/20151027235635.16059.11630.stgit@pjt-glaptop.roam.corp.google.com
Link: http://lkml.kernel.org/r/20150624222609.6116.86035.stgit@kitami.mtv.corp.google.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Thomas Gleixner <redacted>
CC: Paul Turner <redacted>
CC: Andrew Hunter <redacted>
CC: Peter Zijlstra <redacted>
CC: Andy Lutomirski <redacted>
CC: Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org>
CC: Dave Watson <redacted>
CC: Chris Lameter <redacted>
CC: Ingo Molnar <redacted>
CC: "H. Peter Anvin" <redacted>
CC: Ben Maurer <redacted>
CC: Steven Rostedt <redacted>
CC: "Paul E. McKenney" <redacted>
CC: Josh Triplett <redacted>
CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
CC: Catalin Marinas <redacted>
CC: Will Deacon <redacted>
CC: Michael Kerrisk <redacted>
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes since v1:
- Return -1, errno=EINVAL if cpu_cache pointer is not aligned on
sizeof(int32_t).
- Update man page to describe the pointer alignement requirements and
update atomicity guarantees.
- Add MAINTAINERS file GETCPU_CACHE entry.
- Remove dynamic memory allocation: go back to having a single
getcpu_cache entry per thread. Update documentation accordingly.
- Rebased on Linux 4.4.
Changes since v2:
- Introduce a "cmd" argument, along with an enum with GETCPU_CACHE_GET
and GETCPU_CACHE_SET. Introduce a uapi header linux/getcpu_cache.h
defining this enumeration.
- Split resume notifier architecture implementation from the system call
wire up in the following arch-specific patches.
- Man pages updates.
- Handle 32-bit compat pointers.
- Simplify handling of getcpu_cache GETCPU_CACHE_SET compiler barrier:
set the current cpu cache pointer before doing the cache update, and
set it back to NULL if the update fails. Setting it back to NULL on
error ensures that no resume notifier will trigger a SIGSEGV if a
migration happened concurrently.
Changes since v3:
- Fix __user annotations in compat code,
- Update memory ordering comments.
- Rebased on kernel v4.5-rc5.
Rationale for the getcpu_cache system call rather than the thread-local
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this system
call for a cpu number cache only. Anyway, the thread-local ABI approach
would have required that we introduce "feature" flags, which would have
ended up reimplementing multiplexing of features on top of a system
call. It seems better to introduce one system call per feature instead.
Man page associated:
GETCPU_CACHE(2) Linux Programmer's Manual GETCPU_CACHE(2)
NAME
getcpu_cache - cache CPU number on which the calling thread
is running
SYNOPSIS
#include <linux/getcpu_cache.h>
#include <stdint.h>
int getcpu_cache(int cmd, int32_t **cpu_cachep, int flags);
DESCRIPTION
The getcpu_cache() helps speeding up reading the current CPU
number by ensuring that the memory location registered by
each user-space thread is always updated with the CPU number
on which the thread is running when reading that memory loca‐
tion.
The cmd argument is one of the following:
GETCPU_CACHE_GET
Get the pointer to the current cpu number cache into
the memory location targeted by the cpu_cachep
pointer.
GETCPU_CACHE_SET
Attempt to set the current cpu number cache by using
the pointer located in the memory location targeted by
the cpu_cachep pointer. This pointer must be aligned
on 4-byte multiples (natural alignment).
The cpu_cachep argument is a pointer to a int32_t pointer. It
is used as an output argument for GETCPU_CACHE_GET, and as an
input argument for GETCPU_CACHE_SET.
The flags argument is currently unused and must be specified
as 0.
Typically, a library or application will keep the cpu number
cache in a thread-local storage variable, or other memory
areas belonging to each thread. It is recommended to perform
a volatile read of the cpu number cache to prevent the com‐
piler from doing load tearing. An alternative approach is to
read the cpu number cache from inline assembly in a single
instruction.
Each thread is responsible for registering its own cpu number
cache. Only one cpu cache address can be registered per
thread.
The symbol __getcpu_cache_tls is recommended to be used
across libraries and applications wishing to register a
thread-local getcpu_cache. The attribute "weak" is recom‐
mended when declaring this variable in libraries. Applica‐
tions can choose to define their own version of this symbol
without the weak attribute as a performance improvement.
In a typical usage scenario, the thread registering the cpu
number cache will be performing reads from that cache. It is
however also allowed to read the cpu number cache from other
threads. The cpu number cache updates performed by the kernel
provide single-copy atomicity semantics, which guarantee that
other threads performing single-copy atomic reads of the cpu
number cache will always observe a consistent value.
Memory registered as cpu number cache should never be deallo‐
cated before the thread which registered it exits: specifi‐
cally, it should not be freed, and the library containing the
registered thread-local storage should not be dlclose'd.
Unregistration of associated cpu cache is implicitly per‐
formed when a thread or process exit.
RETURN VALUE
A return value of 0 indicates success. On error, -1 is
returned, and errno is set appropriately.
ERRORS
EINVAL Either flags is non-zero, an invalid cmd has been
specified, or the GETCPU_CACHE_GET command has been
specified and cpu_cachep points to a location contain‐
ing an invalid address, or cpu_cachep points to a
location containing an address which is not aligned on
4-byte multiples.
ENOSYS The getcpu_cache() system call is not implemented by
this kernel.
EFAULT cpu_cachep is an invalid address, or cpu_cachep points
to a location containing an invalid address.
EBUSY The GETCPU_CACHE_SET command has been specified, and a
cpu cache address which differs from the content of
the memory location pointed to by cpu_cachep is
already registered for this thread.
ENOENT The GETCPU_CACHE_GET command has been specified, but
no cpu cache has been registered for this thread.
VERSIONS
The getcpu_cache() system call was added in Linux 4.X (TODO).
CONFORMING TO
getcpu_cache() is Linux-specific.
EXAMPLE
The following code uses the getcpu_cache() system call to
keep a thread local storage variable up to date with the cur‐
rent CPU number, with a fallback on sched_getcpu(3) if the
cache is not available. For example simplicity, it is done in
main(), but multithreaded programs would need to invoke
getcpu_cache() from each program thread.
#define _GNU_SOURCE
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <stdint.h>
#include <sched.h>
#include <linux/getcpu_cache.h>
#include <sys/syscall.h>
static inline int
getcpu_cache(int cmd, volatile int32_t **cpu_cachep, int flags)
{
return syscall(__NR_getcpu_cache, cmd, cpu_cachep, flags);
}
/*
* __getcpu_cache_tls is recommended as symbol name for the
* cpu number cache. Weak attribute is recommended when
* declaring this variable in libraries. Applications can
* choose to define their own version of this symbol without
* the weak attribute and access it directly as a
* performance improvement when it matches the address
* returned by GETCPU_CACHE_GET. The initial value "-1"
* will be read in case the getcpu cache is not available.
*/
__thread __attribute__((weak)) volatile int32_t
__getcpu_cache_tls = -1;
int
main(int argc, char **argv)
{
volatile int32_t *cpu_cache = &__getcpu_cache_tls;
int32_t cpu;
/* Try to register the CPU cache. */
if (getcpu_cache(GETCPU_CACHE_SET, &cpu_cache, 0) < 0) {
perror("getcpu_cache set");
fprintf(stderr, "Using sched_getcpu() as fallback.\n");
}
cpu = __getcpu_cache_tls; /* Read current CPU number. */
if (cpu < 0) {
/* Fallback on sched_getcpu(). */
cpu = sched_getcpu();
}
printf("Current CPU number: %d\n", cpu);
exit(EXIT_SUCCESS);
}
SEE ALSO
sched_getcpu(3)
Linux 2016-01-27 GETCPU_CACHE(2)
---
MAINTAINERS | 7 ++
fs/exec.c | 1 +
include/linux/sched.h | 36 +++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/getcpu_cache.h | 42 ++++++++++
init/Kconfig | 10 +++
kernel/Makefile | 1 +
kernel/fork.c | 4 +
kernel/getcpu_cache.c | 163 ++++++++++++++++++++++++++++++++++++++
kernel/sched/sched.h | 1 +
kernel/sys_ni.c | 3 +
11 files changed, 269 insertions(+)
create mode 100644 include/uapi/linux/getcpu_cache.h
create mode 100644 kernel/getcpu_cache.c
@@ -1830,6 +1830,9 @@ struct task_struct {unsignedlongtask_state_change;#endifintpagefault_disabled;+#ifdef CONFIG_GETCPU_CACHE+int32_t__user*cpu_cache;+#endif/* CPU-specific state of this task */structthread_structthread;/*
@@ -3207,4 +3210,37 @@ static inline unsigned long rlimit_max(unsigned int limit)returntask_rlimit_max(current,limit);}+#ifdef CONFIG_GETCPU_CACHE+voidgetcpu_cache_fork(structtask_struct*t);+voidgetcpu_cache_execve(structtask_struct*t);+voidgetcpu_cache_exit(structtask_struct*t);+void__getcpu_cache_handle_notify_resume(structtask_struct*t);+staticinlinevoidgetcpu_cache_set_notify_resume(structtask_struct*t)+{+if(t->cpu_cache)+set_tsk_thread_flag(t,TIF_NOTIFY_RESUME);+}+staticinlinevoidgetcpu_cache_handle_notify_resume(structtask_struct*t)+{+if(t->cpu_cache)+__getcpu_cache_handle_notify_resume(t);+}+#else+staticinlinevoidgetcpu_cache_fork(structtask_struct*t)+{+}+staticinlinevoidgetcpu_cache_execve(structtask_struct*t)+{+}+staticinlinevoidgetcpu_cache_exit(structtask_struct*t)+{+}+staticinlinevoidgetcpu_cache_set_notify_resume(structtask_struct*t)+{+}+staticinlinevoidgetcpu_cache_handle_notify_resume(structtask_struct*t)+{+}+#endif+#endif
Wire up the getcpu cache system call on 32-bit ARM.
This provides an ABI improving the speed of a getcpu operation
on ARM by skipping the getcpu system call on the fast path.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Russell King <redacted>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <redacted>
CC: Thomas Gleixner <redacted>
CC: Paul Turner <redacted>
CC: Andrew Hunter <redacted>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <redacted>
CC: Dave Watson <redacted>
CC: Chris Lameter <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: Ben Maurer <redacted>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <redacted>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: linux-api@vger.kernel.org
---
arch/arm/include/uapi/asm/unistd.h | 1 +
arch/arm/kernel/calls.S | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
Wire up the getcpu_cache system call on x86 32/64.
This provides an ABI improving the speed of a getcpu operation
on x86 by removing the need to perform a function call, "lsl"
instruction, or system call on the fast path.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <redacted>
CC: Paul Turner <redacted>
CC: Andrew Hunter <redacted>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <redacted>
CC: Dave Watson <redacted>
CC: Chris Lameter <redacted>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <redacted>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <redacted>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: linux-api@vger.kernel.org
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
2 files changed, 2 insertions(+)
@@ -333,6 +333,7 @@ 324 common membarrier sys_membarrier 325 common mlock2 sys_mlock2 326 common copy_file_range sys_copy_file_range+326 common getcpu_cache sys_getcpu_cache # # x32-specific system call numbers start at 512 to avoid cache impact
Wire up the getcpu cache system call on 32-bit ARM.
This provides an ABI improving the speed of a getcpu operation
on ARM by skipping the getcpu system call on the fast path.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
CC: Catalin Marinas <redacted>
CC: Will Deacon <redacted>
CC: Thomas Gleixner <redacted>
CC: Paul Turner <redacted>
CC: Andrew Hunter <redacted>
CC: Peter Zijlstra <redacted>
CC: Andy Lutomirski <redacted>
CC: Andi Kleen <andi-Vw/NltI1exuRpAAqCnN02g@public.gmane.org>
CC: Dave Watson <redacted>
CC: Chris Lameter <redacted>
CC: Ingo Molnar <redacted>
CC: Ben Maurer <redacted>
CC: Steven Rostedt <redacted>
CC: "Paul E. McKenney" <redacted>
CC: Josh Triplett <redacted>
CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
arch/arm/include/asm/unistd.h | 2 +-
arch/arm/include/uapi/asm/unistd.h | 1 +
arch/arm/kernel/calls.S | 3 ++-
3 files changed, 4 insertions(+), 2 deletions(-)
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-02-24 01:37:55
On 02/23/2016 03:28 PM, Mathieu Desnoyers wrote:
Hi,
Here is a patchset implementing a cache for the CPU number of the
currently running thread in user-space.
Benchmarks comparing this approach to a getcpu based on system call on
ARM show a 44x speedup. They show a 14x speedup on x86-64 compared to
executing lsl from a vDSO through glibc.
I'm added a man page in the changelog of patch 1/3, which shows an
example usage of this new system call.
This series is based on v4.5-rc5, submitted for Linux 4.6.
Feedback is welcome,
What is the resulting context switch overhead?
-hpa
----- On Feb 23, 2016, at 8:36 PM, H. Peter Anvin hpa@zytor.com wrote:
On 02/23/2016 03:28 PM, Mathieu Desnoyers wrote:
quoted
Hi,
Here is a patchset implementing a cache for the CPU number of the
currently running thread in user-space.
Benchmarks comparing this approach to a getcpu based on system call on
ARM show a 44x speedup. They show a 14x speedup on x86-64 compared to
executing lsl from a vDSO through glibc.
I'm added a man page in the changelog of patch 1/3, which shows an
example usage of this new system call.
This series is based on v4.5-rc5, submitted for Linux 4.6.
Feedback is welcome,
What is the resulting context switch overhead?
The getcpu_cache only adds code to the thread migration path,
and to the resume notifier. The context switch path per se is
untouched. I would therefore expect the overhead on context
switch to be within the noise, except if stuff like hackbench
would be so sensitive to the size of struct task_struct that
a single extra pointer added at the end of struct task_struct
would throw off the benchmarks.
Is that what you are concerned about ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
arch/arm/kernel/entry-common.S:284: undefined reference to `sys_getcpu_cache'
vim +284 arch/arm/kernel/entry-common.S
^1da177e4 Linus Torvalds 2005-04-16 278 b ret_slow_syscall
^1da177e4 Linus Torvalds 2005-04-16 279
3302caddf Russell King 2015-08-20 280 __sys_trace_return_nosave:
e0aa3a665 Russell King 2015-08-20 281 enable_irq_notrace
3302caddf Russell King 2015-08-20 282 mov r0, sp
3302caddf Russell King 2015-08-20 283 bl syscall_trace_exit
3302caddf Russell King 2015-08-20 @284 b ret_slow_syscall
3302caddf Russell King 2015-08-20 285
^1da177e4 Linus Torvalds 2005-04-16 286 .align 5
^1da177e4 Linus Torvalds 2005-04-16 287 #ifdef CONFIG_ALIGNMENT_TRAP
:::::: The code at line 284 was first introduced by commit
:::::: 3302caddf10ad50710dbb7a94ccbdb3ad5bf1412 ARM: entry: efficiency cleanups
:::::: TO: Russell King [off-list ref]
:::::: CC: Russell King [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
arch/arm/kernel/entry-header.S:312: undefined reference to `sys_getcpu_cache'
vim +312 arch/arm/kernel/entry-header.S
a18f3645 Daniel Thompson 2015-01-09 306 ldmdb r2, {r0 - lr}^ @ get calling r0 - lr
b86040a5 Catalin Marinas 2009-07-24 307 .endif
8e4971f2 Anders Grafström 2010-03-15 308 mov r0, r0 @ ARMv5T and earlier require a nop
8e4971f2 Anders Grafström 2010-03-15 309 @ after ldm {}^
a18f3645 Daniel Thompson 2015-01-09 310 add sp, sp, #\offset + S_FRAME_SIZE
b86040a5 Catalin Marinas 2009-07-24 311 movs pc, lr @ return & move spsr_svc into cpsr
aa06e5c1 Russell King 2015-08-26 @312 #elif defined(CONFIG_CPU_V7M)
aa06e5c1 Russell King 2015-08-26 313 @ V7M restore.
aa06e5c1 Russell King 2015-08-26 314 @ Note that we don't need to do clrex here as clearing the local
aa06e5c1 Russell King 2015-08-26 315 @ monitor is part of the exception entry and exit sequence.
:::::: The code at line 312 was first introduced by commit
:::::: aa06e5c1f9c2b466712be904cc5b56a813e24cfd ARM: entry: get rid of multiple macro definitions
:::::: TO: Russell King [off-list ref]
:::::: CC: Russell King [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-02-24 20:20:59
On February 23, 2016 8:09:23 PM PST, Mathieu Desnoyers [off-list ref] wrote:
----- On Feb 23, 2016, at 8:36 PM, H. Peter Anvin hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org wrote:
quoted
On 02/23/2016 03:28 PM, Mathieu Desnoyers wrote:
quoted
Hi,
Here is a patchset implementing a cache for the CPU number of the
currently running thread in user-space.
Benchmarks comparing this approach to a getcpu based on system call
on
quoted
quoted
ARM show a 44x speedup. They show a 14x speedup on x86-64 compared
to
quoted
quoted
executing lsl from a vDSO through glibc.
I'm added a man page in the changelog of patch 1/3, which shows an
example usage of this new system call.
This series is based on v4.5-rc5, submitted for Linux 4.6.
Feedback is welcome,
What is the resulting context switch overhead?
The getcpu_cache only adds code to the thread migration path,
and to the resume notifier. The context switch path per se is
untouched. I would therefore expect the overhead on context
switch to be within the noise, except if stuff like hackbench
would be so sensitive to the size of struct task_struct that
a single extra pointer added at the end of struct task_struct
would throw off the benchmarks.
Is that what you are concerned about ?
Thanks,
Mathieu
Yes, I'd like to see numbers. It is way easy to handwave small changes away, but they add up over time. Without numbers it is a bit hard to quantify the pro vs con.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
----- On Feb 24, 2016, at 3:07 PM, H. Peter Anvin hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org wrote:
On February 23, 2016 8:09:23 PM PST, Mathieu Desnoyers
[off-list ref] wrote:
quoted
----- On Feb 23, 2016, at 8:36 PM, H. Peter Anvin hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org wrote:
quoted
On 02/23/2016 03:28 PM, Mathieu Desnoyers wrote:
quoted
Hi,
Here is a patchset implementing a cache for the CPU number of the
currently running thread in user-space.
Benchmarks comparing this approach to a getcpu based on system call
on
quoted
quoted
ARM show a 44x speedup. They show a 14x speedup on x86-64 compared
to
quoted
quoted
executing lsl from a vDSO through glibc.
I'm added a man page in the changelog of patch 1/3, which shows an
example usage of this new system call.
This series is based on v4.5-rc5, submitted for Linux 4.6.
Feedback is welcome,
What is the resulting context switch overhead?
The getcpu_cache only adds code to the thread migration path,
and to the resume notifier. The context switch path per se is
untouched. I would therefore expect the overhead on context
switch to be within the noise, except if stuff like hackbench
would be so sensitive to the size of struct task_struct that
a single extra pointer added at the end of struct task_struct
would throw off the benchmarks.
Is that what you are concerned about ?
Thanks,
Mathieu
Yes, I'd like to see numbers. It is way easy to handwave small changes away,
but they add up over time. Without numbers it is a bit hard to quantify the
pro vs con.
- Speed
Running 10 runs of hackbench -l 100000 on a 2 sockets * 8-core Intel(R) Xeon(R) CPU
E5-2630 v3 @ 2.40GHz (directly on hardware, no virtualization), with
hyperthreading, with a 4.5-rc5 defconfig+localyesconfig, getcpu_cache series
applied, seems to indicate that the sched switch impact of this new configuration
option is within the noise:
* CONFIG_GETCPU_CACHE=n
avg.: 26.63 s
std.dev.: 0.38 s
* CONFIG_GETCPU_CACHE=y
avg.: 26.52 s
std.dev.: 0.47 s
- Size
Between CONFIG_GETCPU_CACHE=n/y, the size delta added to the compressed kernel
zImage is 704 bytes. The text size increase of vmlinux is 512 bytes, and the data
size increase of vmlinux is also 512 bytes.
* CONFIG_GETCPU_CACHE=n
text data bss dec hex filename
16802349 2745968 1564672 21112989 142289d vmlinux
* CONFIG_GETCPU_CACHE=y
text data bss dec hex filename
16802861 2746480 1564672 21114013 1422c9d vmlinux
Am I missing anything ? I plan to add this information to the
changelog for my next round (v5).
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-25 09:57:12
On Tue, Feb 23, 2016 at 06:28:36PM -0500, Mathieu Desnoyers wrote:
This approach is inspired by Paul Turner and Andrew Hunter's work
on percpu atomics, which lets the kernel handle restart of critical
sections. [1] [2]
So I'd like a few extra words on the intersection with that work.
Yes, that also needs a CPU number, but that needs a little extra as
well. Can this work be extended to provide the little extra and is the
getcpu name still sane in that case?
Alternatively, could you not, at equal speed, get the CPU number from
the restartable sequence data?
That is, do explain why we want both.
(And remind Paul to keep pushing that)
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
On Tue, Feb 23, 2016 at 06:28:36PM -0500, Mathieu Desnoyers wrote:
quoted
This approach is inspired by Paul Turner and Andrew Hunter's work
on percpu atomics, which lets the kernel handle restart of critical
sections. [1] [2]
So I'd like a few extra words on the intersection with that work.
Yes, that also needs a CPU number, but that needs a little extra as
well. Can this work be extended to provide the little extra and is the
getcpu name still sane in that case?
Alternatively, could you not, at equal speed, get the CPU number from
the restartable sequence data?
That is, do explain why we want both.
Paul Turner's percpu atomics (restartable sequences) allow
turning atomic instructions (e.g. LOCK; cmpxchg on x86) meant
to update userspace per-cpu data into a sequence of instructions
that end with a single commit instruction. The primary use-case
for this is for implementing efficient memory allocators with
per-cpu memory pools (rather than global or per-thread pools).
This is made possible with the collaboration between kernel and
user-space, where user-space marks the surrounding of this "rseq"
critical section, and the kernel moves the instruction pointer
to a restart address (also published by user-space) if it
preempts/migrates/delivers a signal over that critical section.
The benefit of those restartable sequences over atomic instructions
is that it is much faster to execute a sequence of simple non-atomic
instructions (e.g. load, test, cond. branch, store) than a single
atomic instruction.
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
However, there are other use-cases for having a fast mechanism
for reading the current CPU number, besides restartable sequences.
For instance, it can be used by glibc to implement a faster
sched_getcpu. Therefore, implementing getcpu_cache as its own
system call makes sense: an architecture could very well just
introduce getcpu_cache even if it cannot support restartable
sequences for some reason. Also, a kernel configuration can
enable getcpu_cache (since it has no effect on the scheduler
switch time, only migration) without enabling restartable
sequences.
The main reason why I decided to start working on getcpu_cache
is because I noticed that the restartable sequences system
call originally proposed by Paul Turner was trying to accomplish
too much at once: both handling of restartable sequences, and
quickly reading the current CPU number. My thinking is that
the issue of reading the current CPU number could be completely
taken out of the rseq picture by having rseq rely on the
address registered by getcpu_cache to read the CPU number.
This would therefore simplify the implementation of rseq,
and allow us to focus the rseq review discussions without
being side-tracked on the simpler problem of quickly reading
the current CPU number.
(And remind Paul to keep pushing that)
Indeed, I look forward to Paul's feedback on my review of his
last patchset round. Hopefully this getcpu_cache work will
allow us to better focus the discussions on rseq work.
Is the explanation above OK for you ? I'll add it to the
Changelog in v5 of the getcpu_cache series if so.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-25 17:04:40
On Thu, Feb 25, 2016 at 04:55:26PM +0000, Mathieu Desnoyers wrote:
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
Yes yes, I know how restartable sequences work.
But what I worry about is that they want a cpu number and a sequence
number, and for performance it would be very good if those live in the
same cacheline.
That means either getcpu needs to grow a seq number, or restartable
sequences need to _also_ provide the cpu number.
----- On Feb 25, 2016, at 12:04 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
On Thu, Feb 25, 2016 at 04:55:26PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
Yes yes, I know how restartable sequences work.
But what I worry about is that they want a cpu number and a sequence
number, and for performance it would be very good if those live in the
same cacheline.
That means either getcpu needs to grow a seq number, or restartable
sequences need to _also_ provide the cpu number.
If we plan things well, we could have both the cpu number and the
seqnum in the same cache line, registered by two different system
calls. It's up to user-space to organize those two variables
to fit within the same cache-line.
getcpu_cache GETCPU_CACHE_SET operation takes the address where
the CPU number should live as input.
rseq system call could do the same for the seqnum address.
The question becomes: how do we introduce this to user-space,
considering that only a single address per thread is allowed
for each of getcpu_cache and rseq ?
If both CPU number and seqnum are centralized in a TLS within
e.g. glibc, that would be OK, but if we intend to allow libraries
or applications to directly register their own getcpu_cache
address and/or rseq, we may end up in situations where we have
to fallback on using two different cache-lines. But how much
should we care about performance in cases where non-generic
libraries directly use those system calls ?
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-26 11:33:15
On Thu, Feb 25, 2016 at 05:17:51PM +0000, Mathieu Desnoyers wrote:
----- On Feb 25, 2016, at 12:04 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
quoted
On Thu, Feb 25, 2016 at 04:55:26PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
Yes yes, I know how restartable sequences work.
But what I worry about is that they want a cpu number and a sequence
number, and for performance it would be very good if those live in the
same cacheline.
That means either getcpu needs to grow a seq number, or restartable
sequences need to _also_ provide the cpu number.
If we plan things well, we could have both the cpu number and the
seqnum in the same cache line, registered by two different system
calls. It's up to user-space to organize those two variables
to fit within the same cache-line.
I feel this is more fragile than needed. Why not do a single systemcall
that does both?
getcpu_cache GETCPU_CACHE_SET operation takes the address where
the CPU number should live as input.
rseq system call could do the same for the seqnum address.
So I really don't like that, that means we have to track more kernel
state -- we have to carry two pointers instead of one, we have to have
more update functions etc..
That just increases the total overhead of all of this.
The question becomes: how do we introduce this to user-space,
considering that only a single address per thread is allowed
for each of getcpu_cache and rseq ?
If both CPU number and seqnum are centralized in a TLS within
e.g. glibc, that would be OK, but if we intend to allow libraries
or applications to directly register their own getcpu_cache
address and/or rseq, we may end up in situations where we have
to fallback on using two different cache-lines. But how much
should we care about performance in cases where non-generic
libraries directly use those system calls ?
Thoughts ?
Yeah, not sure, but that is a separate problem. Both your proposed code
and the rseq code have this. Having them separate system calls just
increases the amount of ways you can do it wrong.
From: Thomas Gleixner <hidden> Date: 2016-02-26 16:31:43
On Fri, 26 Feb 2016, Peter Zijlstra wrote:
On Thu, Feb 25, 2016 at 05:17:51PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 12:04 PM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
quoted
On Thu, Feb 25, 2016 at 04:55:26PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org wrote:
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
Yes yes, I know how restartable sequences work.
But what I worry about is that they want a cpu number and a sequence
number, and for performance it would be very good if those live in the
same cacheline.
That means either getcpu needs to grow a seq number, or restartable
sequences need to _also_ provide the cpu number.
If we plan things well, we could have both the cpu number and the
seqnum in the same cache line, registered by two different system
calls. It's up to user-space to organize those two variables
to fit within the same cache-line.
I feel this is more fragile than needed. Why not do a single systemcall
that does both?
Right. There is no point in having two calls and two update mechanisms for a
very similar purpose.
So let userspace have one struct where cpu/seq and whatever is required for
rseq is located and flag at register time which parts of the struct need to be
updated.
Thanks,
tglx
----- On Feb 26, 2016, at 11:29 AM, Thomas Gleixner tglx@linutronix.de wrote:
On Fri, 26 Feb 2016, Peter Zijlstra wrote:
quoted
On Thu, Feb 25, 2016 at 05:17:51PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 12:04 PM, Peter Zijlstra peterz@infradead.org wrote:
quoted
On Thu, Feb 25, 2016 at 04:55:26PM +0000, Mathieu Desnoyers wrote:
quoted
----- On Feb 25, 2016, at 4:56 AM, Peter Zijlstra peterz@infradead.org wrote:
The restartable sequences are intrinsically designed to work
on per-cpu data, so they need to fetch the current CPU number
within the rseq critical section. This is where the getcpu_cache
system call becomes very useful when combined with rseq:
getcpu_cache allows reading the current CPU number in a
fraction of cycle.
Yes yes, I know how restartable sequences work.
But what I worry about is that they want a cpu number and a sequence
number, and for performance it would be very good if those live in the
same cacheline.
That means either getcpu needs to grow a seq number, or restartable
sequences need to _also_ provide the cpu number.
If we plan things well, we could have both the cpu number and the
seqnum in the same cache line, registered by two different system
calls. It's up to user-space to organize those two variables
to fit within the same cache-line.
I feel this is more fragile than needed. Why not do a single systemcall
that does both?
Right. There is no point in having two calls and two update mechanisms for a
very similar purpose.
So let userspace have one struct where cpu/seq and whatever is required for
rseq is located and flag at register time which parts of the struct need to be
updated.
If we put both cpu/seq/other in that structure, why not plan ahead and make
it extensible then ?
That looks very much like the "Thread-local ABI" series I posted last year.
See https://lkml.org/lkml/2015/12/22/464
Here is why I ended up introducing the specialized "getcpu_cache" system call
rather than the "generic" system call (quote from the getcpu_cache changelog):
Rationale for the getcpu_cache system call rather than the thread-local
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this system
call for a cpu number cache only. Anyway, the thread-local ABI approach
would have required that we introduce "feature" flags, which would have
ended up reimplementing multiplexing of features on top of a system
call. It seems better to introduce one system call per feature instead.
If everyone end up preferring that we introduce a system call that implements
many features at once, that's indeed something we can do, but I remember
being told in the past that this is generally a bad idea.
For one thing, it would make the interface more cumbersome to deal with
from user-space in terms of feature detection: if we want to make this
interface extensible, in addition to check -1, errno=ENOSYS, userspace
would have to deal with a field containing the length of the structure
as expected by user-space and kernel, and feature flags to see the common
set of features supported by kernel and user-space.
Having one system call per feature seems simpler to handle in terms of
feature availability detection from a userspace point of view.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Thomas Gleixner <hidden> Date: 2016-02-26 18:03:35
On Fri, 26 Feb 2016, Mathieu Desnoyers wrote:
----- On Feb 26, 2016, at 11:29 AM, Thomas Gleixner tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org wrote:
quoted
Right. There is no point in having two calls and two update mechanisms for a
very similar purpose.
So let userspace have one struct where cpu/seq and whatever is required for
rseq is located and flag at register time which parts of the struct need to be
updated.
If we put both cpu/seq/other in that structure, why not plan ahead and make
it extensible then ?
That looks very much like the "Thread-local ABI" series I posted last year.
See https://lkml.org/lkml/2015/12/22/464
Here is why I ended up introducing the specialized "getcpu_cache" system call
rather than the "generic" system call (quote from the getcpu_cache changelog):
Rationale for the getcpu_cache system call rather than the thread-local
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this system
call for a cpu number cache only. Anyway, the thread-local ABI approach
would have required that we introduce "feature" flags, which would have
ended up reimplementing multiplexing of features on top of a system
call. It seems better to introduce one system call per feature instead.
If everyone end up preferring that we introduce a system call that implements
many features at once, that's indeed something we can do, but I remember
being told in the past that this is generally a bad idea.
It's a bad idea if you mix stuff which does not belong together, but if you
have stuff which shares a substantial amount of things then it makes a lot of
sense. Especially if it adds similar stuff into hotpathes.
For one thing, it would make the interface more cumbersome to deal with
from user-space in terms of feature detection: if we want to make this
interface extensible, in addition to check -1, errno=ENOSYS, userspace
would have to deal with a field containing the length of the structure
as expected by user-space and kernel, and feature flags to see the common
set of features supported by kernel and user-space.
Having one system call per feature seems simpler to handle in terms of
feature availability detection from a userspace point of view.
That might well be, but that does not justify two fastpath updates, two
seperate pointers to handle, etc ....
Thanks,
tglx
----- On Feb 26, 2016, at 1:01 PM, Thomas Gleixner tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org wrote:
On Fri, 26 Feb 2016, Mathieu Desnoyers wrote:
quoted
----- On Feb 26, 2016, at 11:29 AM, Thomas Gleixner tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org wrote:
quoted
Right. There is no point in having two calls and two update mechanisms for a
very similar purpose.
So let userspace have one struct where cpu/seq and whatever is required for
rseq is located and flag at register time which parts of the struct need to be
updated.
If we put both cpu/seq/other in that structure, why not plan ahead and make
it extensible then ?
That looks very much like the "Thread-local ABI" series I posted last year.
See https://lkml.org/lkml/2015/12/22/464
Here is why I ended up introducing the specialized "getcpu_cache" system call
rather than the "generic" system call (quote from the getcpu_cache changelog):
Rationale for the getcpu_cache system call rather than the thread-local
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this system
call for a cpu number cache only. Anyway, the thread-local ABI approach
would have required that we introduce "feature" flags, which would have
ended up reimplementing multiplexing of features on top of a system
call. It seems better to introduce one system call per feature instead.
If everyone end up preferring that we introduce a system call that implements
many features at once, that's indeed something we can do, but I remember
being told in the past that this is generally a bad idea.
It's a bad idea if you mix stuff which does not belong together, but if you
have stuff which shares a substantial amount of things then it makes a lot of
sense. Especially if it adds similar stuff into hotpathes.
quoted
For one thing, it would make the interface more cumbersome to deal with
from user-space in terms of feature detection: if we want to make this
interface extensible, in addition to check -1, errno=ENOSYS, userspace
would have to deal with a field containing the length of the structure
as expected by user-space and kernel, and feature flags to see the common
set of features supported by kernel and user-space.
Having one system call per feature seems simpler to handle in terms of
feature availability detection from a userspace point of view.
That might well be, but that does not justify two fastpath updates, two
seperate pointers to handle, etc ....
Keeping two separate pointers in the task_struct rather than a single one
might indeed be unwelcome, but I'm not sure I fully grasp the fast path
argument in this case: getcpu_cache only sets a notifier thread flag
on thread migration, whereas AFAIU rseq adds code to context switch and signal
delivery, which are prone to have a higher impact.
Indeed both will have their own code in the resume notifier, but is it really
a fast path ?
From my point of view, making it easy for userspace to just enable getcpu_cache
without having the scheduler and signal delivery fast-path overhead of rseq seems
like a good thing. I'm not all that sure that saving an extra pointer in
task_struct justifies the added system call interface complexity.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
rather than the "generic" system call (quote from the getcpu_cache
changelog):
quoted
quoted
Rationale for the getcpu_cache system call rather than the
thread-local
quoted
quoted
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this
system
quoted
quoted
call for a cpu number cache only. Anyway, the thread-local ABI
approach
quoted
quoted
would have required that we introduce "feature" flags, which
would have
quoted
quoted
ended up reimplementing multiplexing of features on top of a
system
quoted
quoted
call. It seems better to introduce one system call per feature
instead.
quoted
quoted
If everyone end up preferring that we introduce a system call that
implements
quoted
quoted
many features at once, that's indeed something we can do, but I
remember
quoted
quoted
being told in the past that this is generally a bad idea.
It's a bad idea if you mix stuff which does not belong together, but
if you
quoted
have stuff which shares a substantial amount of things then it makes
a lot of
quoted
sense. Especially if it adds similar stuff into hotpathes.
quoted
For one thing, it would make the interface more cumbersome to deal
with
quoted
quoted
from user-space in terms of feature detection: if we want to make
this
quoted
quoted
interface extensible, in addition to check -1, errno=ENOSYS,
userspace
quoted
quoted
would have to deal with a field containing the length of the
structure
quoted
quoted
as expected by user-space and kernel, and feature flags to see the
common
quoted
quoted
set of features supported by kernel and user-space.
Having one system call per feature seems simpler to handle in terms
of
quoted
quoted
feature availability detection from a userspace point of view.
That might well be, but that does not justify two fastpath updates,
two
quoted
seperate pointers to handle, etc ....
Keeping two separate pointers in the task_struct rather than a single
one
might indeed be unwelcome, but I'm not sure I fully grasp the fast path
argument in this case: getcpu_cache only sets a notifier thread flag
on thread migration, whereas AFAIU rseq adds code to context switch and
signal
delivery, which are prone to have a higher impact.
Indeed both will have their own code in the resume notifier, but is it
really
a fast path ?
From my point of view, making it easy for userspace to just enable
getcpu_cache
without having the scheduler and signal delivery fast-path overhead of
rseq seems
like a good thing. I'm not all that sure that saving an extra pointer
in
task_struct justifies the added system call interface complexity.
Thanks,
Mathieu
I think it would be a good idea to make this a general pointer for the kernel to be able to write per thread state to user space, which obviously can't be done with the vDSO.
This means the libc per thread startup should query the kernel for the size of this structure and allocate thread local data accordingly. We can then grow this structure if needed without making the ABI even more complex.
This is more than a system call: this is an entirely new way for userspace to interact with the kernel. Therefore we should make it a general facility.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
rather than the "generic" system call (quote from the getcpu_cache
changelog):
quoted
quoted
Rationale for the getcpu_cache system call rather than the
thread-local
quoted
quoted
ABI system call proposed earlier:
Rather than doing a "generic" thread-local ABI, specialize this
system
quoted
quoted
call for a cpu number cache only. Anyway, the thread-local ABI
approach
quoted
quoted
would have required that we introduce "feature" flags, which
would have
quoted
quoted
ended up reimplementing multiplexing of features on top of a
system
quoted
quoted
call. It seems better to introduce one system call per feature
instead.
quoted
quoted
If everyone end up preferring that we introduce a system call that
implements
quoted
quoted
many features at once, that's indeed something we can do, but I
remember
quoted
quoted
being told in the past that this is generally a bad idea.
It's a bad idea if you mix stuff which does not belong together, but
if you
quoted
have stuff which shares a substantial amount of things then it makes
a lot of
quoted
sense. Especially if it adds similar stuff into hotpathes.
quoted
For one thing, it would make the interface more cumbersome to deal
with
quoted
quoted
from user-space in terms of feature detection: if we want to make
this
quoted
quoted
interface extensible, in addition to check -1, errno=ENOSYS,
userspace
quoted
quoted
would have to deal with a field containing the length of the
structure
quoted
quoted
as expected by user-space and kernel, and feature flags to see the
common
quoted
quoted
set of features supported by kernel and user-space.
Having one system call per feature seems simpler to handle in terms
of
quoted
quoted
feature availability detection from a userspace point of view.
That might well be, but that does not justify two fastpath updates,
two
quoted
seperate pointers to handle, etc ....
Keeping two separate pointers in the task_struct rather than a single
one
might indeed be unwelcome, but I'm not sure I fully grasp the fast path
argument in this case: getcpu_cache only sets a notifier thread flag
on thread migration, whereas AFAIU rseq adds code to context switch and
signal
delivery, which are prone to have a higher impact.
Indeed both will have their own code in the resume notifier, but is it
really
a fast path ?
From my point of view, making it easy for userspace to just enable
getcpu_cache
without having the scheduler and signal delivery fast-path overhead of
rseq seems
like a good thing. I'm not all that sure that saving an extra pointer
in
task_struct justifies the added system call interface complexity.
Thanks,
Mathieu
I think it would be a good idea to make this a general pointer for the kernel to
be able to write per thread state to user space, which obviously can't be done
with the vDSO.
This means the libc per thread startup should query the kernel for the size of
this structure and allocate thread local data accordingly. We can then grow
this structure if needed without making the ABI even more complex.
This is more than a system call: this is an entirely new way for userspace to
interact with the kernel. Therefore we should make it a general facility.
I'm really glad to see I'm not the only one seeing potential for
genericity here. :-) This is exactly what I had in mind
last year when proposing the thread_local_abi() system call:
a generic way to register an extensible per-thread data structure
so the kernel can communicate with user-space and vice-versa.
Rather than having the libc query the kernel for size of the structure,
I would recommend that libc tells the kernel the size of the thread-local
ABI structure it supports. The idea here is that both the kernel and libc
need to know about the fields in that structure to allow a two-way
interaction. Fields known only by either the kernel or userspace
are useless for a given thread anyway. This way, libc could statically
define the structure.
I would be tempted to also add "features" flags, so both user-space
and the kernel could tell each other what they support: user-space
would announce the set of features it supports, and it could also
query the kernel for the set of supported features. One simple approach
would be to use a uint64_t as type for those feature flags, and
reserve the last bit for extending to future flags if we ever have
more than 64.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-02-27 06:28:41
On 02/26/16 16:40, Mathieu Desnoyers wrote:
quoted
I think it would be a good idea to make this a general pointer for the kernel to
be able to write per thread state to user space, which obviously can't be done
with the vDSO.
This means the libc per thread startup should query the kernel for the size of
this structure and allocate thread local data accordingly. We can then grow
this structure if needed without making the ABI even more complex.
This is more than a system call: this is an entirely new way for userspace to
interact with the kernel. Therefore we should make it a general facility.
I'm really glad to see I'm not the only one seeing potential for
genericity here. :-) This is exactly what I had in mind
last year when proposing the thread_local_abi() system call:
a generic way to register an extensible per-thread data structure
so the kernel can communicate with user-space and vice-versa.
Rather than having the libc query the kernel for size of the structure,
I would recommend that libc tells the kernel the size of the thread-local
ABI structure it supports. The idea here is that both the kernel and libc
need to know about the fields in that structure to allow a two-way
interaction. Fields known only by either the kernel or userspace
are useless for a given thread anyway. This way, libc could statically
define the structure.
Big fat NOPE there. Why? Because it means that EVERY interaction with
this memory, no matter how critical, needs to be conditionalized.
Furthermore, userspace != libc. Applications or higher-layer libraries
might have more information than the running libc about additional
fields, but with your proposal libc would gate them.
As far as the kernel providing the size in the structure (alone) -- I
*really* hope you can see what is wrong with that!! That doesn't mean
we can't provide it in the structure as well, and that too might avoid
the skipped libc problem.
I would be tempted to also add "features" flags, so both user-space
and the kernel could tell each other what they support: user-space
would announce the set of features it supports, and it could also
query the kernel for the set of supported features. One simple approach
would be to use a uint64_t as type for those feature flags, and
reserve the last bit for extending to future flags if we ever have
more than 64.
Thoughts ?
It doesn't seem like it would hurt, although the size of the flags field
could end up being an issue.
-hpa
----- On Feb 27, 2016, at 1:24 AM, H. Peter Anvin hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org wrote:
On 02/26/16 16:40, Mathieu Desnoyers wrote:
quoted
quoted
I think it would be a good idea to make this a general pointer for the kernel to
be able to write per thread state to user space, which obviously can't be done
with the vDSO.
This means the libc per thread startup should query the kernel for the size of
this structure and allocate thread local data accordingly. We can then grow
this structure if needed without making the ABI even more complex.
This is more than a system call: this is an entirely new way for userspace to
interact with the kernel. Therefore we should make it a general facility.
I'm really glad to see I'm not the only one seeing potential for
genericity here. :-) This is exactly what I had in mind
last year when proposing the thread_local_abi() system call:
a generic way to register an extensible per-thread data structure
so the kernel can communicate with user-space and vice-versa.
Rather than having the libc query the kernel for size of the structure,
I would recommend that libc tells the kernel the size of the thread-local
ABI structure it supports. The idea here is that both the kernel and libc
need to know about the fields in that structure to allow a two-way
interaction. Fields known only by either the kernel or userspace
are useless for a given thread anyway. This way, libc could statically
define the structure.
Big fat NOPE there. Why? Because it means that EVERY interaction with
this memory, no matter how critical, needs to be conditionalized.
Furthermore, userspace != libc. Applications or higher-layer libraries
might have more information than the running libc about additional
fields, but with your proposal libc would gate them.
Good point!
As far as the kernel providing the size in the structure (alone) -- I
*really* hope you can see what is wrong with that!! That doesn't mean
we can't provide it in the structure as well, and that too might avoid
the skipped libc problem.
Indeed, libc would need to query the size before it can allocate
the structure.
quoted
I would be tempted to also add "features" flags, so both user-space
and the kernel could tell each other what they support: user-space
would announce the set of features it supports, and it could also
query the kernel for the set of supported features. One simple approach
would be to use a uint64_t as type for those feature flags, and
reserve the last bit for extending to future flags if we ever have
more than 64.
Thoughts ?
It doesn't seem like it would hurt, although the size of the flags field
could end up being an issue.
I'm concerned that this thread-local ABI structure may become messy.
Let's just imagine how we would first introduce a "cpu_id" field (int32_t),
and eventually add a "seqnum" field for rseq in the future (unsigned long).
Both fields need to be read with single-copy semantics as volatile
reads, and both need to be naturally aligned. However, I'm tempted
to use the "packed" attribute on the structure since it's an ABI
between kernel and user-space. A pretty bad example of what this
could become, due to alignment constraints, looks like:
/* This structure needs to be aligned on pointer size. */
struct thread_local_abi {
int32_t cpu_id;
int32_t __unused1;
unsigned long seqnum;
/* Add new fields at the end. */
} __attribute__((packed));
And this is just a start. It may become messier as we append
new fields in the future.
The main argument I currently see in favor of having this
meta system call for all per-thread features is to only
maintain a single pointer in the kernel task_struct rather
than one per thread-local feature.
If the goal is really to keep the burden on the task struct
small, we could use kmalloc()/kfree() to allocate and free an
array of pointers to the various per-thread features, rather
than putting them directly in task_struct. We could keep a
mask of the enabled features in the task struct too (which
we will likely have to do even if we go the the thread-local
ABI meta system call).
Having this per-task allocated pointer array at kernel-level
would allow us to have one system call per feature, with clear
semantics, without evolving a messy thread-local ABI structure
due to all sorts of alignment constraints.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-27 14:58:30
On Sat, Feb 27, 2016 at 02:15:01PM +0000, Mathieu Desnoyers wrote:
I'm concerned that this thread-local ABI structure may become messy.
Let's just imagine how we would first introduce a "cpu_id" field (int32_t),
and eventually add a "seqnum" field for rseq in the future (unsigned long).
The rseq seq number can be uint32_t, in fact it is in Paul's patches.
(This is true because every seq increment will guarantee a userspace
exception and reload of the value, its impossible to wrap the thing and
get a false positive.)
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Although he allows the post_commit_ip to be a separate field (which I
don't think makes sense).
/* This structure needs to be aligned on pointer size. */
I would mandate the thing be cacheline aligned, and sod packed, that can
lead to horrible layouts.
If the goal is really to keep the burden on the task struct
small, we could use kmalloc()/kfree() to allocate and free an
array of pointers to the various per-thread features, rather
*groan*, no that's even worse, then you get even more loads to update
the fields. The point is to reduce the total overhead of having this
stuff.
Having a single pointer with known offsets is best because then its
guaranteed a single load, then having the whole data structure in a
single cacheline again saves on memops, you can only miss once.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-02-27 15:07:52
On February 27, 2016 6:15:01 AM PST, Mathieu Desnoyers [off-list ref] wrote:
----- On Feb 27, 2016, at 1:24 AM, H. Peter Anvin hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org wrote:
quoted
On 02/26/16 16:40, Mathieu Desnoyers wrote:
quoted
quoted
I think it would be a good idea to make this a general pointer for
the kernel to
quoted
quoted
quoted
be able to write per thread state to user space, which obviously
can't be done
quoted
quoted
quoted
with the vDSO.
This means the libc per thread startup should query the kernel for
the size of
quoted
quoted
quoted
this structure and allocate thread local data accordingly. We can
then grow
quoted
quoted
quoted
this structure if needed without making the ABI even more complex.
This is more than a system call: this is an entirely new way for
userspace to
quoted
quoted
quoted
interact with the kernel. Therefore we should make it a general
facility.
quoted
quoted
I'm really glad to see I'm not the only one seeing potential for
genericity here. :-) This is exactly what I had in mind
last year when proposing the thread_local_abi() system call:
a generic way to register an extensible per-thread data structure
so the kernel can communicate with user-space and vice-versa.
Rather than having the libc query the kernel for size of the
structure,
quoted
quoted
I would recommend that libc tells the kernel the size of the
thread-local
quoted
quoted
ABI structure it supports. The idea here is that both the kernel and
libc
quoted
quoted
need to know about the fields in that structure to allow a two-way
interaction. Fields known only by either the kernel or userspace
are useless for a given thread anyway. This way, libc could
statically
quoted
quoted
define the structure.
Big fat NOPE there. Why? Because it means that EVERY interaction
with
quoted
this memory, no matter how critical, needs to be conditionalized.
Furthermore, userspace != libc. Applications or higher-layer
libraries
quoted
might have more information than the running libc about additional
fields, but with your proposal libc would gate them.
Good point!
quoted
As far as the kernel providing the size in the structure (alone) -- I
*really* hope you can see what is wrong with that!! That doesn't
mean
quoted
we can't provide it in the structure as well, and that too might
avoid
quoted
the skipped libc problem.
Indeed, libc would need to query the size before it can allocate
the structure.
quoted
quoted
I would be tempted to also add "features" flags, so both user-space
and the kernel could tell each other what they support: user-space
would announce the set of features it supports, and it could also
query the kernel for the set of supported features. One simple
approach
quoted
quoted
would be to use a uint64_t as type for those feature flags, and
reserve the last bit for extending to future flags if we ever have
more than 64.
Thoughts ?
It doesn't seem like it would hurt, although the size of the flags
field
quoted
could end up being an issue.
I'm concerned that this thread-local ABI structure may become messy.
Let's just imagine how we would first introduce a "cpu_id" field
(int32_t),
and eventually add a "seqnum" field for rseq in the future (unsigned
long).
Both fields need to be read with single-copy semantics as volatile
reads, and both need to be naturally aligned. However, I'm tempted
to use the "packed" attribute on the structure since it's an ABI
between kernel and user-space. A pretty bad example of what this
could become, due to alignment constraints, looks like:
/* This structure needs to be aligned on pointer size. */
struct thread_local_abi {
int32_t cpu_id;
int32_t __unused1;
unsigned long seqnum;
/* Add new fields at the end. */
} __attribute__((packed));
And this is just a start. It may become messier as we append
new fields in the future.
The main argument I currently see in favor of having this
meta system call for all per-thread features is to only
maintain a single pointer in the kernel task_struct rather
than one per thread-local feature.
If the goal is really to keep the burden on the task struct
small, we could use kmalloc()/kfree() to allocate and free an
array of pointers to the various per-thread features, rather
than putting them directly in task_struct. We could keep a
mask of the enabled features in the task struct too (which
we will likely have to do even if we go the the thread-local
ABI meta system call).
Having this per-task allocated pointer array at kernel-level
would allow us to have one system call per feature, with clear
semantics, without evolving a messy thread-local ABI structure
due to all sorts of alignment constraints.
Thoughts ?
Thanks,
Mathieu
I think you are worried about problems which we have already solved many, many times - structures are very common in the user space ABI and we know how to deal with this.
And when you say:
However, I'm tempted
to use the "packed" attribute on the structure
since it's an ABI
between kernel and user-space.
and mention "unsigned long" in a user space ABI all I can think of that you really have not followed the issues of user space ABI design as they have evolved over the last 20 years.
Simply put: non-problem.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
On Sat, Feb 27, 2016 at 6:58 AM, Peter Zijlstra [off-list ref] wrote:
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Please don't do "unsigned long" in ABI structures any more.
Make it u64, and make sure it is 64-bit aligned (which it would be in
this case). Make it so that we don't have to have separate compat
paths.
Linus
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-02-27 19:05:46
On February 27, 2016 10:35:28 AM PST, Linus Torvalds [off-list ref] wrote:
On Sat, Feb 27, 2016 at 6:58 AM, Peter Zijlstra [off-list ref]
wrote:
quoted
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Please don't do "unsigned long" in ABI structures any more.
Make it u64, and make sure it is 64-bit aligned (which it would be in
this case). Make it so that we don't have to have separate compat
paths.
Linus
Yes, if we have to do compat crap for this entire new ABI path I think I'll scream.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
----- On Feb 27, 2016, at 1:35 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
On Sat, Feb 27, 2016 at 6:58 AM, Peter Zijlstra [off-list ref] wrote:
quoted
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Please don't do "unsigned long" in ABI structures any more.
Make it u64, and make sure it is 64-bit aligned (which it would be in
this case). Make it so that we don't have to have separate compat
paths.
AFAIU, this "post_commit_ip" field is expected to be updated
with a single-copy-store by user-space. If we want to handle both
32-bit and 64-bit processes, how do you recommend doing this
without an unsigned long type ?
A 64-bit integer would not be a single-copy store for
32-bit processes, but a 32-bit integer would not be large
enough for 64-bit processes.
Would a
union {
uint32_t val32;
uint64_t val64;
} field;
be an acceptable option ? Then the kernel could use
one field or the other depending on the process bitness.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Hi Linus,
On Sat, Feb 27, 2016 at 7:35 PM, Linus Torvalds
[off-list ref] wrote:
On Sat, Feb 27, 2016 at 6:58 AM, Peter Zijlstra [off-list ref] wrote:
quoted
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Please don't do "unsigned long" in ABI structures any more.
Make it u64, and make sure it is 64-bit aligned (which it would be in
this case). Make it so that we don't have to have separate compat
paths.
__alignof__(u64) is not 8 on all architectures.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sun, Feb 28, 2016 at 5:07 AM, Geert Uytterhoeven
[off-list ref] wrote:
__alignof__(u64) is not 8 on all architectures.
Indeed, which is why I said "make sure it's 64-bit aligned". We do it
manually for ABI structures (although we did have some discussion
about adding a alignment directive, and then having an explicitly
unaligned type for legacy cases that we got wrong).
In the above case it was already properly aligned, because the
previous structure members added up to 64-bit boundaries.
Of course, nothing then stops user space from giving us structures
that are unaligned to begin with, but that's not our problem. As long
as the layout is correct, we're fine, and that's all we care about.
Linus
From: Peter Zijlstra <peterz@infradead.org> Date: 2016-02-29 10:02:16
On Sat, Feb 27, 2016 at 10:35:28AM -0800, Linus Torvalds wrote:
On Sat, Feb 27, 2016 at 6:58 AM, Peter Zijlstra [off-list ref] wrote:
quoted
Paul's patches have the following structure:
struct thread_local_abi {
union {
struct {
u32 cpu_id;
u32 seq;
};
u64 cpu_seq;
};
unsigned long post_commit_ip;
};
Please don't do "unsigned long" in ABI structures any more.
Make it u64, and make sure it is 64-bit aligned (which it would be in
this case). Make it so that we don't have to have separate compat
paths.
Yes, for sure. I was 'only' trying to reflect the state of the last rseq
patches. But yes, I should have called that out and avoided 'confusion'.