* [RFC PATCH for 4.21 10/16] arm: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Paul Turner <pjt@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
arch/arm/tools/syscall.tbl | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 8edf93b4490f..da2cb7a12644 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -414,3 +414,4 @@
397 common statx sys_statx
398 common rseq sys_rseq
399 common io_pgetevents sys_io_pgetevents
+400 common cpu_opv sys_cpu_opv
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 09/16] powerpc: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/uapi/asm/unistd.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 01b5171ea189..8f58710f5e8b 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -394,3 +394,4 @@ SYSCALL(pkey_free)
SYSCALL(pkey_mprotect)
SYSCALL(rseq)
COMPAT_SYS(io_pgetevents)
+SYSCALL(cpu_opv)
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 985534d0b448..112e2c54750a 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -400,5 +400,6 @@
#define __NR_pkey_mprotect 386
#define __NR_rseq 387
#define __NR_io_pgetevents 388
+#define __NR_cpu_opv 389
#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 08/16] x86: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
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(+)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 3cf7b533b3d1..d3253547e15e 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -398,3 +398,4 @@
384 i386 arch_prctl sys_arch_prctl __ia32_compat_sys_arch_prctl
385 i386 io_pgetevents sys_io_pgetevents __ia32_compat_sys_io_pgetevents
386 i386 rseq sys_rseq __ia32_sys_rseq
+387 i386 cpu_opv sys_cpu_opv __ia32_sys_cpu_opv
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index f0b1709a5ffb..1391971b1517 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -343,6 +343,7 @@
332 common statx __x64_sys_statx
333 common io_pgetevents __x64_sys_io_pgetevents
334 common rseq __x64_sys_rseq
+335 common cpu_opv __x64_sys_cpu_opv
#
# x32-specific system call numbers start at 512 to avoid cache impact
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 07/16] cpu_opv: limit amount of virtual address space used by cpu_opv
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Introduce sysctl cpu_opv_va_max_bytes, which limits the amount of
virtual address space that can be used by cpu_opv.
Its default value is the maximum amount of virtual address space which
can be used by a single cpu_opv system call (e.g. 64 kB on x86).
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
kernel/cpu_opv.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
kernel/sysctl.c | 15 ++++++++++++
2 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/kernel/cpu_opv.c b/kernel/cpu_opv.c
index 6ee7ca3376be..309d20a35227 100644
--- a/kernel/cpu_opv.c
+++ b/kernel/cpu_opv.c
@@ -59,6 +59,16 @@
/* Maximum number of virtual addresses per op. */
#define CPU_OP_VEC_MAX_ADDR (2 * CPU_OP_VEC_LEN_MAX)
+/* Maximum address range size (aligned on SHMLBA) per virtual address. */
+#define CPU_OP_RANGE_PER_ADDR_MAX (2 * SHMLBA)
+
+/*
+ * Minimum value for sysctl_cpu_opv_va_max_bytes is the maximum virtual memory
+ * space needed by one cpu_opv system call.
+ */
+#define CPU_OPV_VA_MAX_BYTES_MIN \
+ (CPU_OP_VEC_MAX_ADDR * CPU_OP_RANGE_PER_ADDR_MAX)
+
union op_fn_data {
uint8_t _u8;
uint16_t _u16;
@@ -96,6 +106,15 @@ struct opv_ipi_args {
*/
static DEFINE_MUTEX(cpu_opv_offline_lock);
+/* Maximum virtual address space which can be used by cpu_opv. */
+int sysctl_cpu_opv_va_max_bytes __read_mostly;
+int sysctl_cpu_opv_va_max_bytes_min;
+
+static atomic_t cpu_opv_va_allocated_bytes;
+
+/* Waitqueue for cpu_opv blocked on virtual address space reservation. */
+static DECLARE_WAIT_QUEUE_HEAD(cpu_opv_va_wait);
+
/*
* The cpu_opv system call executes a vector of operations on behalf of
* user-space on a specific CPU either with interrupts disabled or within
@@ -489,6 +508,43 @@ static int cpu_opv_pin_pages_op(struct cpu_op *op,
return 0;
}
+/*
+ * Approximate the amount of virtual address space required per
+ * vaddr to a worse-case of CPU_OP_RANGE_PER_ADDR_MAX.
+ */
+static int cpu_opv_reserve_va(int nr_vaddr, int *reserved_va)
+{
+ int nr_bytes = nr_vaddr * CPU_OP_RANGE_PER_ADDR_MAX;
+ int old_bytes, new_bytes;
+
+ WARN_ON_ONCE(*reserved_va != 0);
+ if (nr_bytes > sysctl_cpu_opv_va_max_bytes) {
+ WARN_ON_ONCE(1);
+ return -EINVAL;
+ }
+ do {
+ wait_event(cpu_opv_va_wait,
+ (old_bytes = atomic_read(&cpu_opv_va_allocated_bytes)) +
+ nr_bytes <= sysctl_cpu_opv_va_max_bytes);
+ new_bytes = old_bytes + nr_bytes;
+ } while (atomic_cmpxchg(&cpu_opv_va_allocated_bytes,
+ old_bytes, new_bytes) != old_bytes);
+
+ *reserved_va = nr_bytes;
+ return 0;
+}
+
+static void cpu_opv_unreserve_va(int *reserved_va)
+{
+ int nr_bytes = *reserved_va;
+
+ if (!nr_bytes)
+ return;
+ atomic_sub(nr_bytes, &cpu_opv_va_allocated_bytes);
+ wake_up(&cpu_opv_va_wait);
+ *reserved_va = 0;
+}
+
static int cpu_opv_pin_pages(struct cpu_op *cpuop, int cpuopcnt,
struct cpu_opv_vaddr *vaddr_ptrs)
{
@@ -898,7 +954,7 @@ SYSCALL_DEFINE4(cpu_opv, struct cpu_op __user *, ucpuopv, int, cpuopcnt,
struct cpu_opv_vaddr vaddr_ptrs = {
.nr_vaddr = 0,
};
- int ret, i, nr_vaddr = 0;
+ int ret, i, nr_vaddr = 0, reserved_va = 0;
bool retry = false;
if (unlikely(flags & ~(CPU_OP_NR_FLAG | CPU_OP_VEC_LEN_MAX_FLAG)))
@@ -922,6 +978,9 @@ SYSCALL_DEFINE4(cpu_opv, struct cpu_op __user *, ucpuopv, int, cpuopcnt,
if (nr_vaddr > NR_VADDR)
return -EINVAL;
again:
+ ret = cpu_opv_reserve_va(nr_vaddr, &reserved_va);
+ if (ret)
+ goto end;
ret = cpu_opv_pin_pages(cpuopv, cpuopcnt, &vaddr_ptrs);
if (ret)
goto end;
@@ -946,6 +1005,7 @@ SYSCALL_DEFINE4(cpu_opv, struct cpu_op __user *, ucpuopv, int, cpuopcnt,
*/
if (vaddr_ptrs.nr_vaddr)
vm_unmap_aliases();
+ cpu_opv_unreserve_va(&reserved_va);
if (retry) {
retry = false;
vaddr_ptrs.nr_vaddr = 0;
@@ -953,3 +1013,15 @@ SYSCALL_DEFINE4(cpu_opv, struct cpu_op __user *, ucpuopv, int, cpuopcnt,
}
return ret;
}
+
+/*
+ * Dynamic initialization is required on sparc because SHMLBA is not a
+ * constant.
+ */
+static int __init cpu_opv_init(void)
+{
+ sysctl_cpu_opv_va_max_bytes = CPU_OPV_VA_MAX_BYTES_MIN;
+ sysctl_cpu_opv_va_max_bytes_min = CPU_OPV_VA_MAX_BYTES_MIN;
+ return 0;
+}
+core_initcall(cpu_opv_init);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index cc02050fd0c4..eb34c6be2aa4 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -175,6 +175,11 @@ extern int unaligned_dump_stack;
extern int no_unaligned_warning;
#endif
+#ifdef CONFIG_CPU_OPV
+extern int sysctl_cpu_opv_va_max_bytes;
+extern int sysctl_cpu_opv_va_max_bytes_min;
+#endif
+
#ifdef CONFIG_PROC_SYSCTL
/**
@@ -1233,6 +1238,16 @@ static struct ctl_table kern_table[] = {
.extra2 = &one,
},
#endif
+#ifdef CONFIG_CPU_OPV
+ {
+ .procname = "cpu_opv_va_max_bytes",
+ .data = &sysctl_cpu_opv_va_max_bytes,
+ .maxlen = sizeof(sysctl_cpu_opv_va_max_bytes),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &sysctl_cpu_opv_va_max_bytes_min,
+ },
+#endif
{ }
};
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 06/16] cpu_opv: Provide cpu_opv system call (v9)
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
The cpu_opv system call executes a vector of operations on behalf of
user-space on a specific CPU with interrupts disabled or within an
IPI handler. It is similar to readv() and writev() system calls which
take a "struct iovec" array as argument.
The operations available are: comparison, memcpy, memcpy_release, add,
and add_release. The system call receives a CPU number from user-space
as argument, which is the CPU on which those operations need to be
performed. All pointers in the ops must have been set up to point to
the per CPU memory of the CPU on which the operations should be
executed. The "comparison" operation can be used to check that the data
used in the preparation step did not change between preparation of
system call inputs and operation execution within the irq-off critical
section.
The reason why for requiring all pointer offsets to be calculated by
user-space beforehand is because get_user_pages() first needs to be
used to pin all pages touched by each operation. This takes care of
faulting-in the pages. Those pages are then vmap'd into the kernel
virtual address range. Then the operations are performed atomically with
respect to other thread's execution on that CPU, without generating any
page fault, by means of a IPI handler (or disabling interrupts).
An overall maximum of 4120 bytes in enforced on the sum of operation
length within an operation vector, so user-space cannot generate a
too long irq-off critical section. The maximum number of operations
supported in a vector is 4. User-space can query the maximum vector
size and the number of operation "instructions" supported with by
passing the appropriate flags as system call parameter. The cache cold
critical section duration has been measured as 4.7µs on x86-64 for 16
operations, therefore a more restrictive limit of 4 operations should
cause an even shorter irq-off latency. Each operation is also limited to
a length of 4096 bytes, meaning that an operation can touch a maximum of
4 pages (memcpy: 2 pages for source, 2 pages for destination if
addresses are not aligned on page boundaries).
**** Justification for cpu_opv ****
Here are a few reasons justifying why the cpu_opv system call is
needed in addition to rseq:
1) Allow algorithms to perform per-cpu data migration without relying on
sched_setaffinity()
The use-cases are migration of memory between per-cpu memory free-lists,
and stealing tasks from other per-cpu work queues: each require that
accesses to remote per-cpu data structures are performed.
Just rseq is not enough to cover those use-cases without additionally
relying on sched_setaffinity, which is unfortunately not
CPU-hotplug-safe.
The cpu_opv system call receives a CPU number as argument, and
performs the operation sequence in a IPI handler on the right CPU. The
IPI handler ensures any restartable sequence critical section it
interrupts will be aborted. If the requested CPU is offline, it performs
the operations from the current CPU while preventing CPU hotplug, and
with a mutex held.
2) Handling single-stepping from tools
Tools like debuggers and simulators use single-stepping to run through
existing programs. If core libraries start to use restartable sequences
for e.g. memory allocation, this means pre-existing programs cannot be
single-stepped, simply because the underlying glibc or jemalloc has
changed.
The rseq user-space does expose a __rseq_table section for the sake of
debuggers, so they can skip over the rseq critical sections if they
want. However, this requires upgrading tools, and still breaks
single-stepping in case where glibc or jemalloc is updated, but not the
tooling.
Having a performance-related library improvement break tooling is likely
to cause a big push-back against wide adoption of rseq.
3) Forward-progress guarantee
Having a piece of user-space code that stops progressing due to external
conditions is pretty bad. Developers are used to think of fast-path and
slow-path (e.g. for locking), where the contended vs uncontended cases
have different performance characteristics, but each need to provide
some level of progress guarantees.
There are concerns about using just "rseq" without the associated
slow-path (cpu_opv) that guarantees progress. It's just asking for
trouble when real-life happen: page faults, uprobes, and other
unforeseen conditions that would seldom cause a rseq fast-path to never
progress.
4) Handling page faults
It's pretty easy to come up with corner-case scenarios where rseq does
not progress without the help from cpu_opv. For instance, a system with
swap enabled which is under high memory pressure could trigger page
faults at pretty much every rseq attempt. Although this scenario
is extremely unlikely, rseq becomes the weak link of the chain.
5) Comparison with LL/SC handling of debugger single-stepping
The layman versed in the load-link/store-conditional instructions in
RISC architectures will notice the similarity between rseq and LL/SC
critical sections. The comparison can even be pushed further: since
debuggers can handle those LL/SC critical sections, they should be
able to handle rseq c.s. in the same way.
First, the way gdb recognises LL/SC c.s. patterns is very fragile:
it's limited to specific common patterns, and will miss the pattern
in all other cases. But fear not, having the rseq c.s. expose a
__rseq_table to debuggers removes that guessing part.
The main difference between LL/SC and rseq is that debuggers had
to support single-stepping through LL/SC critical sections from the
get go in order to support a given architecture. For rseq, we're
adding critical sections into pre-existing applications/libraries,
so the user expectation is that tools don't break due to a library
optimization.
6) Perform maintenance operations on per-cpu data
rseq c.s. are quite limited feature-wise: they need to end with a
*single* commit instruction that updates a memory location. On the other
hand, the cpu_opv system call can combine a sequence of operations that
need to be executed atomically with respect to concurrent execution on
a given CPU. While slower than rseq, this allows for more complex
maintenance operations to be performed on per-cpu data concurrently with
rseq fast-paths, in cases where it's not possible to map those sequences
of ops to a rseq.
7) Use cpu_opv as generic implementation for architectures not
implementing rseq assembly code
rseq critical sections require architecture-specific user-space code to
be crafted in order to port an algorithm to a given architecture. In
addition, it requires that the kernel architecture implementation adds
hooks into signal delivery and resume to user-space.
In order to facilitate integration of rseq into user-space, cpu_opv can
provide a (relatively slower) architecture-agnostic implementation of
rseq. This means that user-space code can be ported to all architectures
through use of cpu_opv initially, and have the fast-path use rseq
whenever the asm code is implemented.
8) Allow libraries with multi-part algorithms to work on same per-cpu
data without affecting the allowed cpu mask
The lttng-ust tracer presents an interesting use-case for per-cpu
buffers: the algorithm needs to update a "reserve" counter, serialize
data into the buffer, and then update a "commit" counter _on the same
per-cpu buffer_. Using rseq for both reserve and commit can bring
significant performance benefits.
Clearly, if rseq reserve fails, the algorithm can retry on a different
per-cpu buffer. However, it's not that easy for the commit. It needs to
be performed on the same per-cpu buffer as the reserve.
The cpu_opv system call solves that problem by receiving the cpu number
on which the operation needs to be performed as argument. It uses an
IPI to perform the operations on the requested CPU.
Changing the allowed cpu mask for the current thread is not an
acceptable alternative for a tracing library, because the application
being traced does not expect that mask to be changed by libraries.
9) Ensure that data structures don't need store-release/load-acquire
semantic to handle fall-back
cpu_opv performs the fall-back on the requested CPU with an IPI to that
CPU. Executing the slow-path on the right CPU ensures that
store-release/load-acquire semantic is not required neither on the
fast-path nor slow-path.
10) Allow use of rseq critical sections from signal handlers
Considering that rseq needs to be registered/unregistered from the
current thread, it means there is a window at thread creation/exit where
a signal handler can nest over the thread before rseq is registered by
glibc, or after it has been unregistered by glibc. One possibility to
handle this would be to extend clone() to have rseq registered
immediately when the thread is created, and unregistered implicitly when
the thread vanishes. Adding complexity to clone() has not been an idea
received well so far. So an alternative solution is to ensure that
signal handlers using rseq critical sections have a fallback mechanism
(cpu_opv) to work on per-cpu data structures when they are nested over
threads for which rseq is not currently registered.
11) Inability to mix rseq and non-rseq atomic operations on percpu data
A typical approach when dealing with locking fast-path and slow-path is
to fall-back on a slower/less efficient mechanism to perform what the
fast-path cannot do.
One approach that naturally comes to mind when considering rseq
fast-path abort would be to rather issue the same side-effect by
means of an atomic instruction. Arguably, before rseq, updates to
per-cpu data structures used to be done by reading the current CPU
number and by then using an atomic instruction to update the data in a
way that is safe against concurrent updates. This atomic instruction was
indeed needed to deal with migration between reading the current CPU
number and doing the update.
Unfortunately, it is a _bug_ to mix concurrent access to a per-cpu data
with both rseq (guaranteed to be on the right CPU, never migrated before
the commit) and non-rseq atomic instructions (which can be issued from
the wrong CPU), because the rseq critical section (on the right CPU)
executing concurrently with the atomic instruction (on another CPU due
to migration) on can cause data corruption of the per-cpu data.
**** rseq and cpu_opv use-cases ****
1) per-cpu spinlock
A per-cpu spinlock can be implemented as a rseq consisting of a
comparison operation (== 0) on a word, and a word store (1), followed
by an acquire barrier after control dependency. The unlock path can be
performed with a simple store-release of 0 to the word, which does
not require rseq.
The cpu_opv fallback requires a single-word comparison (== 0) and a
single-word store (1).
2) per-cpu statistics counters
A per-cpu statistics counters can be implemented as a rseq consisting
of a final "add" instruction on a word as commit.
The cpu_opv fallback can be implemented as a "ADD" operation.
Besides statistics tracking, these counters can be used to implement
user-space RCU per-cpu grace period tracking for both single and
multi-process user-space RCU.
3) per-cpu LIFO linked-list (unlimited size stack)
A per-cpu LIFO linked-list has a "push" and "pop" operation,
which respectively adds an item to the list, and removes an
item from the list.
The "push" operation can be implemented as a rseq consisting of
a word comparison instruction against head followed by a word store
(commit) to head. Its cpu_opv fallback can be implemented as a
word-compare followed by word-store as well.
The "pop" operation can be implemented as a rseq consisting of
loading head, comparing it against NULL, loading the next pointer
at the right offset within the head item, and the next pointer as
a new head, returning the old head on success.
The cpu_opv fallback for "pop" differs from its rseq algorithm:
considering that cpu_opv requires to know all pointers at system
call entry so it can pin all pages, so cpu_opv cannot simply load
head and then load the head->next address within the irq-off
critical section. User-space needs to pass the head and head->next
addresses to the kernel, and the kernel needs to check that the
head address is unchanged since it has been loaded by user-space.
However, when accessing head->next in a ABA situation, it's
possible that head is unchanged, but loading head->next can
result in a page fault due to a concurrently freed head object.
This is why the "expect_fault" operation field is introduced: if a
fault is triggered by this access, "-EAGAIN" will be returned by
cpu_opv rather than -EFAULT, thus indicating the the operation
vector should be attempted again. The "pop" operation can thus be
implemented as a word comparison of head against the head loaded
by user-space, followed by a load of the head->next pointer (which
may fault), and a store of that pointer as a new head.
4) per-cpu LIFO ring buffer with pointers to objects (fixed-sized stack)
This structure is useful for passing around allocated objects
by passing pointers through per-cpu fixed-sized stack.
The "push" side can be implemented with a check of the current
offset against the maximum buffer length, followed by a rseq
consisting of a comparison of the previously loaded offset
against the current offset, a word "try store" operation into the
next ring buffer array index (it's OK to abort after a try-store,
since it's not the commit, and its side-effect can be overwritten),
then followed by a word-store to increment the current offset (commit).
The "push" cpu_opv fallback can be done with the comparison, and
two consecutive word stores, all within the irq-off section.
The "pop" side can be implemented with a check that offset is not
0 (whether the buffer is empty), a load of the "head" pointer before the
offset array index, followed by a rseq consisting of a word
comparison checking that the offset is unchanged since previously
loaded, another check ensuring that the "head" pointer is unchanged,
followed by a store decrementing the current offset.
The cpu_opv "pop" can be implemented with the same algorithm
as the rseq fast-path (compare, compare, store).
5) per-cpu LIFO ring buffer with pointers to objects (fixed-sized stack)
supporting "peek" from remote CPU
In order to implement work queues with work-stealing between CPUs, it is
useful to ensure the offset "commit" in scenario 4) "push" have a
store-release semantic, thus allowing remote CPU to load the offset
with acquire semantic, and load the top pointer, in order to check if
work-stealing should be performed. The task (work queue item) existence
should be protected by other means, e.g. RCU.
If the peek operation notices that work-stealing should indeed be
performed, a thread can use cpu_opv to move the task between per-cpu
workqueues, by first invoking cpu_opv passing the remote work queue
cpu number as argument to pop the task, and then again as "push" with
the target work queue CPU number.
6) per-cpu LIFO ring buffer with data copy (fixed-sized stack)
(with and without acquire-release)
This structure is useful for passing around data without requiring
memory allocation by copying the data content into per-cpu fixed-sized
stack.
The "push" operation is performed with an offset comparison against
the buffer size (figuring out if the buffer is full), followed by
a rseq consisting of a comparison of the offset, a try-memcpy attempting
to copy the data content into the buffer (which can be aborted and
overwritten), and a final store incrementing the offset.
The cpu_opv fallback needs to same operations, except that the memcpy
is guaranteed to complete, given that it is performed with irqs
disabled or from IPI handler. This requires a memcpy operation
supporting length up to 4kB.
The "pop" operation is similar to the "push, except that the offset
is first compared to 0 to ensure the buffer is not empty. The
copy source is the ring buffer, and the destination is an output
buffer.
7) per-cpu FIFO ring buffer (fixed-sized queue)
This structure is useful wherever a FIFO behavior (queue) is needed.
One major use-case is tracer ring buffer.
An implementation of this ring buffer has a "reserve", followed by
serialization of multiple bytes into the buffer, ended by a "commit".
The "reserve" can be implemented as a rseq consisting of a word
comparison followed by a word store. The reserve operation moves the
producer "head". The multi-byte serialization can be performed
non-atomically. Finally, the "commit" update can be performed with
a rseq "add" commit instruction with store-release semantic. The
ring buffer consumer reads the commit value with load-acquire
semantic to know whenever it is safe to read from the ring buffer.
This use-case requires that both "reserve" and "commit" operations
be performed on the same per-cpu ring buffer, even if a migration
happens between those operations. In the typical case, both operations
will happens on the same CPU and use rseq. In the unlikely event of a
migration, the cpu_opv system call will ensure the commit can be
performed on the right CPU by migrating the task to that CPU.
On the consumer side, an alternative to using store-release and
load-acquire on the commit counter would be to use cpu_opv to
ensure the commit counter load is performed on the right CPU through an
IPI.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-api@vger.kernel.org
---
Changes since v1:
- handle CPU hotplug,
- cleanup implementation using function pointers: We can use function
pointers to implement the operations rather than duplicating all the
user-access code.
- refuse device pages: Performing cpu_opv operations on io map'd pages
with preemption disabled could generate long preempt-off critical
sections, which leads to unwanted scheduler latency. Return EFAULT if
a device page is received as parameter
- restrict op vector to 4216 bytes length sum: Restrict the operation
vector to length sum of:
- 4096 bytes (typical page size on most architectures, should be
enough for a string, or structures)
- 15 * 8 bytes (typical operations on integers or pointers).
The goal here is to keep the duration of preempt off critical section
short, so we don't add significant scheduler latency.
- Add INIT_ONSTACK macro: Introduce the
CPU_OP_FIELD_u32_u64_INIT_ONSTACK() macros to ensure that users
correctly initialize the upper bits of CPU_OP_FIELD_u32_u64() on their
stack to 0 on 32-bit architectures.
- Add CPU_MB_OP operation:
Use-cases with:
- two consecutive stores,
- a mempcy followed by a store,
require a memory barrier before the final store operation. A typical
use-case is a store-release on the final store. Given that this is a
slow path, just providing an explicit full barrier instruction should
be sufficient.
- Add expect fault field:
The use-case of list_pop brings interesting challenges. With rseq, we
can use rseq_cmpnev_storeoffp_load(), and therefore load a pointer,
compare it against NULL, add an offset, and load the target "next"
pointer from the object, all within a single req critical section.
Life is not so easy for cpu_opv in this use-case, mainly because we
need to pin all pages we are going to touch in the preempt-off
critical section beforehand. So we need to know the target object (in
which we apply an offset to fetch the next pointer) when we pin pages
before disabling preemption.
So the approach is to load the head pointer and compare it against
NULL in user-space, before doing the cpu_opv syscall. User-space can
then compute the address of the head->next field, *without loading it*.
The cpu_opv system call will first need to pin all pages associated
with input data. This includes the page backing the head->next object,
which may have been concurrently deallocated and unmapped. Therefore,
in this case, getting -EFAULT when trying to pin those pages may
happen: it just means they have been concurrently unmapped. This is
an expected situation, and should just return -EAGAIN to user-space,
to user-space can distinguish between "should retry" type of
situations and actual errors that should be handled with extreme
prejudice to the program (e.g. abort()).
Therefore, add "expect_fault" fields along with op input address
pointers, so user-space can identify whether a fault when getting a
field should return EAGAIN rather than EFAULT.
- Add compiler barrier between operations: Adding a compiler barrier
between store operations in a cpu_opv sequence can be useful when
paired with membarrier system call.
An algorithm with a paired slow path and fast path can use
sys_membarrier on the slow path to replace fast-path memory barriers
by compiler barrier.
Adding an explicit compiler barrier between operations allows
cpu_opv to be used as fallback for operations meant to match
the membarrier system call.
Changes since v2:
- Fix memory leak by introducing struct cpu_opv_pinned_pages.
Suggested by Boqun Feng.
- Cast argument 1 passed to access_ok from integer to void __user *,
fixing sparse warning.
Changes since v3:
- Fix !SMP by adding push_task_to_cpu() empty static inline.
- Add missing sys_cpu_opv() asmlinkage declaration to
include/linux/syscalls.h.
Changes since v4:
- Cleanup based on Thomas Gleixner's feedback.
- Handle retry in case where the scheduler migrates the thread away
from the target CPU after migration within the syscall rather than
returning EAGAIN to user-space.
- Move push_task_to_cpu() to its own patch.
- New scheme for touching user-space memory:
1) get_user_pages_fast() to pin/get all pages (which can sleep),
2) vm_map_ram() those pages
3) grab mmap_sem (read lock)
4) __get_user_pages_fast() (or get_user_pages() on failure)
-> Confirm that the same page pointers are returned. This
catches cases where COW mappings are changed concurrently.
-> If page pointers differ, or on gup failure, release mmap_sem,
vm_unmap_ram/put_page and retry from step (1).
-> perform put_page on the extra reference immediately for each
page.
5) preempt disable
6) Perform operations on vmap. Those operations are normal
loads/stores/memcpy.
7) preempt enable
8) release mmap_sem
9) vm_unmap_ram() all virtual addresses
10) put_page() all pages
- Handle architectures with VIVT caches along with vmap(): call
flush_kernel_vmap_range() after each "write" operation. This
ensures that the user-space mapping and vmap reach a consistent
state between each operation.
- Depend on MMU for is_zero_pfn(). e.g. Blackfin and SH architectures
don't provide the zero_pfn symbol.
Changes since v5:
- Fix handling of push_task_to_cpu() when argument is a cpu which is
not part of the task's allowed cpu mask.
- Add CPU_OP_NR_FLAG flag, which returns the number of operations
supported by the system call.
Changes since v6:
- Use __u* in public uapi header rather than uint*_t.
- Disallow cpu_opv targeting noncached vma, which requires using
get_user_pages() rather than get_user_pages_fast() to get the
vma.
- Fix handling of vm_map_ram() errors by increasing nr_vaddr only after
success.
- Issue vm_unmap_aliases() after each cpu_opv system call, thus ensuring
lazy unmapping does not exhaust vmalloc address space in stress-tests on
32-bit systems.
- Use vm_map_user_ram() and vm_unmap_user_ram() to ensure cache coherency
on virtually aliased architectures.
Changes since v7:
- Adapt to removal of types_32_64.h.
Changes since v8:
- Use IPI to interpret operation vector (or interrupt off critical
section). This is possible now that the interpreter touches a shadow
mapping (vmap) of the user-space pages, and it is simpler than trying
to migrate the current thread.
- Update documentation to reflect the change from preempt-off critical
section to IPI.
- Introduce SPDX license comments.
- Remove unused bitwise and shift operations (reduce instruction-set),
- Remove "mb" instruction,
- Introduce memcpy_release and add_release instructions,
- Allow user-space to query operation vector size supported by the kernel,
- Reduce operation vector size supported from 16 to 4.
---
Man page associated:
CPU_OPV(2) Linux Programmer's Manual CPU_OPV(2)
NAME
cpu_opv - Per-CPU-atomic operation vector system call
SYNOPSIS
#include <linux/cpu_opv.h>
int cpu_opv(struct cpu_op * cpu_opv, int cpuopcnt, int cpu, int flags);
DESCRIPTION
The cpu_opv system call executes a vector of operations on behalf
of user-space on a specific CPU atomically with respect to con‐
current execution on that CPU.
The term CPU used in this documentation refers to a hardware exe‐
cution context.
The operations available are: comparison, memcpy, add. Both mem‐
cpy and add operations have a counterpart with release semantic.
The system call receives a CPU number from user-space as argu‐
ment, which is the CPU on which those operations need to be per‐
formed. All pointers in the ops must have been set up to point
to the per CPU memory of the CPU on which the operations should
be executed. The "comparison" operation can be used to check that
the data used in the preparation step did not change between
preparation of system call inputs and operation execution by the
kernel.
An overall maximum of 4216 bytes in enforced on the sum of opera‐
tion length within an operation vector, so user-space cannot gen‐
erate a too long interrupt-off critical section or inter-proces‐
sor interrupt handler. Each operation is also limited a length of
4096 bytes. A maximum limit of 4 operations per cpu_opv syscall
invocation is enforced.
The layout of struct cpu_opv is as follows:
Fields
op Operation of type enum cpu_op_type to perform. This opera‐
tion type selects the associated "u" union field.
len
Length (in bytes) of data to consider for this operation.
u.compare_op
For a CPU_COMPARE_EQ_OP , and CPU_COMPARE_NE_OP , a and b
are pointers to data meant to be compared. The
expect_fault_a and expect_fault_b fields indicate whether
a page fault should be expected when accessing the memory
holding this data. If expect_fault_a , or expect_fault_b
is set, EAGAIN is returned on fault, else EFAULT is
returned. The len field is allowed to take values from 0
to 4096 for comparison operations.
u.memcpy_op
For a CPU_MEMCPY_OP , or CPU_MEMCPY_RELEASE_OP , contains
the dst and src pointers, expressing a copy of src into
dst. The expect_fault_dst and expect_fault_src fields
indicate whether a page fault should be expected when
accessing the memory holding both source and destination,
which starts at the pointer address, of length len . If
expect_fault_dst , or expect_fault_src is set, EAGAIN is
returned on fault, else EFAULT is returned. The len field
is allowed to take values from 0 to 4096 for memcpy opera‐
tions.
u.arithmetic_op
For a CPU_ADD_OP , or CPU_ADD_RELEASE_OP , contains the p
, count , and expect_fault_p fields, which are respec‐
tively a pointer to the memory location to increment, the
64-bit signed integer value to add, and whether a page
fault should be expected for p . If expect_fault_p is
set, EAGAIN is returned on fault, else EFAULT is returned.
The len field is allowed to take values of 1, 2, 4, 8
bytes for arithmetic operations.
The enum cpu_op_types contains the following operations:
· CPU_COMPARE_EQ_OP: Compare whether two memory locations are
equal,
· CPU_COMPARE_NE_OP: Compare whether two memory locations differ,
· CPU_MEMCPY_OP: Copy a source memory location into a destina‐
tion,
· CPU_MEMCPY_RELEASE_OP: Copy a source memory location into a
destination, with release semantic,
· CPU_ADD_OP: Increment a target memory location of a given
count,
· CPU_ADD_RELEASE_OP: Increment a target memory location of a
given count, with release semantic,
All of the operations above provide single-copy atomicity guar‐
antees for word-sized, word-aligned target pointers, for both
loads and stores.
The cpuopcnt argument is the number of elements in the cpu_opv
array. It can take values from 0 to an upper limit returned by
invoking cpu_opv() with the CPU_OP_VEC_LEN_MAX_FLAG flag set.
The cpu argument is the CPU number on which the operation
sequence needs to be executed.
The flags argument is a bitmask. When CPU_OP_NR_FLAG is set, the
cpu_opv() system call returns the number of operations available.
When CPU_OP_VEC_LEN_MAX_FLAG is set, the cpu_opv() system call
returns the maximum length of the sequence of operations that is
accepted as input argument by the system call. When flags is 0,
the sequence of operations received as parameter is performed.
RETURN VALUE
When invoked with flags set at 0, a return value of 0 indicates
success. On error, -1 is returned, and errno is set. If a com‐
parison operation fails, execution of the operation vector is
stopped, and the return value is the index after the comparison
operation (values between 1 and 4).
When flags is non-zero, on error, -1 is returned, and errno is
set. On success, the behavior is described in the DESCRIPTION
section for each flag.
ERRORS
EAGAIN cpu_opv() system call should be attempted again.
EINVAL Either flags contains an invalid value, or cpu contains an
invalid value or a value not allowed by the current
thread's allowed cpu mask, or cpuopcnt contains an invalid
value, or the cpu_opv operation vector contains an invalid
op value, or the cpu_opv operation vector contains an
invalid len value, or the cpu_opv operation vector sum of
len values is too large.
ENOSYS The cpu_opv() system call is not implemented by this ker‐
nel.
EFAULT cpu_opv is an invalid address, or a pointer contained
within an operation is invalid (and a fault is not
expected for that pointer). Pointers to device and non‐
cached memory within an operation are considered invalid.
VERSIONS
The cpu_opv() system call was added in Linux 4.X (TODO).
CONFORMING TO
cpu_opv() is Linux-specific.
SEE ALSO
membarrier(2), rseq(2)
Linux 2018-10-27 CPU_OPV(2)
---
MAINTAINERS | 7 +
include/linux/syscalls.h | 3 +
include/uapi/linux/cpu_opv.h | 69 ++++
init/Kconfig | 18 +
kernel/Makefile | 1 +
kernel/cpu_opv.c | 955 +++++++++++++++++++++++++++++++++++++++++++
kernel/sys_ni.c | 1 +
7 files changed, 1054 insertions(+)
create mode 100644 include/uapi/linux/cpu_opv.h
create mode 100644 kernel/cpu_opv.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b2f710eee67a..de59c7c12c8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3872,6 +3872,13 @@ B: https://bugzilla.kernel.org
F: drivers/cpuidle/*
F: include/linux/cpuidle.h
+PER-CPU-ATOMIC OPERATION VECTOR SUPPORT
+M: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+L: linux-kernel@vger.kernel.org
+S: Supported
+F: kernel/cpu_opv.c
+F: include/uapi/linux/cpu_opv.h
+
CRAMFS FILESYSTEM
M: Nicolas Pitre <nico@linaro.org>
S: Maintained
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2ff814c92f7f..c5af29eccd0e 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -68,6 +68,7 @@ struct perf_event_attr;
struct file_handle;
struct sigaltstack;
struct rseq;
+struct cpu_op;
union bpf_attr;
#include <linux/types.h>
@@ -906,6 +907,8 @@ asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
unsigned mask, struct statx __user *buffer);
asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
int flags, uint32_t sig);
+asmlinkage long sys_cpu_opv(struct cpu_op __user *ucpuopv, int cpuopcnt,
+ int cpu, int flags);
/*
* Architecture-specific system calls
diff --git a/include/uapi/linux/cpu_opv.h b/include/uapi/linux/cpu_opv.h
new file mode 100644
index 000000000000..350e5a7a61f2
--- /dev/null
+++ b/include/uapi/linux/cpu_opv.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_CPU_OPV_H
+#define _UAPI_LINUX_CPU_OPV_H
+
+/*
+ * linux/cpu_opv.h
+ *
+ * Per-CPU-atomic operation vector system call API
+ *
+ * Copyright (c) 2017-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ */
+
+#include <linux/types.h>
+
+/* Maximum size of operation structure within struct cpu_op. */
+#define CPU_OP_ARG_LEN_MAX 24
+/* Maximum data len for compare and memcpy operations. */
+#define CPU_OP_DATA_LEN_MAX 4096
+/* Maximum data len for arithmetic operations. */
+#define CPU_OP_ARITHMETIC_DATA_LEN_MAX 8
+
+enum cpu_op_flags {
+ CPU_OP_NR_FLAG = (1U << 0),
+ CPU_OP_VEC_LEN_MAX_FLAG = (1U << 1),
+};
+
+enum cpu_op_type {
+ /* compare */
+ CPU_COMPARE_EQ_OP,
+ CPU_COMPARE_NE_OP,
+ /* memcpy */
+ CPU_MEMCPY_OP,
+ CPU_MEMCPY_RELEASE_OP,
+ /* arithmetic */
+ CPU_ADD_OP,
+ CPU_ADD_RELEASE_OP,
+
+ NR_CPU_OPS,
+};
+
+/* Vector of operations to perform. Limited to 16. */
+struct cpu_op {
+ /* enum cpu_op_type. */
+ __s32 op;
+ /* data length, in bytes. */
+ __u32 len;
+ union {
+ struct {
+ __u64 a;
+ __u64 b;
+ __u8 expect_fault_a;
+ __u8 expect_fault_b;
+ } compare_op;
+ struct {
+ __u64 dst;
+ __u64 src;
+ __u8 expect_fault_dst;
+ __u8 expect_fault_src;
+ } memcpy_op;
+ struct {
+ __u64 p;
+ __s64 count;
+ __u8 expect_fault_p;
+ } arithmetic_op;
+ char __padding[CPU_OP_ARG_LEN_MAX];
+ } u;
+};
+
+#endif /* _UAPI_LINUX_CPU_OPV_H */
diff --git a/init/Kconfig b/init/Kconfig
index 1e234e2f1cba..e7c21a683642 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1483,6 +1483,8 @@ config RSEQ
bool "Enable rseq() system call" if EXPERT
default y
depends on HAVE_RSEQ
+ depends on MMU
+ select CPU_OPV
select MEMBARRIER
help
Enable the restartable sequences system call. It provides a
@@ -1502,6 +1504,22 @@ config DEBUG_RSEQ
If unsure, say N.
+# CPU_OPV depends on MMU for is_zero_pfn()
+config CPU_OPV
+ bool "Enable cpu_opv() system call" if EXPERT
+ default y
+ depends on MMU
+ help
+ Enable the per-CPU-atomic operation vector system call.
+ It allows user-space to perform a sequence of operations on
+ per-CPU data atomically with respect to concurrent execution on that
+ CPU. Useful as single-stepping fall-back for restartable sequences,
+ migration of data between per-CPU data structures, and for performing
+ more complex operations on per-CPU data that would not be otherwise
+ possible to do with restartable sequences.
+
+ If unsure, say Y.
+
config EMBEDDED
bool "Embedded system"
option allnoconfig_y
diff --git a/kernel/Makefile b/kernel/Makefile
index 7a63d567fdb5..507150b93521 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_TORTURE_TEST) += torture.o
obj-$(CONFIG_HAS_IOMEM) += iomem.o
obj-$(CONFIG_ZONE_DEVICE) += memremap.o
obj-$(CONFIG_RSEQ) += rseq.o
+obj-$(CONFIG_CPU_OPV) += cpu_opv.o
$(obj)/configs.o: $(obj)/config_data.h
diff --git a/kernel/cpu_opv.c b/kernel/cpu_opv.c
new file mode 100644
index 000000000000..6ee7ca3376be
--- /dev/null
+++ b/kernel/cpu_opv.c
@@ -0,0 +1,955 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Per-CPU-atomic operation vector system call
+ *
+ * It allows user-space to perform a sequence of operations on per-cpu
+ * data in the user-space address space atomically with respect to concurrent
+ * accesses from the same cpu. Useful as single-stepping fall-back for
+ * restartable sequences, and for performing more complex operations on per-cpu
+ * data that would not be otherwise possible to do with restartable sequences,
+ * such as migration of per-cpu data from one cpu to another.
+ *
+ * Copyright (C) 2017-2018 EfficiOS Inc.,
+ * Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ */
+
+#include <linux/sched.h>
+#include <linux/uaccess.h>
+#include <linux/syscalls.h>
+#include <linux/cpu_opv.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/pagemap.h>
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
+#include <linux/atomic.h>
+#include <linux/smp.h>
+#include <asm/ptrace.h>
+#include <asm/byteorder.h>
+#include <asm/cacheflush.h>
+
+#include "sched/sched.h"
+
+/*
+ * Maximum number of operations expected for a vector.
+ */
+#define CPU_OP_VEC_LEN_MAX 4
+
+/*
+ * Maximum data len for overall vector. Restrict the amount of user-space
+ * data touched by the kernel in interrupt-off or IPI handler context, so it
+ * does not introduce long interrupt latencies.
+ * cpu_opv allows one copy of up to 4096 bytes, and 3 operations touching 8
+ * bytes each.
+ * This limit is applied to the sum of length specified for all operations
+ * in a vector.
+ */
+#define CPU_OP_VEC_DATA_LEN_MAX (CPU_OP_DATA_LEN_MAX + \
+ (CPU_OP_VEC_LEN_MAX - 1) * CPU_OP_ARITHMETIC_DATA_LEN_MAX)
+
+/*
+ * Invocation of cpu_opv requires a maximum of 8 virtual address pointers.
+ * Keep those in an array on the stack of the cpu_opv system call.
+ */
+#define NR_VADDR 8
+
+/* Maximum pages per op. */
+#define CPU_OP_MAX_PAGES 4
+
+/* Maximum number of virtual addresses per op. */
+#define CPU_OP_VEC_MAX_ADDR (2 * CPU_OP_VEC_LEN_MAX)
+
+union op_fn_data {
+ uint8_t _u8;
+ uint16_t _u16;
+ uint32_t _u32;
+ uint64_t _u64;
+#if (BITS_PER_LONG < 64)
+ uint32_t _u64_split[2];
+#endif
+};
+
+struct vaddr {
+ unsigned long mem;
+ unsigned long uaddr;
+ struct page *pages[2];
+ unsigned int nr_pages;
+ int write;
+};
+
+struct cpu_opv_vaddr {
+ struct vaddr addr[NR_VADDR];
+ size_t nr_vaddr;
+};
+
+typedef int (*op_fn_t)(union op_fn_data *data, uint64_t v, uint32_t len);
+
+struct opv_ipi_args {
+ struct cpu_op *cpuop;
+ int cpuopcnt;
+ int ret;
+};
+
+/*
+ * Provide mutual exclution for threads executing a cpu_opv against an
+ * offline CPU.
+ */
+static DEFINE_MUTEX(cpu_opv_offline_lock);
+
+/*
+ * The cpu_opv system call executes a vector of operations on behalf of
+ * user-space on a specific CPU either with interrupts disabled or within
+ * a interrupt handler. It is inspired by readv() and writev() system
+ * calls which take a "struct iovec" array as argument.
+ *
+ * The operations available are: comparison, memcpy, memcpy_release, add,
+ * add_release. The system call receives a CPU number from user-space as
+ * argument, which is the CPU on which those operations need to be
+ * performed. All pointers in the ops must have been set up to point to
+ * the per CPU memory of the CPU on which the operations should be
+ * executed. The "comparison" operation can be used to check that the data
+ * used in the preparation step did not change between preparation of
+ * system call inputs and interpretation of the operation vector within
+ * the kernel.
+ *
+ * The reason why we require all pointer offsets to be calculated by
+ * user-space beforehand is because we need to use get_user_pages()
+ * to first pin all pages touched by each operation. This takes care of
+ * faulting-in the pages. Then, either interrupts are disabled if the
+ * target CPU is the current CPU, or an IPI is sent to the target CPU, and
+ * the operations are performed atomically with respect to other thread
+ * execution on that CPU, without generating any page fault. If the IPI
+ * nests over a restartable sequence critical section, it will abort that
+ * critical section.
+ *
+ * An overall maximum of 4120 bytes in enforced on the sum of operation
+ * length within an operation vector, so user-space cannot generate a
+ * too long interrupt-off critical section or IPI handler. The operation
+ * vector size is limited to 4 operations. The cache cold critical section
+ * duration has been measured as 4.7µs for 16 operations on x86-64. Each
+ * operation is also limited a length of 4096 bytes, meaning that an
+ * operation can touch a maximum of 4 pages (memcpy: 2 pages for source, 2
+ * pages for destination if addresses are not aligned on page boundaries).
+ *
+ * If the current thread running on the requested CPU, interrupts are
+ * disabled around interpretation of the operation vector. If the target
+ * CPU differs from the current CPU, an IPI is sent to the remote CPU
+ * to interpret the operation vector. If the remote CPU is offline, the
+ * operation vector is executed while holding a reference count preventing
+ * concurrent CPU hotplug changes, with cpu_opv_offline_lock mutex held.
+ */
+
+static unsigned long cpu_op_range_nr_pages(unsigned long addr,
+ unsigned long len)
+{
+ return ((addr + len - 1) >> PAGE_SHIFT) - (addr >> PAGE_SHIFT) + 1;
+}
+
+static int cpu_op_count_pages(u64 addr, unsigned long len)
+{
+ unsigned long nr_pages;
+
+ /*
+ * Validate that the address is within the process address space.
+ * This allows cast of those addresses to unsigned long throughout the
+ * rest of this system call, because it would be invalid to have an
+ * address over 4GB on a 32-bit kernel.
+ */
+ if (addr >= TASK_SIZE)
+ return -EINVAL;
+ if (!len)
+ return 0;
+ nr_pages = cpu_op_range_nr_pages(addr, len);
+ if (nr_pages > 2) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
+ return nr_pages;
+}
+
+/*
+ * Check operation types and length parameters. Count number of pages.
+ */
+static int cpu_opv_check_op(struct cpu_op *op, int *nr_vaddr, uint32_t *sum)
+{
+ int ret;
+
+ *sum += op->len;
+
+ /* Validate inputs. */
+ switch (op->op) {
+ case CPU_COMPARE_EQ_OP:
+ case CPU_COMPARE_NE_OP:
+ case CPU_MEMCPY_OP:
+ case CPU_MEMCPY_RELEASE_OP:
+ if (op->len > CPU_OP_DATA_LEN_MAX)
+ return -EINVAL;
+ break;
+ case CPU_ADD_OP:
+ case CPU_ADD_RELEASE_OP:
+ switch (op->len) {
+ case 1:
+ case 2:
+ case 4:
+ case 8:
+ break;
+ default:
+ return -EINVAL;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Validate pointers, count pages and virtual addresses. */
+ switch (op->op) {
+ case CPU_COMPARE_EQ_OP:
+ case CPU_COMPARE_NE_OP:
+ ret = cpu_op_count_pages(op->u.compare_op.a, op->len);
+ if (ret < 0)
+ return ret;
+ ret = cpu_op_count_pages(op->u.compare_op.b, op->len);
+ if (ret < 0)
+ return ret;
+ *nr_vaddr += 2;
+ break;
+ case CPU_MEMCPY_OP:
+ case CPU_MEMCPY_RELEASE_OP:
+ ret = cpu_op_count_pages(op->u.memcpy_op.dst, op->len);
+ if (ret < 0)
+ return ret;
+ ret = cpu_op_count_pages(op->u.memcpy_op.src, op->len);
+ if (ret < 0)
+ return ret;
+ *nr_vaddr += 2;
+ break;
+ case CPU_ADD_OP:
+ case CPU_ADD_RELEASE_OP:
+ ret = cpu_op_count_pages(op->u.arithmetic_op.p, op->len);
+ if (ret < 0)
+ return ret;
+ (*nr_vaddr)++;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+/*
+ * Check operation types and length parameters. Count number of pages.
+ */
+static int cpu_opv_check(struct cpu_op *cpuopv, int cpuopcnt, int *nr_vaddr)
+{
+ uint32_t sum = 0;
+ int i, ret;
+
+ for (i = 0; i < cpuopcnt; i++) {
+ ret = cpu_opv_check_op(&cpuopv[i], nr_vaddr, &sum);
+ if (ret)
+ return ret;
+ }
+ if (sum > CPU_OP_VEC_DATA_LEN_MAX)
+ return -EINVAL;
+ return 0;
+}
+
+static int cpu_op_check_page(struct page *page, int write)
+{
+ struct address_space *mapping;
+
+ if (is_zone_device_page(page))
+ return -EFAULT;
+
+ /*
+ * The page lock protects many things but in this context the page
+ * lock stabilizes mapping, prevents inode freeing in the shared
+ * file-backed region case and guards against movement to swap
+ * cache.
+ *
+ * Strictly speaking the page lock is not needed in all cases being
+ * considered here and page lock forces unnecessarily serialization
+ * From this point on, mapping will be re-verified if necessary and
+ * page lock will be acquired only if it is unavoidable
+ *
+ * Mapping checks require the head page for any compound page so the
+ * head page and mapping is looked up now.
+ */
+ page = compound_head(page);
+ mapping = READ_ONCE(page->mapping);
+
+ /*
+ * If page->mapping is NULL, then it cannot be a PageAnon page;
+ * but it might be the ZERO_PAGE (which is OK to read from), or
+ * in the gate area or in a special mapping (for which this
+ * check should fail); or it may have been a good file page when
+ * get_user_pages found it, but truncated or holepunched or
+ * subjected to invalidate_complete_page2 before the page lock
+ * is acquired (also cases which should fail). Given that a
+ * reference to the page is currently held, refcount care in
+ * invalidate_complete_page's remove_mapping prevents
+ * drop_caches from setting mapping to NULL concurrently.
+ *
+ * The case to guard against is when memory pressure cause
+ * shmem_writepage to move the page from filecache to swapcache
+ * concurrently: an unlikely race, but a retry for page->mapping
+ * is required in that situation.
+ */
+ if (!mapping) {
+ int shmem_swizzled;
+
+ /*
+ * Check again with page lock held to guard against
+ * memory pressure making shmem_writepage move the page
+ * from filecache to swapcache.
+ */
+ lock_page(page);
+ shmem_swizzled = PageSwapCache(page) || page->mapping;
+ unlock_page(page);
+ if (shmem_swizzled)
+ return -EAGAIN;
+ /*
+ * It is valid to read from, but invalid to write to the
+ * ZERO_PAGE.
+ */
+ if (!(is_zero_pfn(page_to_pfn(page)) ||
+ is_huge_zero_page(page)) || write)
+ return -EFAULT;
+ }
+ return 0;
+}
+
+static int cpu_op_check_pages(struct page **pages,
+ unsigned long nr_pages,
+ int write)
+{
+ unsigned long i;
+
+ for (i = 0; i < nr_pages; i++) {
+ int ret;
+
+ ret = cpu_op_check_page(pages[i], write);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int cpu_op_pin_pages(unsigned long addr, unsigned long len,
+ struct cpu_opv_vaddr *vaddr_ptrs,
+ unsigned long *vaddr, int write)
+{
+ struct page *pages[2];
+ struct vm_area_struct *vmas[2];
+ int ret, nr_pages, nr_put_pages, n;
+ unsigned long _vaddr;
+ struct vaddr *va;
+ struct mm_struct *mm = current->mm;
+
+ nr_pages = cpu_op_count_pages(addr, len);
+ if (nr_pages <= 0)
+ return nr_pages;
+again:
+ down_read(&mm->mmap_sem);
+ ret = get_user_pages(addr, nr_pages, write ? FOLL_WRITE : 0, pages,
+ vmas);
+ if (ret < nr_pages) {
+ if (ret >= 0) {
+ nr_put_pages = ret;
+ ret = -EFAULT;
+ } else {
+ nr_put_pages = 0;
+ }
+ up_read(&mm->mmap_sem);
+ goto error;
+ }
+ /*
+ * cpu_opv() accesses its own cached mapping of the userspace pages.
+ * Considering that concurrent noncached and cached accesses may yield
+ * to unexpected results in terms of memory consistency, explicitly
+ * disallow cpu_opv on noncached memory.
+ */
+ for (n = 0; n < nr_pages; n++) {
+ if (is_vma_noncached(vmas[n])) {
+ nr_put_pages = nr_pages;
+ ret = -EFAULT;
+ up_read(&mm->mmap_sem);
+ goto error;
+ }
+ }
+ up_read(&mm->mmap_sem);
+ ret = cpu_op_check_pages(pages, nr_pages, write);
+ if (ret) {
+ nr_put_pages = nr_pages;
+ goto error;
+ }
+ _vaddr = (unsigned long)vm_map_user_ram(pages, nr_pages, addr,
+ numa_node_id(), PAGE_KERNEL);
+ if (!_vaddr) {
+ nr_put_pages = nr_pages;
+ ret = -ENOMEM;
+ goto error;
+ }
+ va = &vaddr_ptrs->addr[vaddr_ptrs->nr_vaddr++];
+ va->mem = _vaddr;
+ va->uaddr = addr;
+ for (n = 0; n < nr_pages; n++)
+ va->pages[n] = pages[n];
+ va->nr_pages = nr_pages;
+ va->write = write;
+ *vaddr = _vaddr + (addr & ~PAGE_MASK);
+ return 0;
+
+error:
+ for (n = 0; n < nr_put_pages; n++)
+ put_page(pages[n]);
+ /*
+ * Retry if a page has been faulted in, or is being swapped in.
+ */
+ if (ret == -EAGAIN)
+ goto again;
+ return ret;
+}
+
+static int cpu_opv_pin_pages_op(struct cpu_op *op,
+ struct cpu_opv_vaddr *vaddr_ptrs,
+ bool *expect_fault)
+{
+ int ret;
+ unsigned long vaddr = 0;
+
+ switch (op->op) {
+ case CPU_COMPARE_EQ_OP:
+ case CPU_COMPARE_NE_OP:
+ ret = -EFAULT;
+ *expect_fault = op->u.compare_op.expect_fault_a;
+ if (!access_ok(VERIFY_READ,
+ (void __user *)(unsigned long)op->u.compare_op.a,
+ op->len))
+ return ret;
+ ret = cpu_op_pin_pages(op->u.compare_op.a, op->len,
+ vaddr_ptrs, &vaddr, 0);
+ if (ret)
+ return ret;
+ op->u.compare_op.a = vaddr;
+ ret = -EFAULT;
+ *expect_fault = op->u.compare_op.expect_fault_b;
+ if (!access_ok(VERIFY_READ,
+ (void __user *)(unsigned long)op->u.compare_op.b,
+ op->len))
+ return ret;
+ ret = cpu_op_pin_pages(op->u.compare_op.b, op->len,
+ vaddr_ptrs, &vaddr, 0);
+ if (ret)
+ return ret;
+ op->u.compare_op.b = vaddr;
+ break;
+ case CPU_MEMCPY_OP:
+ case CPU_MEMCPY_RELEASE_OP:
+ ret = -EFAULT;
+ *expect_fault = op->u.memcpy_op.expect_fault_dst;
+ if (!access_ok(VERIFY_WRITE,
+ (void __user *)(unsigned long)op->u.memcpy_op.dst,
+ op->len))
+ return ret;
+ ret = cpu_op_pin_pages(op->u.memcpy_op.dst, op->len,
+ vaddr_ptrs, &vaddr, 1);
+ if (ret)
+ return ret;
+ op->u.memcpy_op.dst = vaddr;
+ ret = -EFAULT;
+ *expect_fault = op->u.memcpy_op.expect_fault_src;
+ if (!access_ok(VERIFY_READ,
+ (void __user *)(unsigned long)op->u.memcpy_op.src,
+ op->len))
+ return ret;
+ ret = cpu_op_pin_pages(op->u.memcpy_op.src, op->len,
+ vaddr_ptrs, &vaddr, 0);
+ if (ret)
+ return ret;
+ op->u.memcpy_op.src = vaddr;
+ break;
+ case CPU_ADD_OP:
+ case CPU_ADD_RELEASE_OP:
+ ret = -EFAULT;
+ *expect_fault = op->u.arithmetic_op.expect_fault_p;
+ if (!access_ok(VERIFY_WRITE,
+ (void __user *)(unsigned long)op->u.arithmetic_op.p,
+ op->len))
+ return ret;
+ ret = cpu_op_pin_pages(op->u.arithmetic_op.p, op->len,
+ vaddr_ptrs, &vaddr, 1);
+ if (ret)
+ return ret;
+ op->u.arithmetic_op.p = vaddr;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int cpu_opv_pin_pages(struct cpu_op *cpuop, int cpuopcnt,
+ struct cpu_opv_vaddr *vaddr_ptrs)
+{
+ int ret, i;
+ bool expect_fault = false;
+
+ /* Check access, pin pages. */
+ for (i = 0; i < cpuopcnt; i++) {
+ ret = cpu_opv_pin_pages_op(&cpuop[i], vaddr_ptrs,
+ &expect_fault);
+ if (ret)
+ goto error;
+ }
+ return 0;
+
+error:
+ /*
+ * If faulting access is expected, return EAGAIN to user-space.
+ * It allows user-space to distinguish between a fault caused by
+ * an access which is expect to fault (e.g. due to concurrent
+ * unmapping of underlying memory) from an unexpected fault from
+ * which a retry would not recover.
+ */
+ if (ret == -EFAULT && expect_fault)
+ return -EAGAIN;
+ return ret;
+}
+
+static int __op_get(union op_fn_data *data, void *p, size_t len)
+{
+ switch (len) {
+ case 1:
+ data->_u8 = READ_ONCE(*(uint8_t *)p);
+ break;
+ case 2:
+ data->_u16 = READ_ONCE(*(uint16_t *)p);
+ break;
+ case 4:
+ data->_u32 = READ_ONCE(*(uint32_t *)p);
+ break;
+ case 8:
+#if (BITS_PER_LONG == 64)
+ data->_u64 = READ_ONCE(*(uint64_t *)p);
+#else
+ {
+ data->_u64_split[0] = READ_ONCE(*(uint32_t *)p);
+ data->_u64_split[1] = READ_ONCE(*((uint32_t *)p + 1));
+ }
+#endif
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int __op_put(union op_fn_data *data, void *p, size_t len, bool release)
+{
+ switch (len) {
+ case 1:
+ if (release)
+ smp_store_release((uint8_t *)p, data->_u8);
+ else
+ WRITE_ONCE(*(uint8_t *)p, data->_u8);
+ break;
+ case 2:
+ if (release)
+ smp_store_release((uint16_t *)p, data->_u16);
+ else
+ WRITE_ONCE(*(uint16_t *)p, data->_u16);
+ break;
+ case 4:
+ if (release)
+ smp_store_release((uint32_t *)p, data->_u32);
+ else
+ WRITE_ONCE(*(uint32_t *)p, data->_u32);
+ break;
+ case 8:
+#if (BITS_PER_LONG == 64)
+ if (release)
+ smp_store_release((uint64_t *)p, data->_u64);
+ else
+ WRITE_ONCE(*(uint64_t *)p, data->_u64);
+#else
+ {
+ if (release)
+ smp_store_release((uint32_t *)p, data->_u64_split[0]);
+ else
+ WRITE_ONCE(*(uint32_t *)p, data->_u64_split[0]);
+ WRITE_ONCE(*((uint32_t *)p + 1), data->_u64_split[1]);
+ }
+#endif
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+/* Return 0 if same, > 0 if different, < 0 on error. */
+static int do_cpu_op_compare(unsigned long _a, unsigned long _b, uint32_t len)
+{
+ void *a = (void *)_a;
+ void *b = (void *)_b;
+ union op_fn_data tmp[2];
+ int ret;
+
+ switch (len) {
+ case 1:
+ case 2:
+ case 4:
+ case 8:
+ if (!IS_ALIGNED(_a, len) || !IS_ALIGNED(_b, len))
+ goto memcmp;
+ break;
+ default:
+ goto memcmp;
+ }
+
+ ret = __op_get(&tmp[0], a, len);
+ if (ret)
+ return ret;
+ ret = __op_get(&tmp[1], b, len);
+ if (ret)
+ return ret;
+
+ switch (len) {
+ case 1:
+ ret = !!(tmp[0]._u8 != tmp[1]._u8);
+ break;
+ case 2:
+ ret = !!(tmp[0]._u16 != tmp[1]._u16);
+ break;
+ case 4:
+ ret = !!(tmp[0]._u32 != tmp[1]._u32);
+ break;
+ case 8:
+ ret = !!(tmp[0]._u64 != tmp[1]._u64);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return ret;
+
+memcmp:
+ if (memcmp(a, b, len))
+ return 1;
+ return 0;
+}
+
+/* Return 0 on success, < 0 on error. */
+static int do_cpu_op_memcpy(unsigned long _dst, unsigned long _src,
+ uint32_t len, bool release)
+{
+ void *dst = (void *)_dst;
+ void *src = (void *)_src;
+ union op_fn_data tmp;
+ int ret;
+
+ switch (len) {
+ case 1:
+ case 2:
+ case 4:
+ case 8:
+ if (!IS_ALIGNED(_dst, len) || !IS_ALIGNED(_src, len))
+ goto memcpy;
+ break;
+ default:
+ goto memcpy;
+ }
+
+ ret = __op_get(&tmp, src, len);
+ if (ret)
+ return ret;
+ return __op_put(&tmp, dst, len, release);
+
+memcpy:
+ if (release)
+ smp_mb();
+ memcpy(dst, src, len);
+ return 0;
+}
+
+static int op_add_fn(union op_fn_data *data, uint64_t count, uint32_t len)
+{
+ switch (len) {
+ case 1:
+ data->_u8 += (uint8_t)count;
+ break;
+ case 2:
+ data->_u16 += (uint16_t)count;
+ break;
+ case 4:
+ data->_u32 += (uint32_t)count;
+ break;
+ case 8:
+ data->_u64 += (uint64_t)count;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+/* Return 0 on success, < 0 on error. */
+static int do_cpu_op_fn(op_fn_t op_fn, unsigned long _p, uint64_t v,
+ uint32_t len, bool release)
+{
+ union op_fn_data tmp;
+ void *p = (void *)_p;
+ int ret;
+
+ ret = __op_get(&tmp, p, len);
+ if (ret)
+ return ret;
+ ret = op_fn(&tmp, v, len);
+ if (ret)
+ return ret;
+ ret = __op_put(&tmp, p, len, release);
+ if (ret)
+ return ret;
+ return 0;
+}
+
+/*
+ * Return negative value on error, positive value if comparison
+ * fails, 0 on success.
+ */
+static int __do_cpu_opv_op(struct cpu_op *op)
+{
+ /* Guarantee a compiler barrier between each operation. */
+ barrier();
+
+ switch (op->op) {
+ case CPU_COMPARE_EQ_OP:
+ return do_cpu_op_compare(op->u.compare_op.a,
+ op->u.compare_op.b,
+ op->len);
+ case CPU_COMPARE_NE_OP:
+ {
+ int ret;
+
+ ret = do_cpu_op_compare(op->u.compare_op.a,
+ op->u.compare_op.b,
+ op->len);
+ if (ret < 0)
+ return ret;
+ /*
+ * Stop execution, return positive value if comparison
+ * is identical.
+ */
+ if (ret == 0)
+ return 1;
+ return 0;
+ }
+ case CPU_MEMCPY_OP:
+ return do_cpu_op_memcpy(op->u.memcpy_op.dst,
+ op->u.memcpy_op.src,
+ op->len, false);
+ case CPU_MEMCPY_RELEASE_OP:
+ return do_cpu_op_memcpy(op->u.memcpy_op.dst,
+ op->u.memcpy_op.src,
+ op->len, true);
+ case CPU_ADD_OP:
+ return do_cpu_op_fn(op_add_fn, op->u.arithmetic_op.p,
+ op->u.arithmetic_op.count, op->len, false);
+ case CPU_ADD_RELEASE_OP:
+ return do_cpu_op_fn(op_add_fn, op->u.arithmetic_op.p,
+ op->u.arithmetic_op.count, op->len, true);
+ default:
+ return -EINVAL;
+ }
+}
+
+static int __do_cpu_opv(struct cpu_op *cpuop, int cpuopcnt)
+{
+ int i, ret;
+
+ for (i = 0; i < cpuopcnt; i++) {
+ ret = __do_cpu_opv_op(&cpuop[i]);
+ /* If comparison fails, stop execution and return index + 1. */
+ if (ret > 0)
+ return i + 1;
+ /* On error, stop execution. */
+ if (ret < 0)
+ return ret;
+ }
+ return 0;
+}
+
+/*
+ * Check that the page pointers pinned by get_user_pages()
+ * are still in the page table. Invoked with mmap_sem held.
+ * Return 0 if pointers match, -EAGAIN if they don't.
+ */
+static int vaddr_check(struct vaddr *vaddr)
+{
+ struct page *pages[2];
+ int ret, n;
+
+ ret = __get_user_pages_fast(vaddr->uaddr, vaddr->nr_pages,
+ vaddr->write, pages);
+ for (n = 0; n < ret; n++)
+ put_page(pages[n]);
+ if (ret < vaddr->nr_pages) {
+ ret = get_user_pages(vaddr->uaddr, vaddr->nr_pages,
+ vaddr->write ? FOLL_WRITE : 0,
+ pages, NULL);
+ if (ret < 0)
+ return -EAGAIN;
+ for (n = 0; n < ret; n++)
+ put_page(pages[n]);
+ if (ret < vaddr->nr_pages)
+ return -EAGAIN;
+ }
+ for (n = 0; n < vaddr->nr_pages; n++) {
+ if (pages[n] != vaddr->pages[n])
+ return -EAGAIN;
+ }
+ return 0;
+}
+
+static int vaddr_ptrs_check(struct cpu_opv_vaddr *vaddr_ptrs)
+{
+ int i;
+
+ for (i = 0; i < vaddr_ptrs->nr_vaddr; i++) {
+ int ret;
+
+ ret = vaddr_check(&vaddr_ptrs->addr[i]);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static void cpu_opv_ipi(void *info)
+{
+ struct opv_ipi_args *args = info;
+
+ rseq_preempt(current);
+ args->ret = __do_cpu_opv(args->cpuop, args->cpuopcnt);
+}
+
+static int do_cpu_opv(struct cpu_op *cpuop, int cpuopcnt,
+ struct cpu_opv_vaddr *vaddr_ptrs, int cpu)
+{
+ struct mm_struct *mm = current->mm;
+ struct opv_ipi_args args = {
+ .cpuop = cpuop,
+ .cpuopcnt = cpuopcnt,
+ };
+ int ret;
+
+retry:
+ if (!cpumask_test_cpu(cpu, ¤t->cpus_allowed))
+ return -EINVAL;
+ down_read(&mm->mmap_sem);
+ ret = vaddr_ptrs_check(vaddr_ptrs);
+ if (ret)
+ goto end;
+ ret = smp_call_function_single(cpu, cpu_opv_ipi, &args, 1);
+ if (ret) {
+ up_read(&mm->mmap_sem);
+ goto check_online;
+ }
+ ret = args.ret;
+end:
+ up_read(&mm->mmap_sem);
+ return ret;
+
+check_online:
+ get_online_cpus();
+ if (cpu_online(cpu)) {
+ put_online_cpus();
+ goto retry;
+ }
+ /*
+ * CPU is offline. Perform operation from the current CPU with
+ * cpu_online read lock held, preventing that CPU from coming online,
+ * and with mutex held, providing mutual exclusion against other
+ * CPUs also finding out about an offline CPU.
+ */
+ down_read(&mm->mmap_sem);
+ ret = vaddr_ptrs_check(vaddr_ptrs);
+ if (ret)
+ goto offline_end;
+ mutex_lock(&cpu_opv_offline_lock);
+ ret = __do_cpu_opv(cpuop, cpuopcnt);
+ mutex_unlock(&cpu_opv_offline_lock);
+offline_end:
+ up_read(&mm->mmap_sem);
+ put_online_cpus();
+ return ret;
+}
+
+/*
+ * cpu_opv - execute operation vector on a given CPU in interrupt context.
+ *
+ * Userspace should pass the CPU number on which the operation vector
+ * should be executed as parameter.
+ */
+SYSCALL_DEFINE4(cpu_opv, struct cpu_op __user *, ucpuopv, int, cpuopcnt,
+ int, cpu, int, flags)
+{
+ struct cpu_op cpuopv[CPU_OP_VEC_LEN_MAX];
+ struct cpu_opv_vaddr vaddr_ptrs = {
+ .nr_vaddr = 0,
+ };
+ int ret, i, nr_vaddr = 0;
+ bool retry = false;
+
+ if (unlikely(flags & ~(CPU_OP_NR_FLAG | CPU_OP_VEC_LEN_MAX_FLAG)))
+ return -EINVAL;
+ if (flags & CPU_OP_NR_FLAG) {
+ if (flags & CPU_OP_VEC_LEN_MAX_FLAG)
+ return -EINVAL;
+ return NR_CPU_OPS;
+ }
+ if (flags & CPU_OP_VEC_LEN_MAX_FLAG)
+ return CPU_OP_VEC_LEN_MAX;
+ if (unlikely(cpu < 0))
+ return -EINVAL;
+ if (cpuopcnt < 0 || cpuopcnt > CPU_OP_VEC_LEN_MAX)
+ return -EINVAL;
+ if (copy_from_user(cpuopv, ucpuopv, cpuopcnt * sizeof(struct cpu_op)))
+ return -EFAULT;
+ ret = cpu_opv_check(cpuopv, cpuopcnt, &nr_vaddr);
+ if (ret)
+ return ret;
+ if (nr_vaddr > NR_VADDR)
+ return -EINVAL;
+again:
+ ret = cpu_opv_pin_pages(cpuopv, cpuopcnt, &vaddr_ptrs);
+ if (ret)
+ goto end;
+ ret = do_cpu_opv(cpuopv, cpuopcnt, &vaddr_ptrs, cpu);
+ if (ret == -EAGAIN)
+ retry = true;
+end:
+ for (i = 0; i < vaddr_ptrs.nr_vaddr; i++) {
+ struct vaddr *vaddr = &vaddr_ptrs.addr[i];
+ int j;
+
+ vm_unmap_user_ram((void *)vaddr->mem, vaddr->nr_pages);
+ for (j = 0; j < vaddr->nr_pages; j++) {
+ if (vaddr->write)
+ set_page_dirty(vaddr->pages[j]);
+ put_page(vaddr->pages[j]);
+ }
+ }
+ /*
+ * Force vm_map flush to ensure we don't exhaust available vmalloc
+ * address space.
+ */
+ if (vaddr_ptrs.nr_vaddr)
+ vm_unmap_aliases();
+ if (retry) {
+ retry = false;
+ vaddr_ptrs.nr_vaddr = 0;
+ goto again;
+ }
+ return ret;
+}
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index df556175be50..0a6410d77c33 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -435,3 +435,4 @@ COND_SYSCALL(setuid16);
/* restartable sequence */
COND_SYSCALL(rseq);
+COND_SYSCALL(cpu_opv);
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 05/16] mm: Provide is_vma_noncached
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Provide is_vma_noncached() static inline to allow generic code to
check whether the given vma consists of noncached memory.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: linux-mm@kvack.org
---
include/linux/mm.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0416a7204be3..18acf4f339f8 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2551,6 +2551,30 @@ static inline struct page *follow_page(struct vm_area_struct *vma,
return follow_page_mask(vma, address, foll_flags, &unused_page_mask);
}
+static inline bool pgprot_same(pgprot_t a, pgprot_t b)
+{
+ return pgprot_val(a) == pgprot_val(b);
+}
+
+#ifdef pgprot_noncached
+static inline bool is_vma_noncached(struct vm_area_struct *vma)
+{
+ pgprot_t pgprot = vma->vm_page_prot;
+
+ /* Check whether architecture implements noncached pages. */
+ if (pgprot_same(pgprot_noncached(PAGE_KERNEL), PAGE_KERNEL))
+ return false;
+ if (!pgprot_same(pgprot, pgprot_noncached(pgprot)))
+ return false;
+ return true;
+}
+#else
+static inline bool is_vma_noncached(struct vm_area_struct *vma)
+{
+ return false;
+}
+#endif
+
#define FOLL_WRITE 0x01 /* check pte is writable */
#define FOLL_TOUCH 0x02 /* mark page accessed */
#define FOLL_GET 0x04 /* do get_page on page */
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 04/16] mm: Introduce vm_map_user_ram, vm_unmap_user_ram (v2)
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
Create and destroy mappings aliased to a user-space mapping with the same
cache coloring as the userspace mapping. Allow the kernel to load from
and store to pages shared with user-space through its own mapping in
kernel virtual addresses while ensuring cache conherency between kernel
and userspace mappings for virtually aliased architectures.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
---
Changes since v1:
- Use WARN_ON() rather than BUG_ON().
---
include/linux/vmalloc.h | 4 +++
mm/vmalloc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 398e9c95cd61..899657b3d469 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -59,6 +59,10 @@ struct vmap_area {
extern void vm_unmap_ram(const void *mem, unsigned int count);
extern void *vm_map_ram(struct page **pages, unsigned int count,
int node, pgprot_t prot);
+extern void vm_unmap_user_ram(const void *mem, unsigned int count);
+extern void *vm_map_user_ram(struct page **pages, unsigned int count,
+ unsigned long uaddr, int node, pgprot_t prot);
+
extern void vm_unmap_aliases(void);
#ifdef CONFIG_MMU
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a236bac872f0..8df3c572036c 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1188,6 +1188,72 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node, pgprot_t pro
}
EXPORT_SYMBOL(vm_map_ram);
+/**
+ * vm_unmap_user_ram - unmap linear kernel address space set up by vm_map_user_ram
+ * @mem: the pointer returned by vm_map_user_ram
+ * @count: the count passed to that vm_map_user_ram call (cannot unmap partial)
+ */
+void vm_unmap_user_ram(const void *mem, unsigned int count)
+{
+ unsigned long size = (unsigned long)count << PAGE_SHIFT;
+ unsigned long addr = (unsigned long)mem;
+ struct vmap_area *va;
+
+ might_sleep();
+ if (WARN_ON(!addr) ||
+ WARN_ON(addr < VMALLOC_START) ||
+ WARN_ON(addr > VMALLOC_END) ||
+ WARN_ON(!PAGE_ALIGNED(addr)))
+ return;
+
+ debug_check_no_locks_freed(mem, size);
+ va = find_vmap_area(addr);
+ if (WARN_ON(!va))
+ return;
+ free_unmap_vmap_area(va);
+}
+EXPORT_SYMBOL(vm_unmap_user_ram);
+
+/**
+ * vm_map_user_ram - map user space pages linearly into kernel virtual address
+ * @pages: an array of pointers to the virtually contiguous pages to be mapped
+ * @count: number of pages
+ * @uaddr: address within the first page in the userspace mapping
+ * @node: prefer to allocate data structures on this node
+ * @prot: memory protection to use. PAGE_KERNEL for regular RAM
+ *
+ * Create a mapping aliased to a user-space mapping with the same cache
+ * coloring as the userspace mapping. Allow the kernel to load from and
+ * store to pages shared with user-space through its own mapping in kernel
+ * virtual addresses while ensuring cache conherency between kernel and
+ * userspace mappings for virtually aliased architectures.
+ *
+ * Returns: a pointer to the address that has been mapped, or %NULL on failure
+ */
+void *vm_map_user_ram(struct page **pages, unsigned int count,
+ unsigned long uaddr, int node, pgprot_t prot)
+{
+ unsigned long size = (unsigned long)count << PAGE_SHIFT;
+ unsigned long va_offset = ALIGN_DOWN(uaddr, PAGE_SIZE) & (SHMLBA - 1);
+ unsigned long alloc_size = ALIGN(va_offset + size, SHMLBA);
+ struct vmap_area *va;
+ unsigned long addr;
+ void *mem;
+
+ va = alloc_vmap_area(alloc_size, SHMLBA, VMALLOC_START, VMALLOC_END,
+ node, GFP_KERNEL);
+ if (IS_ERR(va))
+ return NULL;
+ addr = va->va_start + va_offset;
+ mem = (void *)addr;
+ if (vmap_page_range(addr, addr + size, prot, pages) < 0) {
+ vm_unmap_user_ram(mem, count);
+ return NULL;
+ }
+ return mem;
+}
+EXPORT_SYMBOL(vm_map_user_ram);
+
static struct vm_struct *vmlist __initdata;
/**
* vm_area_add_early - add vmap area early during boot
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 03/16] mm: Replace BUG_ON() by WARN_ON() in vm_unmap_ram()
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
It is encouraged to warn and return rather than use BUG_ON() when
the condition can be recovered from in ways that are more graceful than
halting the whole system.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
CC: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
CC: Matthew Wilcox <mawilcox@microsoft.com>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Paul Turner <pjt@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andi Kleen <andi@firstfloor.org>
CC: Dave Watson <davejwatson@fb.com>
CC: Chris Lameter <cl@linux.com>
CC: Ingo Molnar <mingo@redhat.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Ben Maurer <bmaurer@fb.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Russell King <linux@arm.linux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
---
mm/vmalloc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index a728fc492557..a236bac872f0 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1123,10 +1123,11 @@ void vm_unmap_ram(const void *mem, unsigned int count)
struct vmap_area *va;
might_sleep();
- BUG_ON(!addr);
- BUG_ON(addr < VMALLOC_START);
- BUG_ON(addr > VMALLOC_END);
- BUG_ON(!PAGE_ALIGNED(addr));
+ if (WARN_ON(!addr) ||
+ WARN_ON(addr < VMALLOC_START) ||
+ WARN_ON(addr > VMALLOC_END) ||
+ WARN_ON(!PAGE_ALIGNED(addr)))
+ return;
if (likely(count <= VMAP_MAX_ALLOC)) {
debug_check_no_locks_freed(mem, size);
@@ -1135,7 +1136,8 @@ void vm_unmap_ram(const void *mem, unsigned int count)
}
va = find_vmap_area(addr);
- BUG_ON(!va);
+ if (WARN_ON(!va))
+ return;
debug_check_no_locks_freed((void *)va->va_start,
(va->va_end - va->va_start));
free_unmap_vmap_area(va);
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 02/16] rseq/selftests: Adapt number of threads to the number of detected cpus
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
On smaller systems, running a test with 200 threads can take a long
time on machines with smaller number of CPUs.
Detect the number of online cpus at test runtime, and multiply that
by 6 to have 6 rseq threads per cpu preempting each other.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Watson <davejwatson@fb.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: linux-kselftest@vger.kernel.org
Cc: "H . Peter Anvin" <hpa@zytor.com>
Cc: Chris Lameter <cl@linux.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Paul Turner <pjt@google.com>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ben Maurer <bmaurer@fb.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
tools/testing/selftests/rseq/run_param_test.sh | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/rseq/run_param_test.sh b/tools/testing/selftests/rseq/run_param_test.sh
index 3acd6d75ff9f..e426304fd4a0 100755
--- a/tools/testing/selftests/rseq/run_param_test.sh
+++ b/tools/testing/selftests/rseq/run_param_test.sh
@@ -1,6 +1,8 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0+ or MIT
+NR_CPUS=`grep '^processor' /proc/cpuinfo | wc -l`
+
EXTRA_ARGS=${@}
OLDIFS="$IFS"
@@ -28,15 +30,16 @@ IFS="$OLDIFS"
REPS=1000
SLOW_REPS=100
+NR_THREADS=$((6*${NR_CPUS}))
function do_tests()
{
local i=0
while [ "$i" -lt "${#TEST_LIST[@]}" ]; do
echo "Running test ${TEST_NAME[$i]}"
- ./param_test ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+ ./param_test ${TEST_LIST[$i]} -r ${REPS} -t ${NR_THREADS} ${@} ${EXTRA_ARGS} || exit 1
echo "Running compare-twice test ${TEST_NAME[$i]}"
- ./param_test_compare_twice ${TEST_LIST[$i]} -r ${REPS} ${@} ${EXTRA_ARGS} || exit 1
+ ./param_test_compare_twice ${TEST_LIST[$i]} -r ${REPS} -t ${NR_THREADS} ${@} ${EXTRA_ARGS} || exit 1
let "i++"
done
}
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 01/16] rseq/selftests: Expose reference counter to coexist with glibc (v2)
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
In-Reply-To: <20181101095844.24462-1-mathieu.desnoyers@efficios.com>
In order to integrate rseq into user-space applications, expose a
reference counter TLS as a __rseq_refcount weak symbol so many rseq
users can be linked into the same application (e.g. librseq and glibc).
The reference count ensures that rseq syscall
registration/unregistration happens only for the most early/late user
for each thread, thus ensuring that rseq is registered across the
lifetime of all rseq users for a given thread.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Shuah Khan <shuah@kernel.org>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: linux-api@vger.kernel.org
---
Changes since v1:
- Error out on refcount overflow/underflow.
- Expose __rseq_refcount weak symbol.
---
tools/testing/selftests/rseq/rseq.c | 23 ++++++++++++++++-------
tools/testing/selftests/rseq/rseq.h | 1 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq.c b/tools/testing/selftests/rseq/rseq.c
index 4847e97ed049..835e3917d220 100644
--- a/tools/testing/selftests/rseq/rseq.c
+++ b/tools/testing/selftests/rseq/rseq.c
@@ -25,18 +25,19 @@
#include <syscall.h>
#include <assert.h>
#include <signal.h>
+#include <limits.h>
#include "rseq.h"
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
-__attribute__((tls_model("initial-exec"))) __thread
+__attribute__((weak)) __thread
volatile struct rseq __rseq_abi = {
.cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
};
-static __attribute__((tls_model("initial-exec"))) __thread
-volatile int refcount;
+__attribute__((weak)) __thread
+volatile uint32_t __rseq_refcount;
static void signal_off_save(sigset_t *oldset)
{
@@ -70,7 +71,11 @@ int rseq_register_current_thread(void)
sigset_t oldset;
signal_off_save(&oldset);
- if (refcount++)
+ if (__rseq_refcount == UINT_MAX) {
+ ret = -1;
+ goto end;
+ }
+ if (__rseq_refcount++)
goto end;
rc = sys_rseq(&__rseq_abi, sizeof(struct rseq), 0, RSEQ_SIG);
if (!rc) {
@@ -78,9 +83,9 @@ int rseq_register_current_thread(void)
goto end;
}
if (errno != EBUSY)
- __rseq_abi.cpu_id = -2;
+ __rseq_abi.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
ret = -1;
- refcount--;
+ __rseq_refcount--;
end:
signal_restore(oldset);
return ret;
@@ -92,7 +97,11 @@ int rseq_unregister_current_thread(void)
sigset_t oldset;
signal_off_save(&oldset);
- if (--refcount)
+ if (!__rseq_refcount) {
+ ret = -1;
+ goto end;
+ }
+ if (--__rseq_refcount)
goto end;
rc = sys_rseq(&__rseq_abi, sizeof(struct rseq),
RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
diff --git a/tools/testing/selftests/rseq/rseq.h b/tools/testing/selftests/rseq/rseq.h
index c72eb70f9b52..47f815c19cef 100644
--- a/tools/testing/selftests/rseq/rseq.h
+++ b/tools/testing/selftests/rseq/rseq.h
@@ -45,6 +45,7 @@
#endif
extern __thread volatile struct rseq __rseq_abi;
+extern __thread volatile uint32_t __rseq_refcount;
#define rseq_likely(x) __builtin_expect(!!(x), 1)
#define rseq_unlikely(x) __builtin_expect(!!(x), 0)
--
2.11.0
^ permalink raw reply related
* [RFC PATCH for 4.21 00/16] rseq updates, new cpu_opv system call (v2)
From: Mathieu Desnoyers @ 2018-11-01 9:58 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
Cc: linux-kernel, linux-api, Thomas Gleixner, Andy Lutomirski,
Dave Watson, Paul Turner, Andrew Morton, Russell King,
Ingo Molnar, H . Peter Anvin, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Linus Torvalds,
Catalin Marinas, Will Deacon, Michael Kerrisk, Joel Fernandes,
Mathieu Desnoyers
Hi,
Here is an updated patchset submitted as RFC for 4.21 (next merge
window).
This series contain:
- rseq selftests:
- Added reference counter within user-space __rseq_abi structure, for
integration of rseq application/libraries with future use by glibc,
- Adapt number of threads to the number of online cpus.
- cpu_opv:
- Introduce vm_map_user_ram()/vm_unmap_user_ram() (mm),
- Provide is_vma_noncached() (mm),
- Introduce cpu_opv system call, with vmap space limiting,
- Wire up cpu_opv on x86, powerpc, arm,
- Provide cpu_opv selftests.
The cpu_opv system call covers the use-cases that rseq does not handle,
namely single-stepping with debuggers, moving data between per-cpu data
structures without interfering with cpu affinity masks, and using rseq
from signal handlers nested between thread creation and rseq
registration by glibc, or between rseq unregistration by glibc and
thread teardown.
The cpu_opv system call has been greatly simplified since the last round
based on feedback from Peter Zijlstra and Will Deacon at OSS Europe.
Major simplifications are:
- Remove unnecessary operations. Only keep compare, memcpy,
memcpy_release, add, add_release,
- Remove the "mb" instruction in favor of a release semantic,
- Use IPI to execute operations on remote CPUs rather than try to
migrate the current thread,
- Reduce the maximum operation vector size from 16 to 4 elements,
thus removing the need to perform memory allocation in the cpu_opv
system call (there is enough space on the stack). Add a new flag
allowing user-space to query the maximum vector size supported by
the kernel for future extensibility.
Feedback is welcome!
Thanks,
Mathieu
Mathieu Desnoyers (16):
rseq/selftests: Expose reference counter to coexist with glibc (v2)
rseq/selftests: Adapt number of threads to the number of detected cpus
mm: Replace BUG_ON() by WARN_ON() in vm_unmap_ram()
mm: Introduce vm_map_user_ram, vm_unmap_user_ram (v2)
mm: Provide is_vma_noncached
cpu_opv: Provide cpu_opv system call (v9)
cpu_opv: limit amount of virtual address space used by cpu_opv
x86: Wire up cpu_opv system call
powerpc: Wire up cpu_opv system call
arm: Wire up cpu_opv system call
cpu-opv/selftests: Provide cpu-op library
cpu-opv/selftests: Provide basic test
cpu-opv/selftests: Provide percpu_op API
cpu-opv/selftests: Provide basic percpu ops test
cpu-opv/selftests: Provide parametrized tests
cpu-opv/selftests: Provide Makefile, scripts, gitignore
MAINTAINERS | 8 +
arch/arm/tools/syscall.tbl | 1 +
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/uapi/asm/unistd.h | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/mm.h | 24 +
include/linux/syscalls.h | 3 +
include/linux/vmalloc.h | 4 +
include/uapi/linux/cpu_opv.h | 69 ++
init/Kconfig | 17 +
kernel/Makefile | 1 +
kernel/cpu_opv.c | 1027 +++++++++++++++++
kernel/sys_ni.c | 1 +
kernel/sysctl.c | 15 +
mm/vmalloc.c | 78 +-
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/cpu-opv/.gitignore | 6 +
tools/testing/selftests/cpu-opv/Makefile | 39 +
.../testing/selftests/cpu-opv/basic_cpu_opv_test.c | 1207 ++++++++++++++++++++
.../selftests/cpu-opv/basic_percpu_ops_test.c | 295 +++++
tools/testing/selftests/cpu-opv/cpu-op.c | 362 ++++++
tools/testing/selftests/cpu-opv/cpu-op.h | 43 +
tools/testing/selftests/cpu-opv/param_test.c | 1187 +++++++++++++++++++
tools/testing/selftests/cpu-opv/percpu-op.h | 151 +++
tools/testing/selftests/cpu-opv/run_param_test.sh | 134 +++
tools/testing/selftests/rseq/rseq.c | 23 +-
tools/testing/selftests/rseq/rseq.h | 1 +
tools/testing/selftests/rseq/run_param_test.sh | 7 +-
29 files changed, 4694 insertions(+), 14 deletions(-)
create mode 100644 include/uapi/linux/cpu_opv.h
create mode 100644 kernel/cpu_opv.c
create mode 100644 tools/testing/selftests/cpu-opv/.gitignore
create mode 100644 tools/testing/selftests/cpu-opv/Makefile
create mode 100644 tools/testing/selftests/cpu-opv/basic_cpu_opv_test.c
create mode 100644 tools/testing/selftests/cpu-opv/basic_percpu_ops_test.c
create mode 100644 tools/testing/selftests/cpu-opv/cpu-op.c
create mode 100644 tools/testing/selftests/cpu-opv/cpu-op.h
create mode 100644 tools/testing/selftests/cpu-opv/param_test.c
create mode 100644 tools/testing/selftests/cpu-opv/percpu-op.h
create mode 100755 tools/testing/selftests/cpu-opv/run_param_test.sh
--
2.11.0
^ permalink raw reply
* Re: [RFC PATCH] Implement /proc/pid/kill
From: Andy Lutomirski @ 2018-11-01 4:53 UTC (permalink / raw)
To: Jann Horn
Cc: dancol, LKML, timmurray, Joel Fernandes, surenb,
Andrew Lutomirski, Linux API, Eric W. Biederman
In-Reply-To: <CAG48ez01OWYVqEo+Qf65scG7a-48tDM4Gh3BOUKY+-b9g+9itw@mail.gmail.com>
On Wed, Oct 31, 2018 at 9:23 AM Jann Horn <jannh@google.com> wrote:
>
> +linux-api, Andy Lutomirski, Eric Biederman
>
> On Wed, Oct 31, 2018 at 3:12 AM Daniel Colascione <dancol@google.com> wrote:
> > Add a simple proc-based kill interface. To use /proc/pid/kill, just
> > write the signal number in base-10 ASCII to the kill file of the
> > process to be killed: for example, 'echo 9 > /proc/$$/kill'.
>
> This is a kernel API change, you should CC the linux-api list.
>
> I think that getting the semantics of this right might be easier if
> you used an ioctl handler instead of a write handler.
>
> > Semantically, /proc/pid/kill works like kill(2), except that the
> > process ID comes from the proc filesystem context instead of from an
> > explicit system call parameter. This way, it's possible to avoid races
> > between inspecting some aspect of a process and that process's PID
> > being reused for some other process.
> >
> > With /proc/pid/kill, it's possible to write a proper race-free and
> > safe pkill(1). An approximation follows. A real program might use
> > openat(2), having opened a process's /proc/pid directory explicitly,
> > with the directory file descriptor serving as a sort of "process
> > handle".
> >
> > #!/bin/bash
> > set -euo pipefail
> > pat=$1
> > for proc_status in /proc/*/status; do (
> > cd $(dirname $proc_status)
> > readarray proc_argv -d'' < cmdline
> > if ((${#proc_argv[@]} > 0)) &&
> > [[ ${proc_argv[0]} = *$pat* ]];
> > then
> > echo 15 > kill
> > fi
> > ) || true; done
> >
> > Signed-off-by: Daniel Colascione <dancol@google.com>
> > ---
> > fs/proc/base.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 7e9f07bf260d..923d62b21e67 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -205,6 +205,44 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
> > return result;
> > }
> >
> > +static ssize_t proc_pid_kill_write(struct file *file,
> > + const char __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + ssize_t res;
> > + int sig;
> > + char buffer[4];
> > +
> > + res = -EINVAL;
> > + if (*ppos != 0)
> > + goto out;
> > +
> > + res = -EINVAL;
> > + if (count > sizeof(buffer) - 1)
> > + goto out;
> > +
> > + res = -EFAULT;
> > + if (copy_from_user(buffer, buf, count))
> > + goto out;
> > +
> > + buffer[count] = '\0';
> > + res = kstrtoint(strstrip(buffer), 10, &sig);
> > + if (res)
> > + goto out;
> > +
> > + res = kill_pid(proc_pid(file_inode(file)), sig, 0);
Indeed, you can't do this from .write unless you manage to pass a cred
struct pointer in. ioctl or a new syscall would be better.
--Andy
^ permalink raw reply
* Re: [PATCH v6 0/1] ns: introduce binfmt_misc namespace
From: Jann Horn @ 2018-11-01 3:51 UTC (permalink / raw)
To: James Bottomley
Cc: Laurent Vivier, kernel list, Linux API, containers, dima, Al Viro,
linux-fsdevel, Eric W. Biederman
In-Reply-To: <1541041159.4632.6.camel@HansenPartnership.com>
On Thu, Nov 1, 2018 at 3:59 AM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Tue, 2018-10-16 at 11:52 +0200, Laurent Vivier wrote:
> > Hi,
> >
> > Any comment on this last version?
> >
> > Any chance to be merged?
>
> I've got a use case for this: I went to one of the Graphene talks in
> Edinburgh and it struck me that we seem to keep reinventing the type of
> sandboxing that qemu-user already does. However if you want to do an
> x86 on x86 sandbox, you can't currently use the binfmt_misc mechanism
> because that has you running *every* binary on the system emulated.
> Doing it per user namespace fixes this problem and allows us to at
> least cut down on all the pointless duplication.
Waaaaaait. What? qemu-user does not do "sandboxing". qemu-user makes
your code slower and *LESS* secure. As far as I know, qemu-user is
only intended for purposes like development and testing.
^ permalink raw reply
* Re: [PATCH v6 0/1] ns: introduce binfmt_misc namespace
From: James Bottomley @ 2018-11-01 2:59 UTC (permalink / raw)
To: Laurent Vivier, linux-kernel
Cc: Jann Horn, linux-api, containers, Dmitry Safonov, Alexander Viro,
linux-fsdevel, Eric Biederman
In-Reply-To: <7ed6f823-547b-922d-59ff-aba9c4c3ab39@vivier.eu>
On Tue, 2018-10-16 at 11:52 +0200, Laurent Vivier wrote:
> Hi,
>
> Any comment on this last version?
>
> Any chance to be merged?
I've got a use case for this: I went to one of the Graphene talks in
Edinburgh and it struck me that we seem to keep reinventing the type of
sandboxing that qemu-user already does. However if you want to do an
x86 on x86 sandbox, you can't currently use the binfmt_misc mechanism
because that has you running *every* binary on the system emulated.
Doing it per user namespace fixes this problem and allows us to at
least cut down on all the pointless duplication.
James
^ permalink raw reply
* Re: [RFC PATCH] Minimal non-child process exit notification support
From: Daniel Colascione @ 2018-10-31 17:44 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: linux-kernel, linux-api, Tim Murray, Joel Fernandes
In-Reply-To: <175DDE5D-E738-4C35-B664-6AD8B9CF446D@amacapital.net>
On Wed, Oct 31, 2018 at 5:25 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> I had an old patch to do much the same thing:
It's a perennial idea. :-)
> https://lore.kernel.org/patchwork/patch/345098/
>
> Can you comment as to how your API compares to my old patch?
Sure. Basically, my approach is sort-of eventfd-esque, whereas your
approach involves adding a very unusual operation (poll support) to a
type of file (a directory) that normally doesn't support it. My
approach feels a bit more "conventional" than poll on a dfd.
Additionally, my approach is usable from the shell. In your model,
poll(2) returning *is* the notification, whereas in my approach, the
canonical notification is read() yielding EOF, with poll(2) acting
like a wakeup hint, just like for eventfd. (You can set O_NONBLOCK on
the exithand FD just like you would any other FD.)
The use of read() for notification of exit also allows for a simple
extension in which we return a siginfo_t with exit information to the
waiter, without changing the API model. My initial patch doesn't
include this feature because I wanted to keep the initial version as
simple as possible.
> You’re using
> some fairly gnarly global synchronization
The global synchronization only kicks for a particular process exit if
somebody has used an exithand FD to wait on that process. (Or more
precisely, that process's struct signal.) Since most process exits
don't require global synchronization, I don't think the global
waitqueue for exithand is a big problem, but if it is, there are
options for fixing it.
> , and that seems unnecessary
It is necessary, and I don't see how your patch is correct. In your
proc_task_base_poll, you call poll_wait() with &task->detach_wqh. What
prevents that waitqueue disappearing (and the poll table waitqueue
pointer dangling) immediately after proc_task_base_poll returns? The
proc_inode maintains a reference to a struct pid, not a task_struct,
but your waitqueue lives in task_struct.
The waitqueue living in task_struct is also wrong in the case that a
multithreaded program execs from a non-main thread; in this case (if
I'm reading the code in exec.c right) we destroy the old main thread
task_struct and have the caller-of-exec's task_struct adopt the old
main thread's struct pid. That is, identity-continuity of struct task
is not the same as identity-continuity of the logical thread group.
^ permalink raw reply
* Re: [RFC PATCH] Minimal non-child process exit notification support
From: Daniel Colascione @ 2018-10-31 16:53 UTC (permalink / raw)
To: linux-kernel, linux-api; +Cc: Tim Murray, Joel Fernandes, Daniel Colascione
In-Reply-To: <20181029175322.189042-1-dancol@google.com>
+ linux-api
On Mon, Oct 29, 2018 at 5:53 PM, Daniel Colascione <dancol@google.com> wrote:
> This patch adds a new file under /proc/pid, /proc/pid/exithand.
> Attempting to read from an exithand file will block until the
> corresponding process exits, at which point the read will successfully
> complete with EOF. The file descriptor supports both blocking
> operations and poll(2). It's intended to be a minimal interface for
> allowing a program to wait for the exit of a process that is not one
> of its children.
>
> Why might we want this interface? Android's lmkd kills processes in
> order to free memory in response to various memory pressure
> signals. It's desirable to wait until a killed process actually exits
> before moving on (if needed) to killing the next process. Since the
> processes that lmkd kills are not lmkd's children, lmkd currently
> lacks a way to wait for a proces to actually die after being sent
> SIGKILL; today, lmkd resorts to polling the proc filesystem pid
> entry. This interface allow lmkd to give up polling and instead block
> and wait for process death.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> fs/proc/Makefile | 1 +
> fs/proc/base.c | 1 +
> fs/proc/exithand.c | 117 +++++++++++++++++++++++++++++++++++
> fs/proc/internal.h | 4 ++
> include/linux/sched/signal.h | 7 +++
> kernel/exit.c | 2 +
> kernel/signal.c | 3 +
> 7 files changed, 135 insertions(+)
> create mode 100644 fs/proc/exithand.c
>
> diff --git a/fs/proc/Makefile b/fs/proc/Makefile
> index ead487e80510..21322280a2c1 100644
> --- a/fs/proc/Makefile
> +++ b/fs/proc/Makefile
> @@ -27,6 +27,7 @@ proc-y += softirqs.o
> proc-y += namespaces.o
> proc-y += self.o
> proc-y += thread_self.o
> +proc-y += exithand.o
> proc-$(CONFIG_PROC_SYSCTL) += proc_sysctl.o
> proc-$(CONFIG_NET) += proc_net.o
> proc-$(CONFIG_PROC_KCORE) += kcore.o
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 7e9f07bf260d..31bc6bbb6dc4 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -3006,6 +3006,7 @@ static const struct pid_entry tgid_base_stuff[] = {
> #ifdef CONFIG_LIVEPATCH
> ONE("patch_state", S_IRUSR, proc_pid_patch_state),
> #endif
> + REG("exithand", S_IRUGO, proc_tgid_exithand_operations),
> };
>
> static int proc_tgid_base_readdir(struct file *file, struct dir_context *ctx)
> diff --git a/fs/proc/exithand.c b/fs/proc/exithand.c
> new file mode 100644
> index 000000000000..358b08da6a08
> --- /dev/null
> +++ b/fs/proc/exithand.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Synchronous exit notification of non-child processes
> + *
> + * Simple file descriptor /proc/pid/exithand. Read blocks (and poll
> + * reports non-readable) until process either dies or becomes
> + * a zombie.
> + */
> +#include <linux/printk.h>
> +#include <linux/sched/signal.h>
> +#include <linux/poll.h>
> +#include "internal.h"
> +
> +static int proc_tgid_exithand_open(struct inode *inode, struct file *file)
> +{
> + struct task_struct* task = get_proc_task(inode);
> + /* If get_proc_task failed, it means the task is dead, which
> + * is fine, since a subsequent read will return
> + * immediately. */
> + if (task && !thread_group_leader(task))
> + return -EINVAL;
> + return 0;
> +}
> +
> +static ssize_t proc_tgid_exithand_read(struct file * file,
> + char __user * buf,
> + size_t count, loff_t *ppos)
> +{
> + struct task_struct* task = NULL;
> + wait_queue_entry_t wait;
> + ssize_t res = 0;
> + bool locked = false;
> +
> + for (;;) {
> + /* Retrieve the task from the struct pid each time
> + * through the loop in case the exact struct task
> + * changes underneath us (e.g., if in exec.c, the
> + * execing process kills the group leader and starts
> + * using its PID). The struct signal should be the
> + * same though even in this case.
> + */
> + task = get_proc_task(file_inode(file));
> + res = 0;
> + if (!task)
> + goto out; /* No task? Must have died. */
> +
> + BUG_ON(!thread_group_leader(task));
> +
> + /* Synchronizes with exit.c machinery. */
> + read_lock(&tasklist_lock);
> + locked = true;
> +
> + res = 0;
> + if (task->exit_state)
> + goto out;
> +
> + res = -EAGAIN;
> + if (file->f_flags & O_NONBLOCK)
> + goto out;
> +
> + /* Tell exit.c to go to the trouble of waking our
> + * runqueue when this process gets around to
> + * exiting. */
> + task->signal->exithand_is_interested = true;
> +
> + /* Even if the task identity changes, task->signal
> + * should be invariant across the wait, making it safe
> + * to go remove our wait record from the wait queue
> + * after we come back from schedule. */
> +
> + init_waitqueue_entry(&wait, current);
> + add_wait_queue(&wait_exithand, &wait);
> +
> + read_unlock(&tasklist_lock);
> + locked = false;
> +
> + put_task_struct(task);
> + task = NULL;
> +
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
> + set_current_state(TASK_RUNNING);
> + remove_wait_queue(&wait_exithand, &wait);
> +
> + res = -ERESTARTSYS;
> + if (signal_pending(current))
> + goto out;
> + }
> +out:
> + if (locked)
> + read_unlock(&tasklist_lock);
> + if (task)
> + put_task_struct(task);
> + return res;
> +}
> +
> +static __poll_t proc_tgid_exithand_poll(struct file *file, poll_table *wait)
> +{
> + __poll_t mask = 0;
> + struct task_struct* task = get_proc_task(file_inode(file));
> + if (!task) {
> + mask |= POLLIN;
> + } else if (READ_ONCE(task->exit_state)) {
> + mask |= POLLIN;
> + } else {
> + read_lock(&tasklist_lock);
> + task->signal->exithand_is_interested = true;
> + read_unlock(&tasklist_lock);
> + poll_wait(file, &wait_exithand, wait);
> + }
> + return mask;
> +}
> +
> +const struct file_operations proc_tgid_exithand_operations = {
> + .open = proc_tgid_exithand_open,
> + .read = proc_tgid_exithand_read,
> + .poll = proc_tgid_exithand_poll,
> +};
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 5185d7f6a51e..1009d20475bc 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -304,3 +304,7 @@ extern unsigned long task_statm(struct mm_struct *,
> unsigned long *, unsigned long *,
> unsigned long *, unsigned long *);
> extern void task_mem(struct seq_file *, struct mm_struct *);
> +
> +/* exithand.c */
> +
> +extern const struct file_operations proc_tgid_exithand_operations;
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index 13789d10a50e..44131cb6c7f4 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -74,6 +74,10 @@ struct multiprocess_signals {
> struct hlist_node node;
> };
>
> +/* Need to stick the waitq for exithand outside process structures in
> + * case a process disappears across a poll. */
> +extern wait_queue_head_t wait_exithand;
> +
> /*
> * NOTE! "signal_struct" does not have its own
> * locking, because a shared signal_struct always
> @@ -87,6 +91,9 @@ struct signal_struct {
> int nr_threads;
> struct list_head thread_head;
>
> + /* Protected with tasklist_lock. */
> + bool exithand_is_interested;
> +
> wait_queue_head_t wait_chldexit; /* for wait4() */
>
> /* current thread group signal load-balancing target: */
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 0e21e6d21f35..44a4e3796f8b 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1485,6 +1485,8 @@ void __wake_up_parent(struct task_struct *p, struct task_struct *parent)
> {
> __wake_up_sync_key(&parent->signal->wait_chldexit,
> TASK_INTERRUPTIBLE, 1, p);
> + if (p->signal->exithand_is_interested)
> + __wake_up_sync(&wait_exithand, TASK_INTERRUPTIBLE, 0);
> }
>
> static long do_wait(struct wait_opts *wo)
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 17565240b1c6..e156d48da70a 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -454,6 +454,9 @@ void flush_sigqueue(struct sigpending *queue)
> }
> }
>
> +wait_queue_head_t wait_exithand =
> + __WAIT_QUEUE_HEAD_INITIALIZER(wait_exithand);
> +
> /*
> * Flush all pending signals for this kthread.
> */
> --
> 2.19.1.568.g152ad8e336-goog
>
^ permalink raw reply
* Re: [RFC 00/20] ns: Introduce Time Namespace
From: Andrei Vagin @ 2018-10-31 16:26 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Eric W. Biederman, linux-kselftest@vger.kernel.org,
Dmitry Safonov, linux-api@vger.kernel.org, Jeff Dike,
x86@kernel.org, Dmitry Safonov, linux-kernel@vger.kernel.org,
Oleg Nesterov, criu@openvz.org, Ingo Molnar, Alexey Dobriyan,
Andy Lutomirski, H. Peter Anvin, Cyrill Gorcunov,
Christian Brauner, Pavel Emelianov, Shuah Khan, container
In-Reply-To: <alpine.DEB.2.21.1810292026040.5984@nanos.tec.linutronix.de>
On Mon, Oct 29, 2018 at 09:33:14PM +0100, Thomas Gleixner wrote:
> Andrei,
>
> On Sat, 20 Oct 2018, Andrei Vagin wrote:
> > When a container is migrated to another host, we have to restore its
> > monotonic and boottime clocks, but we still expect that the container
> > will continue using the host real-time clock.
> >
> > Before stating this series, I was thinking about this, I decided that
> > these cases can be solved independently. Probably, the full isolation of
> > the time sub-system will have much higher overhead than just offsets for
> > a few clocks. And the idea that isolation of the real-time clock should
> > be optional gives us another hint that offsets for monotonic and
> > boot-time clocks can be implemented independently.
> >
> > Eric and Tomas, what do you think about this? If you agree that these
> > two cases can be implemented separately, what should we do with this
> > series to make it ready to be merged?
> >
> > I know that we need to:
> >
> > * look at device drivers that report timestamps in CLOCK_MONOTONIC base.
>
> and CLOCK_BOOTTIME and that's quite a few.
>
> > * forbid changing offsets after creating timers
>
> There are more things to think about. What about interfaces which expose
> boot time or monotonic time in /proc?
We didn't find any proc files where boot or monotonic time is reported,
but we will double check this.
>
> Aside of that (I finally came around to look at the series in more detail)
> I'm really unhappy about the unconditional overhead once the Time namespace
> config switch is enabled. This applies especially to the VDSO. We spent
> quite some time recently to squeeze a few cycles out of those functions and
> it would be a pity to pointlessly waste cycles for the !namespace case.
It is a good point. We will work on it.
>
> I can see the urge for this, but please let us think it through properly
> before rushing anything in which we are going to regret once we want to do
> more sophisticated time domain management, e.g. support for isolated clock
> real time. I'm worried, that without a clear plan about the overall
> picture, we end up with duct tape which is hard to distangle after the
> fact.
Thomas, there is no rush at all. This functionality is critical for
CRUI, but we have enough time to solve it properly.
The only thing what I want is that this functionality continues moving
forward and will not be put in the back burner.
>
> There have been a few other things brought up versus time management in
> general, like the TSN folks utilizing grand clock masters which expose
> random time instead of proper TAI. Plus some requirements for exposing some
> sort of 'monotonic' clocks which are derived from external synchronization
> mechanisms, but should not affect the regular time keeping clocks.
>
> While different issues, these all fall into the category of separate time
> domains, so taking a step back to the drawing board is probably the best
> thing what we can do now.
>
> There are certainly a few things which can be looked at independently,
> e.g. the VDSO mechanics or general mechanisms to avoid plastering the whole
> kernel with these name space functions applying offsets left and right. I
> rather have dedicated core functionality which replaces/amends existing
> timer functions to become time namespace aware.
>
> I'll try to find some time in the next weeks to look deeper into that, but
> I can't promise anything before returning from LPC. Btw, LPC would be a
> great opportunity to discuss that. Are you and the other name space wizards
> there by any chance?
Dmitry and I are going to be there.
Thanks!
Andrei
>
> Thanks,
>
> tglx
>
>
^ permalink raw reply
* Re: [RFC PATCH] Implement /proc/pid/kill
From: Jann Horn @ 2018-10-31 16:22 UTC (permalink / raw)
To: dancol
Cc: kernel list, timmurray, Joel Fernandes, surenb, Andy Lutomirski,
Linux API, Eric W. Biederman
In-Reply-To: <20181029221037.87724-1-dancol@google.com>
+linux-api, Andy Lutomirski, Eric Biederman
On Wed, Oct 31, 2018 at 3:12 AM Daniel Colascione <dancol@google.com> wrote:
> Add a simple proc-based kill interface. To use /proc/pid/kill, just
> write the signal number in base-10 ASCII to the kill file of the
> process to be killed: for example, 'echo 9 > /proc/$$/kill'.
This is a kernel API change, you should CC the linux-api list.
I think that getting the semantics of this right might be easier if
you used an ioctl handler instead of a write handler.
> Semantically, /proc/pid/kill works like kill(2), except that the
> process ID comes from the proc filesystem context instead of from an
> explicit system call parameter. This way, it's possible to avoid races
> between inspecting some aspect of a process and that process's PID
> being reused for some other process.
>
> With /proc/pid/kill, it's possible to write a proper race-free and
> safe pkill(1). An approximation follows. A real program might use
> openat(2), having opened a process's /proc/pid directory explicitly,
> with the directory file descriptor serving as a sort of "process
> handle".
>
> #!/bin/bash
> set -euo pipefail
> pat=$1
> for proc_status in /proc/*/status; do (
> cd $(dirname $proc_status)
> readarray proc_argv -d'' < cmdline
> if ((${#proc_argv[@]} > 0)) &&
> [[ ${proc_argv[0]} = *$pat* ]];
> then
> echo 15 > kill
> fi
> ) || true; done
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> fs/proc/base.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 7e9f07bf260d..923d62b21e67 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -205,6 +205,44 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
> return result;
> }
>
> +static ssize_t proc_pid_kill_write(struct file *file,
> + const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + ssize_t res;
> + int sig;
> + char buffer[4];
> +
> + res = -EINVAL;
> + if (*ppos != 0)
> + goto out;
> +
> + res = -EINVAL;
> + if (count > sizeof(buffer) - 1)
> + goto out;
> +
> + res = -EFAULT;
> + if (copy_from_user(buffer, buf, count))
> + goto out;
> +
> + buffer[count] = '\0';
> + res = kstrtoint(strstrip(buffer), 10, &sig);
> + if (res)
> + goto out;
> +
> + res = kill_pid(proc_pid(file_inode(file)), sig, 0);
> + if (res)
> + goto out;
> + res = count;
> +out:
> + return res;
> +
> +}
^ permalink raw reply
* Re: [PATCH v8 1/2] seccomp: add a return code to trap to userspace
From: Oleg Nesterov @ 2018-10-31 13:04 UTC (permalink / raw)
To: Kees Cook
Cc: Tycho Andersen, Andy Lutomirski, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, LKML, Linux Containers, Linux API
In-Reply-To: <CAGXu5j+MYbAiSxdhLnQHdOZC-B=FWZoODxzwkYVEUqDc9SHnBQ@mail.gmail.com>
On 10/30, Kees Cook wrote:
>
> I'd like to avoid changing the return value of __secure_computing() to
> just avoid having to touch all the callers. And I'd prefer not to
> change __seccomp_filter() to a bool, since I'd like the return values
> to be consistent through the call chain.
Sure, please forget.
> I find the existing code more readable than a single-line return, just
> because it's very explicit. I don't want to have to think any harder
> when reading seccomp. ;)
Heh ;) Again, please forget, this is cosmetic.
But I simply can't resist. I asked this question exactly because I was
confused by these 2 lines:
if (__seccomp_filter(this_syscall, NULL, true))
return -1;
return 0;
to me it looks as if we need to filter out some non-zero return values and
turn them into -1. I had to spend some time (and think harder ;) to verify
that this is just the recursive call and nothing more.
nevermind, please ignore.
Oleg.
^ permalink raw reply
* RE: Problems with VM_MIXEDMAP removal from /proc/<pid>/smaps
From: y-goto-LMvhtfratI1BDgjK7y7TUQ @ 2018-10-31 5:59 UTC (permalink / raw)
To: 'Dave Chinner', Dan Williams
Cc: linux-nvdimm, Linux API, linux-xfs, Linux MM, linux-fsdevel,
Jan Kara, linux-ext4
In-Reply-To: <20181030224904.GT19305@dastard>
Hello,
> On Mon, Oct 29, 2018 at 11:30:41PM -0700, Dan Williams wrote:
> > On Thu, Oct 18, 2018 at 5:58 PM Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> > > On Thu, Oct 18, 2018 at 04:55:55PM +0200, Jan Kara wrote:
> > > > On Thu 18-10-18 11:25:10, Dave Chinner wrote:
> > > > > On Wed, Oct 17, 2018 at 04:23:50PM -0400, Jeff Moyer wrote:
> > > > > > MAP_SYNC
> > > > > > - file system guarantees that metadata required to reach faulted-in file
> > > > > > data is consistent on media before a write fault is completed. A
> > > > > > side-effect is that the page cache will not be used for
> > > > > > writably-mapped pages.
> > > > >
> > > > > I think you are conflating current implementation with API
> > > > > requirements - MAP_SYNC doesn't guarantee anything about page cache
> > > > > use. The man page definition simply says "supported only for files
> > > > > supporting DAX" and that it provides certain data integrity
> > > > > guarantees. It does not define the implementation.
> ....
> > > > With O_DIRECT the fallback to buffered IO is quite rare (at least for major
> > > > filesystems) so usually people just won't notice. If fallback for
> > > > MAP_DIRECT will be easy to hit, I'm not sure it would be very useful.
> > >
> > > Which is just like the situation where O_DIRECT on ext3 was not very
> > > useful, but on other filesystems like XFS it was fully functional.
> > >
> > > IMO, the fact that a specific filesytem has a suboptimal fallback
> > > path for an uncommon behaviour isn't an argument against MAP_DIRECT
> > > as a hint - it's actually a feature. If MAP_DIRECT can't be used
> > > until it's always direct access, then most filesystems wouldn't be
> > > able to provide any faster paths at all. It's much better to have
> > > partial functionality now than it is to never have the functionality
> > > at all, and so we need to design in the flexibility we need to
> > > iteratively improve implementations without needing API changes that
> > > will break applications.
> >
> > The hard guarantee requirement still remains though because an
> > application that expects combined MAP_SYNC|MAP_DIRECT semantics will
> > be surprised if the MAP_DIRECT property silently disappears.
>
> Why would they be surprised? They won't even notice it if the
> filesystem can provide MAP_SYNC without MAP_DIRECT.
>
> And that's the whole point.
>
> MAP_DIRECT is a private mapping state. So is MAP_SYNC. They are not
> visible to the filesystem and the filesystem does nothing to enforce
> them. If someone does something that requires the page cache (e.g.
> calls do_splice_direct()) then that MAP_DIRECT mapping has a whole
> heap of new work to do. And, in some cases, the filesystem may not
> be able to provide MAP_DIRECT as a result..
>
> IOWs, the filesystem cannot guarantee MAP_DIRECT and the
> circumstances under which MAP_DIRECT will and will not work are
> dynamic. If MAP_DIRECT is supposed to be a guarantee then we'll have
> applications randomly segfaulting in production as things like
> backups, indexers, etc run over the filesystem and do their work.
>
> This is why MAP_DIRECT needs to be an optimisation, not a
> requirement - things will still work if MAP_DIRECT is not used. What
> matters to these applications is MAP_SYNC - if we break MAP_SYNC,
> then the application data integrity model is violated. That's not an
> acceptible outcome.
>
> The problem, it seems to me, is that people are unable to separate
> MAP_DIRECT and MAP_SYNC. I suspect that is because, at present,
> MAP_SYNC on XFS and ext4 requires MAP_DIRECT. i.e. we can only
> provide MAP_SYNC functionality on DAX mappings. However, that's a
> /filesystem implementation issue/, not an API guarantee we need to
> provide to userspace.
>
> If we implement a persistent page cache (e.g. allocate page cache
> pages out of ZONE_DEVICE pmem), then filesystems like XFS and ext4
> could provide applications with the MAP_SYNC data integrity model
> without MAP_DIRECT. Indeed, those filesystems would not even be able
> to provide MAP_DIRECT semantics because they aren't backed by pmem.
>
> Hence if applications that want MAP_SYNC are hard coded
> MAP_SYNC|MAP_DIRECT and we make MAP_DIRECT a hard guarantee, then
> those applications are going to fail on a filesystem that provides
> only MAP_SYNC. This is despite the fact the applications would
> function correctly and the data integrity model would be maintained.
> i.e. the failure is because applications have assumed MAP_SYNC can
> only be provided by a DAX implementation, not because MAP_SYNC is
> not supported.
>
> MAP_SYNC really isn't about DAX at all. It's about enabling a data
> integrity model that requires the filesystem to provide userspace
> access to CPU addressable persistent memory. DAX+MAP_DIRECT is just
> one method of providing this functionality, but it's not the only
> method. Our API needs to be future proof rather than an encoding of
> the existing implementation limitations, otherwise apps will have to
> be re-written as every new MAP_SYNC capable technology comes along.
>
> In summary:
>
> MAP_DIRECT is an access hint.
>
> MAP_SYNC provides a data integrity model guarantee.
>
> MAP_SYNC may imply MAP_DIRECT for specific implementations,
> but it does not require or guarantee MAP_DIRECT.
>
> Let's compare that with O_DIRECT:
>
> O_DIRECT in an access hint.
>
> O_DSYNC provides a data integrity model guarantee.
>
> O_DSYNC may imply O_DIRECT for specific implementations, but
> it does not require or guarantee O_DIRECT.
>
> Consistency in access and data integrity models is a good thing. DAX
> and pmem is not an exception. We need to use a model we know works
> and has proven itself over a long period of time.
Hmmm, then, I would like to know all of the reasons of breakage of MAP_DIRECT.
(I'm not opposed to your opinion, but I need to know it.)
In O_DIRECT case, in my understanding, the reason of breakage of O_DIRECT is
"wrong alignment is specified by application", right?
When filesystem can not use O_DIRECT and it uses page cache instead,
then system uses more memory resource than user's expectation.
So, there is a side effect, and it may cause other trouble.
(memory pressure, expected performance can not be gained, and so on ..)
In such case its administrator (or technical support engineer) needs to struggle to
investigate what is the reason.
If the reason of the breakage is clear, then it is helpful to find the root cause,
and they can require the developer of wrong application to fix the problem.
"Please fix the alignment!".
So, I would like to know in MAP_DIRECT case, what is the reasons?
I think it will be helpful for users.
Only splice?
(Maybe such document will be necessary....)
Thanks,
>
> > I think
> > it still makes some sense as a hint for apps that want to minimize
> > page cache, but for the applications with a flush from userspace model
> > I think that wants to be an F_SETLEASE / F_DIRECTLCK operation. This
> > still gives the filesystem the option to inject page-cache at will,
> > but with an application coordination point.
>
> Why make it more complex for applications than it needs to be?
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [PATCH v8 1/2] seccomp: add a return code to trap to userspace
From: Kees Cook @ 2018-10-31 1:29 UTC (permalink / raw)
To: Tycho Andersen
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, LKML, Linux Containers, Linux API
In-Reply-To: <20181031002917.GA2180@cisco>
On Tue, Oct 30, 2018 at 5:29 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> On Tue, Oct 30, 2018 at 03:34:54PM -0700, Kees Cook wrote:
>> On Tue, Oct 30, 2018 at 3:32 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>> > On Tue, Oct 30, 2018 at 03:00:17PM -0700, Kees Cook wrote:
>> >> On Tue, Oct 30, 2018 at 2:54 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>> >> > On Tue, Oct 30, 2018 at 02:49:21PM -0700, Kees Cook wrote:
>> >> >> On Mon, Oct 29, 2018 at 3:40 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>> >> >> > * switch to a flags based future-proofing mechanism for struct
>> >> >> > seccomp_notif and seccomp_notif_resp, thus avoiding version issues
>> >> >> > with structure length (Kees)
>> >> >> [...]
>> >> >> >
>> >> >> > +struct seccomp_notif {
>> >> >> > + __u64 id;
>> >> >> > + __u32 pid;
>> >> >> > + __u32 flags;
>> >> >> > + struct seccomp_data data;
>> >> >> > +};
>> >> >> > +
>> >> >> > +struct seccomp_notif_resp {
>> >> >> > + __u64 id;
>> >> >> > + __s64 val;
>> >> >> > + __s32 error;
>> >> >> > + __u32 flags;
>> >> >> > +};
>> >> >>
>> >> >> Hrm, so, what's the plan for when struct seccomp_data changes size?
>> >> >
>> >> > I guess my plan was don't ever change the size again, just use flags
>> >> > and have extra state available via ioctl().
>> >> >
>> >> >> I'm realizing that it might be "too late" for userspace to discover
>> >> >> it's running on a newer kernel. i.e. it gets a user notification, and
>> >> >> discovers flags it doesn't know how to handle. Do we actually need
>> >> >> both flags AND a length? Designing UAPI is frustrating! :)
>> >> >
>> >> > :). I don't see this as such a big problem -- in fact it's better than
>> >> > the length mode, where you don't know what you don't know, because it
>> >> > only copied as much info as you could handle. Older userspace would
>> >> > simply not use information it didn't know how to use.
>> >> >
>> >> >> Do we need another ioctl to discover the seccomp_data size maybe?
>> >> >
>> >> > That could be an option as well, assuming we agree that size would
>> >> > work, which I thought we didn't?
>> >>
>> >> Size alone wasn't able to determine the layout of the seccomp_notif
>> >> structure since it had holes (in the prior version). seccomp_data
>> >> doesn't have holes and is likely to change in size (see the recent
>> >> thread on adding the MPK register to it...)
>> >
>> > Oh, sorry, I misread this as seccomp_notif, not seccomp_data.
>> >
>> >> I'm trying to imagine the right API for this. A portable user of
>> >> seccomp_notif expects the id/pid/flags/data to always be in the same
>> >> place, but it's the size of seccomp_data that may change. So it wants
>> >> to allocate space for seccomp_notif header and "everything else", of
>> >> which is may only understand the start of seccomp_data (and ignore any
>> >> new trailing fields).
>> >>
>> >> So... perhaps the "how big are things?" ioctl would report the header
>> >> size and the seccomp_data size. Then both are flexible. And flags
>> >> would be left as a way to "version" the header?
>> >>
>> >> Any Linux API list members want to chime in here?
>> >
>> > So:
>> >
>> > struct seccomp_notify_sizes {
>> > u16 seccomp_notify;
>> > u16 seccomp_data;
>> > };
>> >
>> > ioctl(fd, SECCOMP_IOCTL_GET_SIZE, &sizes);
>> >
>> > This would be only one extra syscall over the lifetime of the listener
>> > process, which doesn't seem too bad. One thing that's slightly
>> > annoying is that you can't do it until you actually get an event, so
>> > maybe it could be a command on the seccomp syscall instead:
>> >
>> > seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes);
>>
>> Yeah, top-level makes more sense. u16 seems fine too.
>
> So one problem is this is that the third argument of the seccomp
> syscall is declared as const char, so I get:
>
> kernel/seccomp.c: In function ‘seccomp_get_notif_sizes’:
> kernel/seccomp.c:1401:19: warning: passing argument 1 of ‘copy_to_user’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> if (copy_to_user(usizes, &sizes, sizeof(sizes)))
> ^~~~~~
> In file included from ./include/linux/compat.h:19:0,
> from kernel/seccomp.c:19:
> ./include/linux/uaccess.h:152:1: note: expected ‘void *’ but argument is of type ‘const char *’
> copy_to_user(void __user *to, const void *from, unsigned long n)
> ^~~~~~~~~~~~
>
> If I drop the const it doesn't complain, but I'm not sure what the protocol is
> for changing the types of syscall declarations. In principle it doesn't really
> mean anything, but...
I think this should be fine. It's documented as "void *"...
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v8 1/2] seccomp: add a return code to trap to userspace
From: Tycho Andersen @ 2018-10-31 0:29 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, LKML, Linux Containers, Linux API
In-Reply-To: <CAGXu5j+XEYqVye7yZaYF1_-xpCQt7pA=YgsTpQJnYupE5MSa4Q@mail.gmail.com>
On Tue, Oct 30, 2018 at 03:34:54PM -0700, Kees Cook wrote:
> On Tue, Oct 30, 2018 at 3:32 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> > On Tue, Oct 30, 2018 at 03:00:17PM -0700, Kees Cook wrote:
> >> On Tue, Oct 30, 2018 at 2:54 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> >> > On Tue, Oct 30, 2018 at 02:49:21PM -0700, Kees Cook wrote:
> >> >> On Mon, Oct 29, 2018 at 3:40 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> >> >> > * switch to a flags based future-proofing mechanism for struct
> >> >> > seccomp_notif and seccomp_notif_resp, thus avoiding version issues
> >> >> > with structure length (Kees)
> >> >> [...]
> >> >> >
> >> >> > +struct seccomp_notif {
> >> >> > + __u64 id;
> >> >> > + __u32 pid;
> >> >> > + __u32 flags;
> >> >> > + struct seccomp_data data;
> >> >> > +};
> >> >> > +
> >> >> > +struct seccomp_notif_resp {
> >> >> > + __u64 id;
> >> >> > + __s64 val;
> >> >> > + __s32 error;
> >> >> > + __u32 flags;
> >> >> > +};
> >> >>
> >> >> Hrm, so, what's the plan for when struct seccomp_data changes size?
> >> >
> >> > I guess my plan was don't ever change the size again, just use flags
> >> > and have extra state available via ioctl().
> >> >
> >> >> I'm realizing that it might be "too late" for userspace to discover
> >> >> it's running on a newer kernel. i.e. it gets a user notification, and
> >> >> discovers flags it doesn't know how to handle. Do we actually need
> >> >> both flags AND a length? Designing UAPI is frustrating! :)
> >> >
> >> > :). I don't see this as such a big problem -- in fact it's better than
> >> > the length mode, where you don't know what you don't know, because it
> >> > only copied as much info as you could handle. Older userspace would
> >> > simply not use information it didn't know how to use.
> >> >
> >> >> Do we need another ioctl to discover the seccomp_data size maybe?
> >> >
> >> > That could be an option as well, assuming we agree that size would
> >> > work, which I thought we didn't?
> >>
> >> Size alone wasn't able to determine the layout of the seccomp_notif
> >> structure since it had holes (in the prior version). seccomp_data
> >> doesn't have holes and is likely to change in size (see the recent
> >> thread on adding the MPK register to it...)
> >
> > Oh, sorry, I misread this as seccomp_notif, not seccomp_data.
> >
> >> I'm trying to imagine the right API for this. A portable user of
> >> seccomp_notif expects the id/pid/flags/data to always be in the same
> >> place, but it's the size of seccomp_data that may change. So it wants
> >> to allocate space for seccomp_notif header and "everything else", of
> >> which is may only understand the start of seccomp_data (and ignore any
> >> new trailing fields).
> >>
> >> So... perhaps the "how big are things?" ioctl would report the header
> >> size and the seccomp_data size. Then both are flexible. And flags
> >> would be left as a way to "version" the header?
> >>
> >> Any Linux API list members want to chime in here?
> >
> > So:
> >
> > struct seccomp_notify_sizes {
> > u16 seccomp_notify;
> > u16 seccomp_data;
> > };
> >
> > ioctl(fd, SECCOMP_IOCTL_GET_SIZE, &sizes);
> >
> > This would be only one extra syscall over the lifetime of the listener
> > process, which doesn't seem too bad. One thing that's slightly
> > annoying is that you can't do it until you actually get an event, so
> > maybe it could be a command on the seccomp syscall instead:
> >
> > seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes);
>
> Yeah, top-level makes more sense. u16 seems fine too.
So one problem is this is that the third argument of the seccomp
syscall is declared as const char, so I get:
kernel/seccomp.c: In function ‘seccomp_get_notif_sizes’:
kernel/seccomp.c:1401:19: warning: passing argument 1 of ‘copy_to_user’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
if (copy_to_user(usizes, &sizes, sizeof(sizes)))
^~~~~~
In file included from ./include/linux/compat.h:19:0,
from kernel/seccomp.c:19:
./include/linux/uaccess.h:152:1: note: expected ‘void *’ but argument is of type ‘const char *’
copy_to_user(void __user *to, const void *from, unsigned long n)
^~~~~~~~~~~~
If I drop the const it doesn't complain, but I'm not sure what the protocol is
for changing the types of syscall declarations. In principle it doesn't really
mean anything, but...
Tycho
^ permalink raw reply
* Re: Problems with VM_MIXEDMAP removal from /proc/<pid>/smaps
From: Dan Williams @ 2018-10-30 22:59 UTC (permalink / raw)
To: david
Cc: linux-nvdimm, Linux API, linux-xfs, Linux MM, linux-fsdevel,
Jan Kara, linux-ext4
In-Reply-To: <20181030224904.GT19305@dastard>
On Tue, Oct 30, 2018 at 3:49 PM Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
>
> On Mon, Oct 29, 2018 at 11:30:41PM -0700, Dan Williams wrote:
> > On Thu, Oct 18, 2018 at 5:58 PM Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> > > On Thu, Oct 18, 2018 at 04:55:55PM +0200, Jan Kara wrote:
> > > > On Thu 18-10-18 11:25:10, Dave Chinner wrote:
> > > > > On Wed, Oct 17, 2018 at 04:23:50PM -0400, Jeff Moyer wrote:
> > > > > > MAP_SYNC
> > > > > > - file system guarantees that metadata required to reach faulted-in file
> > > > > > data is consistent on media before a write fault is completed. A
> > > > > > side-effect is that the page cache will not be used for
> > > > > > writably-mapped pages.
> > > > >
> > > > > I think you are conflating current implementation with API
> > > > > requirements - MAP_SYNC doesn't guarantee anything about page cache
> > > > > use. The man page definition simply says "supported only for files
> > > > > supporting DAX" and that it provides certain data integrity
> > > > > guarantees. It does not define the implementation.
> ....
> > > > With O_DIRECT the fallback to buffered IO is quite rare (at least for major
> > > > filesystems) so usually people just won't notice. If fallback for
> > > > MAP_DIRECT will be easy to hit, I'm not sure it would be very useful.
> > >
> > > Which is just like the situation where O_DIRECT on ext3 was not very
> > > useful, but on other filesystems like XFS it was fully functional.
> > >
> > > IMO, the fact that a specific filesytem has a suboptimal fallback
> > > path for an uncommon behaviour isn't an argument against MAP_DIRECT
> > > as a hint - it's actually a feature. If MAP_DIRECT can't be used
> > > until it's always direct access, then most filesystems wouldn't be
> > > able to provide any faster paths at all. It's much better to have
> > > partial functionality now than it is to never have the functionality
> > > at all, and so we need to design in the flexibility we need to
> > > iteratively improve implementations without needing API changes that
> > > will break applications.
> >
> > The hard guarantee requirement still remains though because an
> > application that expects combined MAP_SYNC|MAP_DIRECT semantics will
> > be surprised if the MAP_DIRECT property silently disappears.
>
> Why would they be surprised? They won't even notice it if the
> filesystem can provide MAP_SYNC without MAP_DIRECT.
>
> And that's the whole point.
>
> MAP_DIRECT is a private mapping state. So is MAP_SYNC. They are not
> visible to the filesystem and the filesystem does nothing to enforce
> them. If someone does something that requires the page cache (e.g.
> calls do_splice_direct()) then that MAP_DIRECT mapping has a whole
> heap of new work to do. And, in some cases, the filesystem may not
> be able to provide MAP_DIRECT as a result..
>
> IOWs, the filesystem cannot guarantee MAP_DIRECT and the
> circumstances under which MAP_DIRECT will and will not work are
> dynamic. If MAP_DIRECT is supposed to be a guarantee then we'll have
> applications randomly segfaulting in production as things like
> backups, indexers, etc run over the filesystem and do their work.
>
> This is why MAP_DIRECT needs to be an optimisation, not a
> requirement - things will still work if MAP_DIRECT is not used. What
> matters to these applications is MAP_SYNC - if we break MAP_SYNC,
> then the application data integrity model is violated. That's not an
> acceptible outcome.
>
> The problem, it seems to me, is that people are unable to separate
> MAP_DIRECT and MAP_SYNC. I suspect that is because, at present,
> MAP_SYNC on XFS and ext4 requires MAP_DIRECT. i.e. we can only
> provide MAP_SYNC functionality on DAX mappings. However, that's a
> /filesystem implementation issue/, not an API guarantee we need to
> provide to userspace.
>
> If we implement a persistent page cache (e.g. allocate page cache
> pages out of ZONE_DEVICE pmem), then filesystems like XFS and ext4
> could provide applications with the MAP_SYNC data integrity model
> without MAP_DIRECT. Indeed, those filesystems would not even be able
> to provide MAP_DIRECT semantics because they aren't backed by pmem.
>
> Hence if applications that want MAP_SYNC are hard coded
> MAP_SYNC|MAP_DIRECT and we make MAP_DIRECT a hard guarantee, then
> those applications are going to fail on a filesystem that provides
> only MAP_SYNC. This is despite the fact the applications would
> function correctly and the data integrity model would be maintained.
> i.e. the failure is because applications have assumed MAP_SYNC can
> only be provided by a DAX implementation, not because MAP_SYNC is
> not supported.
>
> MAP_SYNC really isn't about DAX at all. It's about enabling a data
> integrity model that requires the filesystem to provide userspace
> access to CPU addressable persistent memory. DAX+MAP_DIRECT is just
> one method of providing this functionality, but it's not the only
> method. Our API needs to be future proof rather than an encoding of
> the existing implementation limitations, otherwise apps will have to
> be re-written as every new MAP_SYNC capable technology comes along.
>
> In summary:
>
> MAP_DIRECT is an access hint.
>
> MAP_SYNC provides a data integrity model guarantee.
>
> MAP_SYNC may imply MAP_DIRECT for specific implementations,
> but it does not require or guarantee MAP_DIRECT.
>
> Let's compare that with O_DIRECT:
>
> O_DIRECT in an access hint.
>
> O_DSYNC provides a data integrity model guarantee.
>
> O_DSYNC may imply O_DIRECT for specific implementations, but
> it does not require or guarantee O_DIRECT.
>
> Consistency in access and data integrity models is a good thing. DAX
> and pmem is not an exception. We need to use a model we know works
> and has proven itself over a long period of time.
>
> > I think
> > it still makes some sense as a hint for apps that want to minimize
> > page cache, but for the applications with a flush from userspace model
> > I think that wants to be an F_SETLEASE / F_DIRECTLCK operation. This
> > still gives the filesystem the option to inject page-cache at will,
> > but with an application coordination point.
>
> Why make it more complex for applications than it needs to be?
With the clarification that MAP_SYNC implies "cpu cache flush to
persistent memory page-cache *or* dax to persistent memory" I think
all of the concerns are addressed. I was conflating MAP_DIRECT as "no
page cache indirection", but the indirection does not matter if the
page cache itself is persisted.
^ permalink raw reply
* Re: Problems with VM_MIXEDMAP removal from /proc/<pid>/smaps
From: Dave Chinner @ 2018-10-30 22:49 UTC (permalink / raw)
To: Dan Williams
Cc: linux-nvdimm, Linux API, linux-xfs, Linux MM, linux-fsdevel,
Jan Kara, linux-ext4
In-Reply-To: <CAPcyv4ixoAh7HEMfm+B4sRDx1Qwm6SHGjtQ+5r3EKsxreRydrA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Oct 29, 2018 at 11:30:41PM -0700, Dan Williams wrote:
> On Thu, Oct 18, 2018 at 5:58 PM Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> > On Thu, Oct 18, 2018 at 04:55:55PM +0200, Jan Kara wrote:
> > > On Thu 18-10-18 11:25:10, Dave Chinner wrote:
> > > > On Wed, Oct 17, 2018 at 04:23:50PM -0400, Jeff Moyer wrote:
> > > > > MAP_SYNC
> > > > > - file system guarantees that metadata required to reach faulted-in file
> > > > > data is consistent on media before a write fault is completed. A
> > > > > side-effect is that the page cache will not be used for
> > > > > writably-mapped pages.
> > > >
> > > > I think you are conflating current implementation with API
> > > > requirements - MAP_SYNC doesn't guarantee anything about page cache
> > > > use. The man page definition simply says "supported only for files
> > > > supporting DAX" and that it provides certain data integrity
> > > > guarantees. It does not define the implementation.
....
> > > With O_DIRECT the fallback to buffered IO is quite rare (at least for major
> > > filesystems) so usually people just won't notice. If fallback for
> > > MAP_DIRECT will be easy to hit, I'm not sure it would be very useful.
> >
> > Which is just like the situation where O_DIRECT on ext3 was not very
> > useful, but on other filesystems like XFS it was fully functional.
> >
> > IMO, the fact that a specific filesytem has a suboptimal fallback
> > path for an uncommon behaviour isn't an argument against MAP_DIRECT
> > as a hint - it's actually a feature. If MAP_DIRECT can't be used
> > until it's always direct access, then most filesystems wouldn't be
> > able to provide any faster paths at all. It's much better to have
> > partial functionality now than it is to never have the functionality
> > at all, and so we need to design in the flexibility we need to
> > iteratively improve implementations without needing API changes that
> > will break applications.
>
> The hard guarantee requirement still remains though because an
> application that expects combined MAP_SYNC|MAP_DIRECT semantics will
> be surprised if the MAP_DIRECT property silently disappears.
Why would they be surprised? They won't even notice it if the
filesystem can provide MAP_SYNC without MAP_DIRECT.
And that's the whole point.
MAP_DIRECT is a private mapping state. So is MAP_SYNC. They are not
visible to the filesystem and the filesystem does nothing to enforce
them. If someone does something that requires the page cache (e.g.
calls do_splice_direct()) then that MAP_DIRECT mapping has a whole
heap of new work to do. And, in some cases, the filesystem may not
be able to provide MAP_DIRECT as a result..
IOWs, the filesystem cannot guarantee MAP_DIRECT and the
circumstances under which MAP_DIRECT will and will not work are
dynamic. If MAP_DIRECT is supposed to be a guarantee then we'll have
applications randomly segfaulting in production as things like
backups, indexers, etc run over the filesystem and do their work.
This is why MAP_DIRECT needs to be an optimisation, not a
requirement - things will still work if MAP_DIRECT is not used. What
matters to these applications is MAP_SYNC - if we break MAP_SYNC,
then the application data integrity model is violated. That's not an
acceptible outcome.
The problem, it seems to me, is that people are unable to separate
MAP_DIRECT and MAP_SYNC. I suspect that is because, at present,
MAP_SYNC on XFS and ext4 requires MAP_DIRECT. i.e. we can only
provide MAP_SYNC functionality on DAX mappings. However, that's a
/filesystem implementation issue/, not an API guarantee we need to
provide to userspace.
If we implement a persistent page cache (e.g. allocate page cache
pages out of ZONE_DEVICE pmem), then filesystems like XFS and ext4
could provide applications with the MAP_SYNC data integrity model
without MAP_DIRECT. Indeed, those filesystems would not even be able
to provide MAP_DIRECT semantics because they aren't backed by pmem.
Hence if applications that want MAP_SYNC are hard coded
MAP_SYNC|MAP_DIRECT and we make MAP_DIRECT a hard guarantee, then
those applications are going to fail on a filesystem that provides
only MAP_SYNC. This is despite the fact the applications would
function correctly and the data integrity model would be maintained.
i.e. the failure is because applications have assumed MAP_SYNC can
only be provided by a DAX implementation, not because MAP_SYNC is
not supported.
MAP_SYNC really isn't about DAX at all. It's about enabling a data
integrity model that requires the filesystem to provide userspace
access to CPU addressable persistent memory. DAX+MAP_DIRECT is just
one method of providing this functionality, but it's not the only
method. Our API needs to be future proof rather than an encoding of
the existing implementation limitations, otherwise apps will have to
be re-written as every new MAP_SYNC capable technology comes along.
In summary:
MAP_DIRECT is an access hint.
MAP_SYNC provides a data integrity model guarantee.
MAP_SYNC may imply MAP_DIRECT for specific implementations,
but it does not require or guarantee MAP_DIRECT.
Let's compare that with O_DIRECT:
O_DIRECT in an access hint.
O_DSYNC provides a data integrity model guarantee.
O_DSYNC may imply O_DIRECT for specific implementations, but
it does not require or guarantee O_DIRECT.
Consistency in access and data integrity models is a good thing. DAX
and pmem is not an exception. We need to use a model we know works
and has proven itself over a long period of time.
> I think
> it still makes some sense as a hint for apps that want to minimize
> page cache, but for the applications with a flush from userspace model
> I think that wants to be an F_SETLEASE / F_DIRECTLCK operation. This
> still gives the filesystem the option to inject page-cache at will,
> but with an application coordination point.
Why make it more complex for applications than it needs to be?
Cheers,
Dave.
--
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
^ permalink raw reply
* Re: [PATCH v8 1/2] seccomp: add a return code to trap to userspace
From: Kees Cook @ 2018-10-30 22:34 UTC (permalink / raw)
To: Tycho Andersen
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, LKML, Linux Containers, Linux API
In-Reply-To: <20181030223228.GG7343@cisco>
On Tue, Oct 30, 2018 at 3:32 PM, Tycho Andersen <tycho@tycho.ws> wrote:
> On Tue, Oct 30, 2018 at 03:00:17PM -0700, Kees Cook wrote:
>> On Tue, Oct 30, 2018 at 2:54 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>> > On Tue, Oct 30, 2018 at 02:49:21PM -0700, Kees Cook wrote:
>> >> On Mon, Oct 29, 2018 at 3:40 PM, Tycho Andersen <tycho@tycho.ws> wrote:
>> >> > * switch to a flags based future-proofing mechanism for struct
>> >> > seccomp_notif and seccomp_notif_resp, thus avoiding version issues
>> >> > with structure length (Kees)
>> >> [...]
>> >> >
>> >> > +struct seccomp_notif {
>> >> > + __u64 id;
>> >> > + __u32 pid;
>> >> > + __u32 flags;
>> >> > + struct seccomp_data data;
>> >> > +};
>> >> > +
>> >> > +struct seccomp_notif_resp {
>> >> > + __u64 id;
>> >> > + __s64 val;
>> >> > + __s32 error;
>> >> > + __u32 flags;
>> >> > +};
>> >>
>> >> Hrm, so, what's the plan for when struct seccomp_data changes size?
>> >
>> > I guess my plan was don't ever change the size again, just use flags
>> > and have extra state available via ioctl().
>> >
>> >> I'm realizing that it might be "too late" for userspace to discover
>> >> it's running on a newer kernel. i.e. it gets a user notification, and
>> >> discovers flags it doesn't know how to handle. Do we actually need
>> >> both flags AND a length? Designing UAPI is frustrating! :)
>> >
>> > :). I don't see this as such a big problem -- in fact it's better than
>> > the length mode, where you don't know what you don't know, because it
>> > only copied as much info as you could handle. Older userspace would
>> > simply not use information it didn't know how to use.
>> >
>> >> Do we need another ioctl to discover the seccomp_data size maybe?
>> >
>> > That could be an option as well, assuming we agree that size would
>> > work, which I thought we didn't?
>>
>> Size alone wasn't able to determine the layout of the seccomp_notif
>> structure since it had holes (in the prior version). seccomp_data
>> doesn't have holes and is likely to change in size (see the recent
>> thread on adding the MPK register to it...)
>
> Oh, sorry, I misread this as seccomp_notif, not seccomp_data.
>
>> I'm trying to imagine the right API for this. A portable user of
>> seccomp_notif expects the id/pid/flags/data to always be in the same
>> place, but it's the size of seccomp_data that may change. So it wants
>> to allocate space for seccomp_notif header and "everything else", of
>> which is may only understand the start of seccomp_data (and ignore any
>> new trailing fields).
>>
>> So... perhaps the "how big are things?" ioctl would report the header
>> size and the seccomp_data size. Then both are flexible. And flags
>> would be left as a way to "version" the header?
>>
>> Any Linux API list members want to chime in here?
>
> So:
>
> struct seccomp_notify_sizes {
> u16 seccomp_notify;
> u16 seccomp_data;
> };
>
> ioctl(fd, SECCOMP_IOCTL_GET_SIZE, &sizes);
>
> This would be only one extra syscall over the lifetime of the listener
> process, which doesn't seem too bad. One thing that's slightly
> annoying is that you can't do it until you actually get an event, so
> maybe it could be a command on the seccomp syscall instead:
>
> seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes);
Yeah, top-level makes more sense. u16 seems fine too.
--
Kees Cook
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox