* [PATCHv2 2/2] x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit
From: Josh Triplett @ 2015-05-11 19:30 UTC (permalink / raw)
To: Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, Linus Torvalds, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
For 32-bit userspace on a 64-bit kernel, this requires modifying
stub32_clone to actually swap the appropriate arguments to match
CONFIG_CLONE_BACKWARDS, rather than just leaving the C argument for tls
broken.
Patch co-authored by Josh Triplett and Thiago Macieira.
Signed-off-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
Acked-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/x86/Kconfig | 1 +
arch/x86/ia32/ia32entry.S | 2 +-
arch/x86/kernel/process_32.c | 6 +++---
arch/x86/kernel/process_64.c | 8 ++++----
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca..4960b0d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -124,6 +124,7 @@ config X86
select MODULES_USE_ELF_REL if X86_32
select MODULES_USE_ELF_RELA if X86_64
select CLONE_BACKWARDS if X86_32
+ select HAVE_COPY_THREAD_TLS
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_QUEUE_RWLOCK
select OLD_SIGSUSPEND3 if X86_32 || IA32_EMULATION
diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 156ebca..0286735 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -487,7 +487,7 @@ GLOBAL(\label)
ALIGN
GLOBAL(stub32_clone)
leaq sys_clone(%rip),%rax
- mov %r8, %rcx
+ xchg %r8, %rcx
jmp ia32_ptregs_common
ALIGN
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 603c4f9..ead28ff 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -129,8 +129,8 @@ void release_thread(struct task_struct *dead_task)
release_vm86_irqs(dead_task);
}
-int copy_thread(unsigned long clone_flags, unsigned long sp,
- unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+ unsigned long arg, struct task_struct *p, unsigned long tls)
{
struct pt_regs *childregs = task_pt_regs(p);
struct task_struct *tsk;
@@ -185,7 +185,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
*/
if (clone_flags & CLONE_SETTLS)
err = do_set_thread_area(p, -1,
- (struct user_desc __user *)childregs->si, 0);
+ (struct user_desc __user *)tls, 0);
if (err && p->thread.io_bitmap_ptr) {
kfree(p->thread.io_bitmap_ptr);
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 67fcc43..c69cabc 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -151,8 +151,8 @@ static inline u32 read_32bit_tls(struct task_struct *t, int tls)
return get_desc_base(&t->thread.tls_array[tls]);
}
-int copy_thread(unsigned long clone_flags, unsigned long sp,
- unsigned long arg, struct task_struct *p)
+int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
+ unsigned long arg, struct task_struct *p, unsigned long tls)
{
int err;
struct pt_regs *childregs;
@@ -209,10 +209,10 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,
#ifdef CONFIG_IA32_EMULATION
if (test_thread_flag(TIF_IA32))
err = do_set_thread_area(p, -1,
- (struct user_desc __user *)childregs->si, 0);
+ (struct user_desc __user *)tls, 0);
else
#endif
- err = do_arch_prctl(p, ARCH_SET_FS, childregs->r8);
+ err = do_arch_prctl(p, ARCH_SET_FS, tls);
if (err)
goto out;
}
--
2.1.4
^ permalink raw reply related
* [PATCHv2 1/2] clone: Support passing tls argument via C rather than pt_regs magic
From: Josh Triplett @ 2015-05-11 19:29 UTC (permalink / raw)
To: Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, Linus Torvalds, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
clone with CLONE_SETTLS accepts an argument to set the thread-local
storage area for the new thread. sys_clone declares an int argument
tls_val in the appropriate point in the argument list (based on the
various CLONE_BACKWARDS variants), but doesn't actually use or pass
along that argument. Instead, sys_clone calls do_fork, which calls
copy_process, which calls the arch-specific copy_thread, and copy_thread
pulls the corresponding syscall argument out of the pt_regs captured at
kernel entry (knowing what argument of clone that architecture passes
tls in).
Apart from being awful and inscrutable, that also only works because
only one code path into copy_thread can pass the CLONE_SETTLS flag, and
that code path comes from sys_clone with its architecture-specific
argument-passing order. This prevents introducing a new version of the
clone system call without propagating the same architecture-specific
position of the tls argument.
However, there's no reason to pull the argument out of pt_regs when
sys_clone could just pass it down via C function call arguments.
Introduce a new CONFIG_HAVE_COPY_THREAD_TLS for architectures to opt
into, and a new copy_thread_tls that accepts the tls parameter as an
additional unsigned long (syscall-argument-sized) argument.
Change sys_clone's tls argument to an unsigned long (which does
not change the ABI), and pass that down to copy_thread_tls.
Architectures that don't opt into copy_thread_tls will continue to
ignore the C argument to sys_clone in favor of the pt_regs captured at
kernel entry, and thus will be unable to introduce new versions of the
clone syscall.
Patch co-authored by Josh Triplett and Thiago Macieira.
Signed-off-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
Acked-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
arch/Kconfig | 7 ++++++
include/linux/sched.h | 14 ++++++++++++
include/linux/syscalls.h | 6 +++---
kernel/fork.c | 55 +++++++++++++++++++++++++++++++-----------------
4 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 05d7a8a..4834a58 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -484,6 +484,13 @@ config HAVE_IRQ_EXIT_ON_IRQ_STACK
This spares a stack switch and improves cache usage on softirq
processing.
+config HAVE_COPY_THREAD_TLS
+ bool
+ help
+ Architecture provides copy_thread_tls to accept tls argument via
+ normal C parameter passing, rather than extracting the syscall
+ argument from pt_regs.
+
#
# ABI hall of shame
#
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a419b65..2cc88c6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2480,8 +2480,22 @@ extern struct mm_struct *mm_access(struct task_struct *task, unsigned int mode);
/* Remove the current tasks stale references to the old mm_struct */
extern void mm_release(struct task_struct *, struct mm_struct *);
+#ifdef CONFIG_HAVE_COPY_THREAD_TLS
+extern int copy_thread_tls(unsigned long, unsigned long, unsigned long,
+ struct task_struct *, unsigned long);
+#else
extern int copy_thread(unsigned long, unsigned long, unsigned long,
struct task_struct *);
+
+/* Architectures that haven't opted into copy_thread_tls get the tls argument
+ * via pt_regs, so ignore the tls argument passed via C. */
+static inline int copy_thread_tls(
+ unsigned long clone_flags, unsigned long sp, unsigned long arg,
+ struct task_struct *p, unsigned long tls)
+{
+ return copy_thread(clone_flags, sp, arg, p);
+}
+#endif
extern void flush_thread(void);
extern void exit_thread(void);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 76d1e38..bb51bec 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -827,15 +827,15 @@ asmlinkage long sys_syncfs(int fd);
asmlinkage long sys_fork(void);
asmlinkage long sys_vfork(void);
#ifdef CONFIG_CLONE_BACKWARDS
-asmlinkage long sys_clone(unsigned long, unsigned long, int __user *, int,
+asmlinkage long sys_clone(unsigned long, unsigned long, int __user *, unsigned long,
int __user *);
#else
#ifdef CONFIG_CLONE_BACKWARDS3
asmlinkage long sys_clone(unsigned long, unsigned long, int, int __user *,
- int __user *, int);
+ int __user *, unsigned long);
#else
asmlinkage long sys_clone(unsigned long, unsigned long, int __user *,
- int __user *, int);
+ int __user *, unsigned long);
#endif
#endif
diff --git a/kernel/fork.c b/kernel/fork.c
index cf65139..b3dadf4 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1192,7 +1192,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
unsigned long stack_size,
int __user *child_tidptr,
struct pid *pid,
- int trace)
+ int trace,
+ unsigned long tls)
{
int retval;
struct task_struct *p;
@@ -1401,7 +1402,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
retval = copy_io(clone_flags, p);
if (retval)
goto bad_fork_cleanup_namespaces;
- retval = copy_thread(clone_flags, stack_start, stack_size, p);
+ retval = copy_thread_tls(clone_flags, stack_start, stack_size, p, tls);
if (retval)
goto bad_fork_cleanup_io;
@@ -1613,7 +1614,7 @@ static inline void init_idle_pids(struct pid_link *links)
struct task_struct *fork_idle(int cpu)
{
struct task_struct *task;
- task = copy_process(CLONE_VM, 0, 0, NULL, &init_struct_pid, 0);
+ task = copy_process(CLONE_VM, 0, 0, NULL, &init_struct_pid, 0, 0);
if (!IS_ERR(task)) {
init_idle_pids(task->pids);
init_idle(task, cpu);
@@ -1628,11 +1629,13 @@ struct task_struct *fork_idle(int cpu)
* It copies the process, and if successful kick-starts
* it and waits for it to finish using the VM if required.
*/
-long do_fork(unsigned long clone_flags,
- unsigned long stack_start,
- unsigned long stack_size,
- int __user *parent_tidptr,
- int __user *child_tidptr)
+static long _do_fork(
+ unsigned long clone_flags,
+ unsigned long stack_start,
+ unsigned long stack_size,
+ int __user *parent_tidptr,
+ int __user *child_tidptr,
+ unsigned long tls)
{
struct task_struct *p;
int trace = 0;
@@ -1657,7 +1660,7 @@ long do_fork(unsigned long clone_flags,
}
p = copy_process(clone_flags, stack_start, stack_size,
- child_tidptr, NULL, trace);
+ child_tidptr, NULL, trace, tls);
/*
* Do this prior waking up the new thread - the thread pointer
* might get invalid after that point, if the thread exits quickly.
@@ -1698,20 +1701,34 @@ long do_fork(unsigned long clone_flags,
return nr;
}
+#ifndef CONFIG_HAVE_COPY_THREAD_TLS
+/* For compatibility with architectures that call do_fork directly rather than
+ * using the syscall entry points below. */
+long do_fork(unsigned long clone_flags,
+ unsigned long stack_start,
+ unsigned long stack_size,
+ int __user *parent_tidptr,
+ int __user *child_tidptr)
+{
+ return _do_fork(clone_flags, stack_start, stack_size,
+ parent_tidptr, child_tidptr, 0);
+}
+#endif
+
/*
* Create a kernel thread.
*/
pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags)
{
- return do_fork(flags|CLONE_VM|CLONE_UNTRACED, (unsigned long)fn,
- (unsigned long)arg, NULL, NULL);
+ return _do_fork(flags|CLONE_VM|CLONE_UNTRACED, (unsigned long)fn,
+ (unsigned long)arg, NULL, NULL, 0);
}
#ifdef __ARCH_WANT_SYS_FORK
SYSCALL_DEFINE0(fork)
{
#ifdef CONFIG_MMU
- return do_fork(SIGCHLD, 0, 0, NULL, NULL);
+ return _do_fork(SIGCHLD, 0, 0, NULL, NULL, 0);
#else
/* can not support in nommu mode */
return -EINVAL;
@@ -1722,8 +1739,8 @@ SYSCALL_DEFINE0(fork)
#ifdef __ARCH_WANT_SYS_VFORK
SYSCALL_DEFINE0(vfork)
{
- return do_fork(CLONE_VFORK | CLONE_VM | SIGCHLD, 0,
- 0, NULL, NULL);
+ return _do_fork(CLONE_VFORK | CLONE_VM | SIGCHLD, 0,
+ 0, NULL, NULL, 0);
}
#endif
@@ -1731,27 +1748,27 @@ SYSCALL_DEFINE0(vfork)
#ifdef CONFIG_CLONE_BACKWARDS
SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
int __user *, parent_tidptr,
- int, tls_val,
+ unsigned long, tls,
int __user *, child_tidptr)
#elif defined(CONFIG_CLONE_BACKWARDS2)
SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned long, clone_flags,
int __user *, parent_tidptr,
int __user *, child_tidptr,
- int, tls_val)
+ unsigned long, tls)
#elif defined(CONFIG_CLONE_BACKWARDS3)
SYSCALL_DEFINE6(clone, unsigned long, clone_flags, unsigned long, newsp,
int, stack_size,
int __user *, parent_tidptr,
int __user *, child_tidptr,
- int, tls_val)
+ unsigned long, tls)
#else
SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
int __user *, parent_tidptr,
int __user *, child_tidptr,
- int, tls_val)
+ unsigned long, tls)
#endif
{
- return do_fork(clone_flags, newsp, 0, parent_tidptr, child_tidptr);
+ return _do_fork(clone_flags, newsp, 0, parent_tidptr, child_tidptr, tls);
}
#endif
--
2.1.4
^ permalink raw reply related
* [PATCHv2 0/2] clone: Support passing tls argument via C rather than pt_regs magic
From: Josh Triplett @ 2015-05-11 19:28 UTC (permalink / raw)
To: Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, Linus Torvalds, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
clone has some of the quirkiest syscall handling in the kernel, with a pile of
special cases, historical curiosities, and architecture-specific calling
conventions. In particular, clone with CLONE_SETTLS accepts a parameter "tls"
that the C entry point completely ignores and some assembly entry points
overwrite; instead, the low-level arch-specific code pulls the tls parameter
out of the arch-specific register captured as part of pt_regs on entry to the
kernel. That's a massive hack, and it makes the arch-specific code only work
when called via the specific existing syscall entry points; because of this
hack, any new clone-like system call would have to accept an identical tls
argument in exactly the same arch-specific position, rather than providing a
unified system call entry point across architectures.
The first patch allows architectures to handle the tls argument via normal C
parameter passing, if they opt in by selecting HAVE_COPY_THREAD_TLS. The
second patch makes 32-bit and 64-bit x86 opt into this.
These two patches came out of the clone4 series, which isn't ready for this
merge window, but these first two cleanup patches were entirely uncontroversial
and have acks. I'd like to go ahead and submit these two so that other
architectures can begin building on top of this and opting into
HAVE_COPY_THREAD_TLS. However, I'm also happy to wait and send these through
the next merge window (along with v3 of clone4) if anyone would prefer that.
v2: Move co-author from signoffs to a note in the commit message, as
required by Ingo Molnar.
Josh Triplett (2):
clone: Support passing tls argument via C rather than pt_regs magic
x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit
arch/Kconfig | 7 ++++++
arch/x86/Kconfig | 1 +
arch/x86/ia32/ia32entry.S | 2 +-
arch/x86/kernel/process_32.c | 6 ++---
arch/x86/kernel/process_64.c | 8 +++----
include/linux/sched.h | 14 +++++++++++
include/linux/syscalls.h | 6 ++---
kernel/fork.c | 55 +++++++++++++++++++++++++++++---------------
8 files changed, 69 insertions(+), 30 deletions(-)
--
2.1.4
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Chris Metcalf @ 2015-05-11 19:25 UTC (permalink / raw)
To: Mike Galbraith, Frederic Weisbecker
Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Gilad Ben Yossef,
Ingo Molnar, Peter Zijlstra, Rik van Riel, Tejun Heo,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <1431371974.3195.126.camel@gmail.com>
On 05/11/2015 03:19 PM, Mike Galbraith wrote:
> I really shouldn't have acked nohz_full -> isolcpus. Beside the fact
> that old static isolcpus was_supposed_ to crawl off and die, I know
> beyond doubt that having isolated a cpu as well as you can definitely
> does NOT imply that said cpu should become tickless.
True, at a high level, I agree that it would be better to have a
top-level concept like Frederic's proposed ISOLATION that includes
isolcpus and nohz_cpu (and other stuff as needed).
That said, what you wrote above is wrong; even with the patch you
acked, setting isolcpus does not automatically turn on nohz_full for
a given cpu. The patch made it true the other way around: when
you say nohz_full, you automatically get isolcpus on that cpu too.
That does, at least, make sense for the semantics of nohz_full.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Mike Galbraith @ 2015-05-11 19:19 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Steven Rostedt, Ingo Molnar, Andrew Morton, Chris Metcalf,
Gilad Ben Yossef, Ingo Molnar, Peter Zijlstra, Rik van Riel,
Tejun Heo, Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511153602.GA32512@lerouge>
On Mon, 2015-05-11 at 17:36 +0200, Frederic Weisbecker wrote:
> I expect some Real Time users may want this kind of dataplane mode where a syscall
> or whatever sleeps until the system is ready to provide the guarantee that no
> disturbance is going to happen for a given time. I'm not sure HPC users are interested
> in that.
I bet they are. RT is just a different way to spell HPC, and reverse.
> In fact it goes along the fact that NO_HZ_FULL was really only supposed to be about
> the tick and now people are introducing more and more kernel default presetting that
> assume NO_HZ_FULL implies ISOLATION which is about all kind of noise (tick, tasks, irqs,
> ...). Which is true but what kind of ISOLATION?
True, nohz mode and various isolation measures are distinct properties.
NO_HZ_FULL is kinda pointless without isolation measures to go with it,
but you're right.
I really shouldn't have acked nohz_full -> isolcpus. Beside the fact
that old static isolcpus was _supposed_ to crawl off and die, I know
beyond doubt that having isolated a cpu as well as you can definitely
does NOT imply that said cpu should become tickless. I routinely run a
load model that wants all the isolation it can get. It's not single
task compute though, rt executive coordinating rt workers, and of course
wants every cycle it can get, so nohz_full is less than helpful.
-Mike
^ permalink raw reply
* Re: [PATCH 5/6] nohz: support PR_DATAPLANE_STRICT mode
From: Chris Metcalf @ 2015-05-11 19:13 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Paul E. McKenney, Frederic Weisbecker, Ingo Molnar, Rik van Riel,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Gleixner, Tejun Heo, Peter Zijlstra, Steven Rostedt,
Christoph Lameter, Gilad Ben Yossef, Linux API
In-Reply-To: <CALCETrUoptUPVUxL87jUgry1pFac0rDPpnZ790zDKyK4a0FARA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 05/09/2015 03:28 AM, Andy Lutomirski wrote:
> On May 8, 2015 11:44 PM, "Chris Metcalf" <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org> wrote:
>> With QUIESCE mode, the task is in principle guaranteed not to be
>> interrupted by the kernel, but only if it behaves. In particular,
>> if it enters the kernel via system call, page fault, or any of
>> a number of other synchronous traps, it may be unexpectedly
>> exposed to long latencies. Add a simple flag that puts the process
>> into a state where any such kernel entry is fatal.
>>
>> To allow the state to be entered and exited, we add an internal
>> bit to current->dataplane_flags that is set when prctl() sets the
>> flags. That way, when we are exiting the kernel after calling
>> prctl() to forbid future kernel exits, we don't get immediately
>> killed.
> Is there any reason this can't already be addressed in userspace using
> /proc/interrupts or perf_events? ISTM the real goal here is to detect
> when we screw up and fail to avoid an interrupt, and killing the task
> seems like overkill to me.
Patch 6/6 proposes a mechanism to track down times when the
kernel screws up and delivers an IRQ to a userspace-only task.
Here, we're just trying to identify the times when an application
screws itself up out of cluelessness, and provide a mechanism
that allows the developer to easily figure out why and fix it.
In particular, /proc/interrupts won't show syscalls or page faults,
which are two easy ways applications can screw themselves
when they think they're in userspace-only mode. Also, they don't
provide sufficient precision to make it clear what part of the
application caused the undesired kernel entry.
In this case, killing the task is appropriate, since that's exactly
the semantics that have been asked for - it's like on architectures
that don't natively support unaligned accesses, but fake it relatively
slowly in the kernel, and in development you just say "give me a
SIGBUS when that happens" and in production you might say
"fix it up and let's try to keep going".
You can argue that this is something that can be done by ftrace,
but certainly you'd want to have a way to programmatically
turn on ftrace at the moment when you're entering userspace-only
mode, so we'd want some API around that anyway. And honestly,
it's so easy to test a task state bit in a couple of places and
generate the failurel on the spot, vs. the relative complexity
of setting up and understanding ftrace, that I think it merits
inclusion on that basis alone.
> Also, can we please stop further torturing the exit paths? We have a
> disaster of assembly code that calls into syscall_trace_leave and
> do_notify_resume. Those functions, in turn, *both* call user_enter
> (WTF?), and on very brief inspection user_enter makes it into the nohz
> code through multiple levels of indirection, which, with these
> patches, has yet another conditionally enabled helper, which does this
> new stuff. It's getting to be impossible to tell what happens when we
> exit to user space any more.
>
> Also, I think your code is buggy. There's no particular guarantee
> that user_enter is only called once between sys_prctl and the final
> exit to user mode (see the above WTF), so you might spuriously kill
> the process.
This is a good point; I also find the x86 kernel entry and exit
paths confusing, although I've reviewed them a bunch of times.
The tile architecture paths are a little easier to understand.
That said, I think the answer here is avoid non-idempotent
actions in the dataplane code, such as clearing a syscall bit.
A better implementation, I think, is to put the tests for "you
screwed up and synchronously entered the kernel" in
the syscall_trace_enter() code, which TIF_NOHZ already
gets us into; there, we can test if the dataplane "strict" bit is
set and the syscall is not prctl(), then we generate the error.
(We'd exclude exit and exit_group here too, since we don't
need to shoot down a task that's just trying to kill itself.)
This needs a bit of platform-specific code for each platform,
but that doesn't seem like too big a problem.
Likewise we can test in exception_enter() since that's only
called for all the synchronous user entries like page faults.
> Also, I think that most users will be quite surprised if "strict
> dataplane" code causes any machine check on the system to kill your
> dataplane task.
Fair point, and avoided by testing as described above instead.
(Though presumably in development it's not such a big deal,
and as I said you'd likely turn it off in production.)
> Similarly, a user accidentally running perf record -a
> probably should have some reasonable semantics.
Yes, also avoided by doing this as above, though I'd argue we
could also just say that running perf disables this mode.
But it's not as clean as the above suggestion.
On 05/09/2015 06:37 AM, Gilad Ben Yossef wrote:
> So, I don't know if it is a practical suggestion or not, but would it better/easier to mark a pending signal on kernel entry for this case?
> The upsides I see is that the user gets her notification (killing the task or just logging the event in a signal handler) and hopefully since return to userspace with a pending signal is already handled we don't need new code in the exit path?
We could certainly do this now that I'm planning to do the
test at kernel entry rather than super-late in kernel exit.
Rather than just do_group_exit(SIGKILL), we should raise
a proper SIGKILL signal via send_sig(SIGKILL, current, 1),
and then we could catch it in the debugger; the pc should
help identify if it was a syscall, page fault, or other trap.
I'm not sure there's an argument to be made for the user
process being able to catch the signal itself; presumably in
production you don't turn this mode on anyway, and in
development, assuming a debugger is probably fine.
But if you want to argue for another signal (SIGILL?) please
do; I'm curious to hear if you think it would make more sense.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* Re: [PATCH 0/3] Allow user to request memory to be locked on page fault
From: Andrew Morton @ 2015-05-11 19:12 UTC (permalink / raw)
To: Eric B Munson
Cc: Shuah Khan, linux-alpha, linux-kernel, linux-mips, linux-parisc,
linuxppc-dev, sparclinux, linux-xtensa, linux-mm, linux-arch,
linux-api
In-Reply-To: <20150511143618.GA30570@akamai.com>
On Mon, 11 May 2015 10:36:18 -0400 Eric B Munson <emunson@akamai.com> wrote:
> On Fri, 08 May 2015, Andrew Morton wrote:
> ...
>
> >
> > Why can't the application mmap only those parts of the file which it
> > wants and mlock those?
>
> There are a number of problems with this approach. The first is it
> presumes the program will know what portions are needed a head of time.
> In many cases this is simply not true. The second problem is the number
> of syscalls required. With my patches, a single mmap() or mlockall()
> call is needed to setup the required locking. Without it, a separate
> mmap call must be made for each piece of data that is needed. This also
> opens up problems for data that is arranged assuming it is contiguous in
> memory. With the single mmap call, the user gets a contiguous VMA
> without having to know about it. mmap() with MAP_FIXED could address
> the problem, but this introduces a new failure mode of your map
> colliding with another that was placed by the kernel.
>
> Another use case for the LOCKONFAULT flag is the security use of
> mlock(). If an application will be using data that cannot be written
> to swap, but the exact size is unknown until run time (all we have a
> build time is the maximum size the buffer can be). The LOCKONFAULT flag
> allows the developer to create the buffer and guarantee that the
> contents are never written to swap without ever consuming more memory
> than is actually needed.
What application(s) or class of applications are we talking about here?
IOW, how generally applicable is this? It sounds rather specialized.
^ permalink raw reply
* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Shuah Khan @ 2015-05-11 18:55 UTC (permalink / raw)
To: Darren Hart, linux-api, Linux Kernel Mailing List
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso,
KOSAKI Motohiro, Shuah Khan
In-Reply-To: <D1763F31.CC237%dvhart@linux.intel.com>
On 05/11/2015 12:22 PM, Darren Hart wrote:
> On 5/11/15, 11:06 AM, "Shuah Khan" <shuahkh@osg.samsung.com> wrote:
>
>> On 05/08/2015 04:09 PM, Darren Hart wrote:
>>> The futextest testsuite [1] provides functional, stress, and
>>> performance tests for the various futex op codes. Those tests will be of
>>> more use to futex developers if they are included with the kernel
>>> source.
>>>
>>> Copy the core infrastructure and the functional tests into selftests,
>>> but adapt them for inclusion in the kernel:
>>>
>>> - Update the Makefile to include the run_tests target, remove reference
>>> to the performance and stress tests from the contributed sources.
>>> - Replace my dead IBM email address with my current Intel email address.
>>> - Remove the warrantee and write-to paragraphs from the license blurbs.
>>> - Remove the NAME section as the filename is easily determined. ;-)
>>> - Make the whitespace usage consistent in a couple of places.
>>> - Cleanup various CodingStyle violations.
>>>
>>> A future effort will explore moving the performance and stress tests
>>> into the kernel.
>>>
>>> 1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git
>>>
>>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>>> Cc: linux-api@vger.kernel.org
>>> Cc: Ingo Molnar <mingo@elte.hu>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Davidlohr Bueso <dave@stgolabs.net>
>>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>>> ---
>>
>> Daren,
>>
>> I am seeing
>>
>> fatal: cannot convert from y to UTF-8
>>
>> when I try to apply the patch. Did you use git send-email?
>
> Hi Shuah,
>
> I've seen that from one of my contributors earlier this month as well. I
> dropped the Content... Header and it applied without problems.
>
> I created the patch using git format-patch and then sent them using git
> send-email - via a script I've been using for years now...
>
> Checking the files locally:
> $ file *
> 0000-cover-letter.patch: ASCII text
> 0001-selftests-Add-futex-functional-tests.patch: unified
> diff output, UTF-8 Unicode text
> 0002-selftest-futex-Update-Makefile-to-use-lib.mk.patch: unified
> diff output, ASCII text
> 0003-selftest-futex-Increment-ksft-pass-and-fail-counters.patch: unified
> diff output, ASCII text
> 0004-selftest-Add-futex-tests-to-the-top-level-Makefile.patch: unified
> diff output, ASCII text
> 0005-kselftest-Add-exit-code-defines.patch: unified
> diff output, ASCII text
>
>
> This shows that only the first in UTF-8 and the rest are ASCII. I presume
> this is due to the Copyright notices in the original files:
>
> Copyright © International Business Machines Corp., 2006-2008
>
> Which use © instead of (C). I just checked and there are 545 instances of
> © in the kernel itself, so this should not present a problem.
>
> I apologize for the glitch in applying. If you use the pull request I
> included that will avoid the mail transport issues, and I will be sure to
> fix my scripts to avoid the issue in the future.
>
> If you want to use the patches directly, please have a look at 1 of 5 and
> just remove the "Content..." header, and I think you'll find "git am" will
> apply it without complaint.
>
Hi Daren,
Removing the Content header got me past the utf error. However, git am
complains:
git am --signoff
../4.2_patches/futex_tests/PATCH_1_5selftestsAddfutexfunctionaltests.mbox
Applying: selftests: Add futex functional tests
/mnt/data/lkml/linux-kselftest/.git/rebase-apply/patch:1457: new blank
line at EOF.
+
warning: 1 line adds whitespace errors.
Could you look into these. Rest of the patches applied fine.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Steven Rostedt @ 2015-05-11 18:36 UTC (permalink / raw)
To: Chris Metcalf
Cc: Frederic Weisbecker, Andrew Morton,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Ingo Molnar,
Gilad Ben Yossef, Peter Zijlstra, Rik van Riel, Tejun Heo,
Thomas Gleixner, Christoph Lameter, Srivatsa S. Bhat,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5550F077.6030906-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
On Mon, 11 May 2015 14:09:59 -0400
Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org> wrote:
> Steven writes:
> > All kidding aside, I think this is the real answer. We don't need a new
> > NO_HZ, we need to make NO_HZ_FULL work. Right now it doesn't do exactly
> > what it was created to do. That should be fixed.
>
> The claim I'm making is that it's worthwhile to differentiate the two
> semantics. Plain NO_HZ_FULL just says "kernel makes a best effort to
> avoid periodic interrupts without incurring any serious overhead". My
> patch series allows an app to request "kernel makes an absolute
> commitment to avoid all interrupts regardless of cost when leaving
> kernel space". These are different enough ideas, and serve different
> enough application needs, that I think they should be kept distinct.
>
> Frederic actually summed this up very nicely in his recent email when
> he wrote "some people may expect hard isolation requirement (Real
> Time, deterministic latency) and others softer isolation (HPC, only
> interested in performance, can live with one rare random tick, so no
> need to loop before returning to userspace until we have the no-noise
> guarantee)."
>
> So we need a way for apps to ask for the "harder" mode and let
> the softer mode be the default.
Fair enough. But I would hope that this would improve on NO_HZ_FULL as
well.
>
> What about naming? We may or may not want to have a Kconfig flag
> for this, and we may or may not have a separate mode for it, but
> we still will need some kind of name to talk about it with. (In
> particular there's the prctl name, if we take that approach, and
> potential boot command-line flags to consider naming for.)
>
> I'll quickly cover the suggestions that have been raised:
>
> - DATAPLANE. My suggestion, seemingly broadly disliked by folks
> who felt it wasn't apparent what it meant. Probably a fair point.
>
> - NO_INTERRUPTS (Andrew). Captures some of the sense, but was
> criticized pretty fairly by Ingo as being too negative, confusing
> with perf nomenclature, and too long :-)
What about NO_INTERRUPTIONS
>
> - PURE (Ingo). Proposed as an alternative to NO_HZ_FULL, but we could
> use it as a name for this new mode. However, I think it's not clear
> enough how FULL and PURE can/should relate to each other from the
> names alone.
I would find the two confusing as well.
>
> - BARE_METAL (me). Ingo observes it's confusing with respect to
> virtualization.
This is also confusing.
>
> - TASK_SOLO (Gilad). Not sure this conveys enough of the semantics.
Agreed.
>
> - OS_LIGHT/OS_ZERO and NO_HZ_LEAVE_ME_THE_FSCK_ALONE. Excellent
> ideas :-)
At least the LEAVE_ME_ALONE conveys the semantics ;-)
>
> - ISOLATION (Frederic). I like this but it conflicts with other uses
> of "isolation" in the kernel: cgroup isolation, lru page isolation,
> iommu isolation, scheduler isolation (at least it's a superset of
> that one), etc. Also, we're not exactly isolating a task - often
> a "dataplane" app consists of a bunch of interacting threads in
> userspace, so not exactly isolated. So perhaps it's too confusing.
>
> - OVERFLOWING (Steven) - not sure I understood this one, honestly.
Actually, that was suggested by Paul McKenney.
>
> I suggested earlier a few other candidates that I don't love, but no
> one commented on: NO_HZ_STRICT, USERSPACE_ONLY, and ZERO_OVERHEAD.
>
> One thing I'm leaning towards is to remove the intermediate state of
> DATAPLANE_ENABLE and say that there is really only one primary state,
> DATAPLANE_QUIESCE (or whatever we call it). The "dataplane but no
> quiesce" state probably isn't that useful, since it doesn't offer the
> hard guarantee that is the entire point of this patch series. So that
> opens the idea of using the name NO_HZ_QUIESCE or just QUIESCE as the
> word that describes the mode; of course this sort of conflicts with
> RCU quiesce (though it is a superset of that so maybe that's OK).
>
> One new idea I had is to use NO_HZ_HARD to reflect what Frederic was
> suggesting about "soft" and "hard" requirements for NO_HZ. So
> enabling NO_HZ_HARD would enable my suggested QUIESCE mode.
>
> One way to focus this discussion is on the user API naming. I had
> prctl(PR_SET_DATAPLANE), which was attractive in being a "positive"
> noun. A lot of the other suggestions fail this test in various way.
> Reasonable candidates seem to be:
>
> PR_SET_OS_ZERO
> PR_SET_TASK_SOLO
> PR_SET_ISOLATION
>
> Another possibility:
>
> PR_SET_NONSTOP
>
> Or take Andrew's NO_INTERRUPTS and have:
>
> PR_SET_UNINTERRUPTED
For another possible answer, what about
SET_TRANQUILITY
A state with no disturbances.
-- Steve
>
> I slightly favor ISOLATION at this point despite the overlap with
> other kernel concepts.
>
> Let the bike-shedding continue! :-)
>
^ permalink raw reply
* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Darren Hart @ 2015-05-11 18:22 UTC (permalink / raw)
To: Shuah Khan, linux-api, Linux Kernel Mailing List
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso,
KOSAKI Motohiro
In-Reply-To: <5550EF8D.8080403@osg.samsung.com>
On 5/11/15, 11:06 AM, "Shuah Khan" <shuahkh@osg.samsung.com> wrote:
>On 05/08/2015 04:09 PM, Darren Hart wrote:
>> The futextest testsuite [1] provides functional, stress, and
>> performance tests for the various futex op codes. Those tests will be of
>> more use to futex developers if they are included with the kernel
>> source.
>>
>> Copy the core infrastructure and the functional tests into selftests,
>> but adapt them for inclusion in the kernel:
>>
>> - Update the Makefile to include the run_tests target, remove reference
>> to the performance and stress tests from the contributed sources.
>> - Replace my dead IBM email address with my current Intel email address.
>> - Remove the warrantee and write-to paragraphs from the license blurbs.
>> - Remove the NAME section as the filename is easily determined. ;-)
>> - Make the whitespace usage consistent in a couple of places.
>> - Cleanup various CodingStyle violations.
>>
>> A future effort will explore moving the performance and stress tests
>> into the kernel.
>>
>> 1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git
>>
>> Cc: Shuah Khan <shuahkh@osg.samsung.com>
>> Cc: linux-api@vger.kernel.org
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Davidlohr Bueso <dave@stgolabs.net>
>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> ---
>
>Daren,
>
>I am seeing
>
>fatal: cannot convert from y to UTF-8
>
>when I try to apply the patch. Did you use git send-email?
Hi Shuah,
I've seen that from one of my contributors earlier this month as well. I
dropped the Content... Header and it applied without problems.
I created the patch using git format-patch and then sent them using git
send-email - via a script I've been using for years now...
Checking the files locally:
$ file *
0000-cover-letter.patch: ASCII text
0001-selftests-Add-futex-functional-tests.patch: unified
diff output, UTF-8 Unicode text
0002-selftest-futex-Update-Makefile-to-use-lib.mk.patch: unified
diff output, ASCII text
0003-selftest-futex-Increment-ksft-pass-and-fail-counters.patch: unified
diff output, ASCII text
0004-selftest-Add-futex-tests-to-the-top-level-Makefile.patch: unified
diff output, ASCII text
0005-kselftest-Add-exit-code-defines.patch: unified
diff output, ASCII text
This shows that only the first in UTF-8 and the rest are ASCII. I presume
this is due to the Copyright notices in the original files:
Copyright © International Business Machines Corp., 2006-2008
Which use © instead of (C). I just checked and there are 545 instances of
© in the kernel itself, so this should not present a problem.
I apologize for the glitch in applying. If you use the pull request I
included that will avoid the mail transport issues, and I will be sure to
fix my scripts to avoid the issue in the future.
If you want to use the patches directly, please have a look at 1 of 5 and
just remove the "Content..." header, and I think you'll find "git am" will
apply it without complaint.
Thanks,
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Chris Metcalf @ 2015-05-11 18:09 UTC (permalink / raw)
To: Steven Rostedt, Frederic Weisbecker
Cc: Andrew Morton, paulmck, Ingo Molnar, Gilad Ben Yossef,
Peter Zijlstra, Rik van Riel, Tejun Heo, Thomas Gleixner,
Christoph Lameter, Srivatsa S. Bhat, linux-doc, linux-api,
linux-kernel
In-Reply-To: <20150511140009.1f7bcf07@gandalf.local.home>
A bunch of issues have been raised by various folks (thanks!) and
I'll try to break them down and respond to them in a few different
emails. This email is just about the issue of naming and whether the
proposed patch series should even have its own "name" or just be part
of NO_HZ_FULL.
First, Ingo and Steven both suggested that this new "dataplane" mode
(or whatever we want to call it; see below) should just be rolled into
the existing NO_HZ_FULL and that we should focus on making that work
better.
Steven writes:
> All kidding aside, I think this is the real answer. We don't need a new
> NO_HZ, we need to make NO_HZ_FULL work. Right now it doesn't do exactly
> what it was created to do. That should be fixed.
The claim I'm making is that it's worthwhile to differentiate the two
semantics. Plain NO_HZ_FULL just says "kernel makes a best effort to
avoid periodic interrupts without incurring any serious overhead". My
patch series allows an app to request "kernel makes an absolute
commitment to avoid all interrupts regardless of cost when leaving
kernel space". These are different enough ideas, and serve different
enough application needs, that I think they should be kept distinct.
Frederic actually summed this up very nicely in his recent email when
he wrote "some people may expect hard isolation requirement (Real
Time, deterministic latency) and others softer isolation (HPC, only
interested in performance, can live with one rare random tick, so no
need to loop before returning to userspace until we have the no-noise
guarantee)."
So we need a way for apps to ask for the "harder" mode and let
the softer mode be the default.
What about naming? We may or may not want to have a Kconfig flag
for this, and we may or may not have a separate mode for it, but
we still will need some kind of name to talk about it with. (In
particular there's the prctl name, if we take that approach, and
potential boot command-line flags to consider naming for.)
I'll quickly cover the suggestions that have been raised:
- DATAPLANE. My suggestion, seemingly broadly disliked by folks
who felt it wasn't apparent what it meant. Probably a fair point.
- NO_INTERRUPTS (Andrew). Captures some of the sense, but was
criticized pretty fairly by Ingo as being too negative, confusing
with perf nomenclature, and too long :-)
- PURE (Ingo). Proposed as an alternative to NO_HZ_FULL, but we could
use it as a name for this new mode. However, I think it's not clear
enough how FULL and PURE can/should relate to each other from the
names alone.
- BARE_METAL (me). Ingo observes it's confusing with respect to
virtualization.
- TASK_SOLO (Gilad). Not sure this conveys enough of the semantics.
- OS_LIGHT/OS_ZERO and NO_HZ_LEAVE_ME_THE_FSCK_ALONE. Excellent
ideas :-)
- ISOLATION (Frederic). I like this but it conflicts with other uses
of "isolation" in the kernel: cgroup isolation, lru page isolation,
iommu isolation, scheduler isolation (at least it's a superset of
that one), etc. Also, we're not exactly isolating a task - often
a "dataplane" app consists of a bunch of interacting threads in
userspace, so not exactly isolated. So perhaps it's too confusing.
- OVERFLOWING (Steven) - not sure I understood this one, honestly.
I suggested earlier a few other candidates that I don't love, but no
one commented on: NO_HZ_STRICT, USERSPACE_ONLY, and ZERO_OVERHEAD.
One thing I'm leaning towards is to remove the intermediate state of
DATAPLANE_ENABLE and say that there is really only one primary state,
DATAPLANE_QUIESCE (or whatever we call it). The "dataplane but no
quiesce" state probably isn't that useful, since it doesn't offer the
hard guarantee that is the entire point of this patch series. So that
opens the idea of using the name NO_HZ_QUIESCE or just QUIESCE as the
word that describes the mode; of course this sort of conflicts with
RCU quiesce (though it is a superset of that so maybe that's OK).
One new idea I had is to use NO_HZ_HARD to reflect what Frederic was
suggesting about "soft" and "hard" requirements for NO_HZ. So
enabling NO_HZ_HARD would enable my suggested QUIESCE mode.
One way to focus this discussion is on the user API naming. I had
prctl(PR_SET_DATAPLANE), which was attractive in being a "positive"
noun. A lot of the other suggestions fail this test in various way.
Reasonable candidates seem to be:
PR_SET_OS_ZERO
PR_SET_TASK_SOLO
PR_SET_ISOLATION
Another possibility:
PR_SET_NONSTOP
Or take Andrew's NO_INTERRUPTS and have:
PR_SET_UNINTERRUPTED
I slightly favor ISOLATION at this point despite the overlap with
other kernel concepts.
Let the bike-shedding continue! :-)
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* Re: [PATCH 0/3] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-05-11 18:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Shuah Khan, linux-alpha, linux-kernel, linux-mips, linux-parisc,
linuxppc-dev, sparclinux, linux-xtensa, linux-mm, linux-arch,
linux-api
In-Reply-To: <20150508124203.6679b1d35ad9555425003929@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]
On Fri, 08 May 2015, Andrew Morton wrote:
> On Fri, 8 May 2015 15:33:43 -0400 Eric B Munson <emunson@akamai.com> wrote:
>
> > mlock() allows a user to control page out of program memory, but this
> > comes at the cost of faulting in the entire mapping when it is
> > allocated. For large mappings where the entire area is not necessary
> > this is not ideal.
> >
> > This series introduces new flags for mmap() and mlockall() that allow a
> > user to specify that the covered are should not be paged out, but only
> > after the memory has been used the first time.
>
> Please tell us much much more about the value of these changes: the use
> cases, the behavioural improvements and performance results which the
> patchset brings to those use cases, etc.
>
To illustrate the proposed use case I wrote a quick program that mmaps
a 5GB file which is filled with random data and accesses 150,000 pages
from that mapping. Setup and processing were timed separately to
illustrate the differences between the three tested approaches. the
setup portion is simply the call to mmap, the processing is the
accessing of the various locations in that mapping. The following
values are in milliseconds and are the averages of 20 runs each with a
call to echo 3 > /proc/sys/vm/drop_caches between each run.
The first mapping was made with MAP_PRIVATE | MAP_LOCKED as a baseline:
Startup average: 9476.506
Processing average: 3.573
The second mapping was simply MAP_PRIVATE but each page was passed to
mlock() before being read:
Startup average: 0.051
Processing average: 721.859
The final mapping was MAP_PRIVATE | MAP_LOCKONFAULT:
Startup average: 0.084
Processing average: 42.125
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Shuah Khan @ 2015-05-11 18:06 UTC (permalink / raw)
To: Darren Hart, linux-api, Linux Kernel Mailing List
Cc: Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso,
KOSAKI Motohiro, Shuah Khan
In-Reply-To: <57d921b1c30e8258ca4b2f55637424527317e92f.1431121818.git.dvhart@linux.intel.com>
On 05/08/2015 04:09 PM, Darren Hart wrote:
> The futextest testsuite [1] provides functional, stress, and
> performance tests for the various futex op codes. Those tests will be of
> more use to futex developers if they are included with the kernel
> source.
>
> Copy the core infrastructure and the functional tests into selftests,
> but adapt them for inclusion in the kernel:
>
> - Update the Makefile to include the run_tests target, remove reference
> to the performance and stress tests from the contributed sources.
> - Replace my dead IBM email address with my current Intel email address.
> - Remove the warrantee and write-to paragraphs from the license blurbs.
> - Remove the NAME section as the filename is easily determined. ;-)
> - Make the whitespace usage consistent in a couple of places.
> - Cleanup various CodingStyle violations.
>
> A future effort will explore moving the performance and stress tests
> into the kernel.
>
> 1. http://git.kernel.org/cgit/linux/kernel/git/dvhart/futextest.git
>
> Cc: Shuah Khan <shuahkh@osg.samsung.com>
> Cc: linux-api@vger.kernel.org
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> ---
Daren,
I am seeing
fatal: cannot convert from y to UTF-8
when I try to apply the patch. Did you use git send-email?
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Steven Rostedt @ 2015-05-11 18:00 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Andrew Morton, paulmck, Ingo Molnar, Chris Metcalf,
Gilad Ben Yossef, Ingo Molnar, Peter Zijlstra, Rik van Riel,
Tejun Heo, Thomas Gleixner, Christoph Lameter, Srivatsa S. Bhat,
linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511173305.GC32512@lerouge>
On Mon, 11 May 2015 19:33:06 +0200
Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Mon, May 11, 2015 at 10:27:44AM -0700, Andrew Morton wrote:
> > On Mon, 11 May 2015 10:19:16 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> >
> > > On Mon, May 11, 2015 at 08:57:59AM -0400, Steven Rostedt wrote:
> > > >
> > > > NO_HZ_LEAVE_ME_THE_FSCK_ALONE!
> > >
> > > NO_HZ_OVERFLOWING?
> >
> > Actually, "NO_HZ" shouldn't appear in the name at all. The objective
> > is to permit userspace to execute without interruption. NO_HZ is a
> > part of that, as is NO_INTERRUPTS. The "NO_HZ" thing is a historical
> > artifact from an early partial implementation.
>
> Agreed! Which is why I'd rather advocate in favour of CONFIG_ISOLATION.
Then we should have CONFIG_LEAVE_ME_THE_FSCK_ALONE. Hmm, I guess that's
just an synonym for CONFIG_ISOLATION.
-- Steve
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Frederic Weisbecker @ 2015-05-11 17:33 UTC (permalink / raw)
To: Andrew Morton
Cc: paulmck, Steven Rostedt, Ingo Molnar, Chris Metcalf,
Gilad Ben Yossef, Ingo Molnar, Peter Zijlstra, Rik van Riel,
Tejun Heo, Thomas Gleixner, Christoph Lameter, Srivatsa S. Bhat,
linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511102744.9ebb2d05a7e8b457d03430bf@linux-foundation.org>
On Mon, May 11, 2015 at 10:27:44AM -0700, Andrew Morton wrote:
> On Mon, 11 May 2015 10:19:16 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
> > On Mon, May 11, 2015 at 08:57:59AM -0400, Steven Rostedt wrote:
> > >
> > > NO_HZ_LEAVE_ME_THE_FSCK_ALONE!
> >
> > NO_HZ_OVERFLOWING?
>
> Actually, "NO_HZ" shouldn't appear in the name at all. The objective
> is to permit userspace to execute without interruption. NO_HZ is a
> part of that, as is NO_INTERRUPTS. The "NO_HZ" thing is a historical
> artifact from an early partial implementation.
Agreed! Which is why I'd rather advocate in favour of CONFIG_ISOLATION.
^ permalink raw reply
* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Sage Weil @ 2015-05-11 17:30 UTC (permalink / raw)
To: Trond Myklebust
Cc: Dave Chinner, Zach Brown, Alexander Viro,
Linux FS-devel Mailing List, Linux Kernel Mailing List,
Linux API Mailing List
In-Reply-To: <CAHQdGtT3rCf-ycAYw-=7HGaemg1+HfY8sw3+kb54VHONxDyP3w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, 11 May 2015, Trond Myklebust wrote:
> On Mon, May 11, 2015 at 12:39 PM, Sage Weil <sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org> wrote:
> > On Mon, 11 May 2015, Dave Chinner wrote:
> >> On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
> >> > On Fri, May 8, 2015 at 6:24 PM, Sage Weil <sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org> wrote:
> >> > > I'm sure you realize what we're try to achieve is the same "invisible IO"
> >> > > that the XFS open by handle ioctls do by default. Would you be more
> >> > > comfortable if this option where only available to the generic
> >> > > open_by_handle syscall, and not to open(2)?
> >> >
> >> > It should be an ioctl(). It has no business being part of
> >> > open_by_handle either, since that is another generic interface.
> >
> > Our use-case doesn't make sense on network file systems, but it does on
> > any reasonably featureful local filesystem, and the goal is to be generic
> > there. If mtime is critical to a network file system's consistency it
> > seems pretty reasonable to disallow/ignore it for just that file system
> > (e.g., by masking off the flag at open time), as others won't have that
> > same problem (cephfs doesn't, for example).
> >
> > Perhaps making each fs opt-in instead of handling it in a generic path
> > would alleviate this concern?
>
> The issue isn't whether or not you have a network file system, it's
> whether or not you want users to be able to manage data. mtime isn't
> useful for the application (which knows whether or not it has changed
> the file) or for the filesystem (ditto). It exists, rather, in order
> to enable data management by users and other applications, letting
> them know whether or not the data contents of the file have changed,
> and when that change occurred.
Agreed.
> If you are able to guarantee that your users don't care about that,
> then fine, but that would be a very special case that doesn't fit the
> way that most data centres are run. Backups are one case where mtime
> matters, tiering and archiving is another.
This is true, although I argue it is becoming increasingly common for the
data management (including backups and so forth) to be layered not on top
of the POSIX file system but on something higher up in the stack. This is
true of pretty much any distributed system (ceph, cassandra, mongo, etc.,
and I assume commercial databases like Oracle, too) where backups,
replication, and any other DR strategies need to be orchestrated across
nodes to be consistent--simply copying files out from underneath them is
already insufficient and a recipe for disaster.
There is a growing category of applications that can benefit from this
capability...
> Neither of these examples
> cases are under the control of the application that calls
> open(O_NOMTIME).
Wouldn't a mount option (e.g., allow_nomtime) address this concern? Only
nodes provisioned explicitly to run these systems would be enable this
option.
> >> I'm happy for it to be an ioctl interface - even an XFS specific
> >> interface if you want to go that route, Sage - and it probably
> >> should emit a warning to syslog first time it is used so there is
> >> trace for bug triage purposes. i.e. we know the app is not using
> >> mtime updates, so bug reports that are the result of mtime
> >> mishandling don't result in large amounts of wasted developer time
> >> trying to understand them...
> >
> > A warning on using the interface (or when mounting with user_nomtime)
> > sounds reasonable.
> >
> > I'd rather not make this XFS specific as other local filesystmes (ext4,
> > f2fs, possibly btrfs) would similarly benefit. (And if we want to target
> > XFS specifically the existing XFS open-by-handle ioctl is sufficient as it
> > already does O_NOMTIME unconditionally.)
>
> Lack of a namespace, doesn't imply that you don't want to manage the
> data. The whole point of using object storage instead of plain old
> block storage is to be able to provide whatever metadata you still
> need in order to manage the object.
Yeah, agreed--this is presumably why open_by_handle(2) (which is what we'd
like to use) doesn't assume O_NOMTIME.
Thanks!
sage
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Andrew Morton @ 2015-05-11 17:27 UTC (permalink / raw)
To: paulmck
Cc: Steven Rostedt, Ingo Molnar, Chris Metcalf, Gilad Ben Yossef,
Ingo Molnar, Peter Zijlstra, Rik van Riel, Tejun Heo,
Frederic Weisbecker, Thomas Gleixner, Christoph Lameter,
Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511171916.GN6776@linux.vnet.ibm.com>
On Mon, 11 May 2015 10:19:16 -0700 "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> On Mon, May 11, 2015 at 08:57:59AM -0400, Steven Rostedt wrote:
> >
> > NO_HZ_LEAVE_ME_THE_FSCK_ALONE!
>
> NO_HZ_OVERFLOWING?
Actually, "NO_HZ" shouldn't appear in the name at all. The objective
is to permit userspace to execute without interruption. NO_HZ is a
part of that, as is NO_INTERRUPTS. The "NO_HZ" thing is a historical
artifact from an early partial implementation.
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Paul E. McKenney @ 2015-05-11 17:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Andrew Morton, Chris Metcalf, Gilad Ben Yossef,
Ingo Molnar, Peter Zijlstra, Rik van Riel, Tejun Heo,
Frederic Weisbecker, Thomas Gleixner, Christoph Lameter,
Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511085759.71deeb64@gandalf.local.home>
On Mon, May 11, 2015 at 08:57:59AM -0400, Steven Rostedt wrote:
>
> NO_HZ_LEAVE_ME_THE_FSCK_ALONE!
NO_HZ_OVERFLOWING?
Kconfig naming controversy aside, I believe this patchset is addressing
a real need. Might need additional adjustment, but something useful.
Thanx, Paul
> On Sat, 9 May 2015 09:05:38 +0200
> Ingo Molnar <mingo@kernel.org> wrote:
>
> > So I think we should either rename NO_HZ_FULL to NO_HZ_PURE, or keep
> > it at NO_HZ_FULL: because the intention of NO_HZ_FULL was always to be
> > such a 'zero overhead' mode of operation, where if user-space runs, it
> > won't get interrupted in any way.
>
>
> All kidding aside, I think this is the real answer. We don't need a new
> NO_HZ, we need to make NO_HZ_FULL work. Right now it doesn't do exactly
> what it was created to do. That should be fixed.
>
> Please lets get NO_HZ_FULL up to par. That should be the main focus.
>
> -- Steve
>
^ permalink raw reply
* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Trond Myklebust @ 2015-05-11 17:12 UTC (permalink / raw)
To: Sage Weil
Cc: Dave Chinner, Zach Brown, Alexander Viro,
Linux FS-devel Mailing List, Linux Kernel Mailing List,
Linux API Mailing List
In-Reply-To: <alpine.DEB.2.00.1505110925150.28239@cobra.newdream.net>
On Mon, May 11, 2015 at 12:39 PM, Sage Weil <sage@newdream.net> wrote:
> On Mon, 11 May 2015, Dave Chinner wrote:
>> On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
>> > On Fri, May 8, 2015 at 6:24 PM, Sage Weil <sage@newdream.net> wrote:
>> > > I'm sure you realize what we're try to achieve is the same "invisible IO"
>> > > that the XFS open by handle ioctls do by default. Would you be more
>> > > comfortable if this option where only available to the generic
>> > > open_by_handle syscall, and not to open(2)?
>> >
>> > It should be an ioctl(). It has no business being part of
>> > open_by_handle either, since that is another generic interface.
>
> Our use-case doesn't make sense on network file systems, but it does on
> any reasonably featureful local filesystem, and the goal is to be generic
> there. If mtime is critical to a network file system's consistency it
> seems pretty reasonable to disallow/ignore it for just that file system
> (e.g., by masking off the flag at open time), as others won't have that
> same problem (cephfs doesn't, for example).
>
> Perhaps making each fs opt-in instead of handling it in a generic path
> would alleviate this concern?
The issue isn't whether or not you have a network file system, it's
whether or not you want users to be able to manage data. mtime isn't
useful for the application (which knows whether or not it has changed
the file) or for the filesystem (ditto). It exists, rather, in order
to enable data management by users and other applications, letting
them know whether or not the data contents of the file have changed,
and when that change occurred.
If you are able to guarantee that your users don't care about that,
then fine, but that would be a very special case that doesn't fit the
way that most data centres are run. Backups are one case where mtime
matters, tiering and archiving is another. Neither of these examples
cases are under the control of the application that calls
open(O_NOMTIME).
>> I'm happy for it to be an ioctl interface - even an XFS specific
>> interface if you want to go that route, Sage - and it probably
>> should emit a warning to syslog first time it is used so there is
>> trace for bug triage purposes. i.e. we know the app is not using
>> mtime updates, so bug reports that are the result of mtime
>> mishandling don't result in large amounts of wasted developer time
>> trying to understand them...
>
> A warning on using the interface (or when mounting with user_nomtime)
> sounds reasonable.
>
> I'd rather not make this XFS specific as other local filesystmes (ext4,
> f2fs, possibly btrfs) would similarly benefit. (And if we want to target
> XFS specifically the existing XFS open-by-handle ioctl is sufficient as it
> already does O_NOMTIME unconditionally.)
Lack of a namespace, doesn't imply that you don't want to manage the
data. The whole point of using object storage instead of plain old
block storage is to be able to provide whatever metadata you still
need in order to manage the object.
Cheers
Trond
^ permalink raw reply
* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Sage Weil @ 2015-05-11 16:39 UTC (permalink / raw)
To: Dave Chinner
Cc: Trond Myklebust, Zach Brown, Alexander Viro,
Linux FS-devel Mailing List, Linux Kernel Mailing List,
Linux API Mailing List
In-Reply-To: <20150511073103.GO4327@dastard>
On Mon, 11 May 2015, Dave Chinner wrote:
> On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
> > On Fri, May 8, 2015 at 6:24 PM, Sage Weil <sage@newdream.net> wrote:
> > > I'm sure you realize what we're try to achieve is the same "invisible IO"
> > > that the XFS open by handle ioctls do by default. Would you be more
> > > comfortable if this option where only available to the generic
> > > open_by_handle syscall, and not to open(2)?
> >
> > It should be an ioctl(). It has no business being part of
> > open_by_handle either, since that is another generic interface.
Our use-case doesn't make sense on network file systems, but it does on
any reasonably featureful local filesystem, and the goal is to be generic
there. If mtime is critical to a network file system's consistency it
seems pretty reasonable to disallow/ignore it for just that file system
(e.g., by masking off the flag at open time), as others won't have that
same problem (cephfs doesn't, for example).
Perhaps making each fs opt-in instead of handling it in a generic path
would alleviate this concern?
> I'm happy for it to be an ioctl interface - even an XFS specific
> interface if you want to go that route, Sage - and it probably
> should emit a warning to syslog first time it is used so there is
> trace for bug triage purposes. i.e. we know the app is not using
> mtime updates, so bug reports that are the result of mtime
> mishandling don't result in large amounts of wasted developer time
> trying to understand them...
A warning on using the interface (or when mounting with user_nomtime)
sounds reasonable.
I'd rather not make this XFS specific as other local filesystmes (ext4,
f2fs, possibly btrfs) would similarly benefit. (And if we want to target
XFS specifically the existing XFS open-by-handle ioctl is sufficient as it
already does O_NOMTIME unconditionally.)
sage
^ permalink raw reply
* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Sage Weil @ 2015-05-11 16:24 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Trond Myklebust, Dave Chinner, Zach Brown, Alexander Viro,
Linux FS-devel Mailing List, Linux Kernel Mailing List,
Linux API Mailing List
In-Reply-To: <20150511144719.GA14088-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
On Mon, 11 May 2015, Theodore Ts'o wrote:
> On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
> > That makes it completely non-generic though. By putting this in the
> > VFS, you are giving applications a loaded gun that is pointed straight
> > at the application user's head.
>
> Let me re-ask the question that I asked last week (and was apparently
> ignored). Why not trying to use the lazytime feature instead of
> pointing a head straight at the application's --- and system
> administrators' --- heads?
Sorry Ted, I thought I responded already.
The goal is to avoid inode writeout entirely when we can, and
as I understand it lazytime will still force writeout before the inode
is dropped from the cache. In systems like Ceph in particular, the
IOs can be spread across lots of files, so simply deferring writeout
doesn't always help.
sage
^ permalink raw reply
* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Frederic Weisbecker @ 2015-05-11 15:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Andrew Morton, Chris Metcalf, Gilad Ben Yossef,
Ingo Molnar, Peter Zijlstra, Rik van Riel, Tejun Heo,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <20150511085759.71deeb64@gandalf.local.home>
On Mon, May 11, 2015 at 08:57:59AM -0400, Steven Rostedt wrote:
>
> NO_HZ_LEAVE_ME_THE_FSCK_ALONE!
>
>
> On Sat, 9 May 2015 09:05:38 +0200
> Ingo Molnar <mingo@kernel.org> wrote:
>
> > So I think we should either rename NO_HZ_FULL to NO_HZ_PURE, or keep
> > it at NO_HZ_FULL: because the intention of NO_HZ_FULL was always to be
> > such a 'zero overhead' mode of operation, where if user-space runs, it
> > won't get interrupted in any way.
>
>
> All kidding aside, I think this is the real answer. We don't need a new
> NO_HZ, we need to make NO_HZ_FULL work. Right now it doesn't do exactly
> what it was created to do. That should be fixed.
>
> Please lets get NO_HZ_FULL up to par. That should be the main focus.
Now if we can achieve to make NO_HZ_FULL behave in a specific way
that fits everyone's usecase, I'll be happy.
But some people may expect hard isolation requirement (Real Time, deterministic
latency) and others softer isolation (HPC, only interested in performance, can
live with one rare random tick, so no need to loop before returning to userspace
until we have the no-noise guarantee).
I expect some Real Time users may want this kind of dataplane mode where a syscall
or whatever sleeps until the system is ready to provide the guarantee that no
disturbance is going to happen for a given time. I'm not sure HPC users are interested
in that.
In fact it goes along the fact that NO_HZ_FULL was really only supposed to be about
the tick and now people are introducing more and more kernel default presetting that
assume NO_HZ_FULL implies ISOLATION which is about all kind of noise (tick, tasks, irqs,
...). Which is true but what kind of ISOLATION?
Probably NO_HZ_FULL should really only be about stopping the tick then some sort
of CONFIG_ISOLATION would drive the kind of isolation we are interested in
and hereby the behaviour of NO_HZ_FULL, workqueues, timers, tasks affinity, irqs
affinity, dataplane mode, ...
^ permalink raw reply
* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Theodore Ts'o @ 2015-05-11 14:47 UTC (permalink / raw)
To: Trond Myklebust
Cc: Sage Weil, Dave Chinner, Zach Brown, Alexander Viro,
Linux FS-devel Mailing List, Linux Kernel Mailing List,
Linux API Mailing List
In-Reply-To: <CAHQdGtTFTN2XuvmarFZ9HPQV=cuhh7FosdHSrJME_U4htr=i8w@mail.gmail.com>
On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
> That makes it completely non-generic though. By putting this in the
> VFS, you are giving applications a loaded gun that is pointed straight
> at the application user's head.
Let me re-ask the question that I asked last week (and was apparently
ignored). Why not trying to use the lazytime feature instead of
pointing a head straight at the application's --- and system
administrators' --- heads?
- Ted
^ permalink raw reply
* Re: [PATCH 1/2] clone: Support passing tls argument via C rather than pt_regs magic
From: Josh Triplett @ 2015-05-11 14:47 UTC (permalink / raw)
To: Vineet Gupta
Cc: Andy Lutomirski, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, Linus Torvalds, linux-api@vger.kernel.org,
linux-kernel@vger.kernel.org, x86@kernel.org
In-Reply-To: <C2D7FE5348E1B147BCA15975FBA230756659F586@IN01WEMBXB.internal.synopsys.com>
On Mon, May 11, 2015 at 02:31:39PM +0000, Vineet Gupta wrote:
> On Tuesday 21 April 2015 11:17 PM, Josh Triplett wrote:
> > clone with CLONE_SETTLS accepts an argument to set the thread-local
> > storage area for the new thread. sys_clone declares an int argument
> > tls_val in the appropriate point in the argument list (based on the
> > various CLONE_BACKWARDS variants), but doesn't actually use or pass
> > along that argument. Instead, sys_clone calls do_fork, which calls
> > copy_process, which calls the arch-specific copy_thread, and copy_thread
> > pulls the corresponding syscall argument out of the pt_regs captured at
> > kernel entry (knowing what argument of clone that architecture passes
> > tls in).
> >
> > Apart from being awful and inscrutable, that also only works because
> > only one code path into copy_thread can pass the CLONE_SETTLS flag, and
> > that code path comes from sys_clone with its architecture-specific
> > argument-passing order. This prevents introducing a new version of the
> > clone system call without propagating the same architecture-specific
> > position of the tls argument.
> >
> > However, there's no reason to pull the argument out of pt_regs when
> > sys_clone could just pass it down via C function call arguments.
> >
> > Introduce a new CONFIG_HAVE_COPY_THREAD_TLS for architectures to opt
> > into, and a new copy_thread_tls that accepts the tls parameter as an
> > additional unsigned long (syscall-argument-sized) argument.
> > Change sys_clone's tls argument to an unsigned long (which does
> > not change the ABI), and pass that down to copy_thread_tls.
> >
> > Architectures that don't opt into copy_thread_tls will continue to
> > ignore the C argument to sys_clone in favor of the pt_regs captured at
> > kernel entry, and thus will be unable to introduce new versions of the
> > clone syscall.
> >
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
> > Acked-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > arch/Kconfig | 7 ++++++
> > include/linux/sched.h | 14 ++++++++++++
> > include/linux/syscalls.h | 6 +++---
> > kernel/fork.c | 55 +++++++++++++++++++++++++++++++-----------------
> > 4 files changed, 60 insertions(+), 22 deletions(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 05d7a8a..4834a58 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -484,6 +484,13 @@ config HAVE_IRQ_EXIT_ON_IRQ_STACK
> > This spares a stack switch and improves cache usage on softirq
> > processing.
> >
> > +config HAVE_COPY_THREAD_TLS
> > + bool
> > + help
> > + Architecture provides copy_thread_tls to accept tls argument via
> > + normal C parameter passing, rather than extracting the syscall
> > + argument from pt_regs.
> > +
> > #
> > # ABI hall of shame
> > #
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index a419b65..2cc88c6 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2480,8 +2480,22 @@ extern struct mm_struct *mm_access(struct task_struct *task, unsigned int mode);
> > /* Remove the current tasks stale references to the old mm_struct */
> > extern void mm_release(struct task_struct *, struct mm_struct *);
> >
> > +#ifdef CONFIG_HAVE_COPY_THREAD_TLS
> > +extern int copy_thread_tls(unsigned long, unsigned long, unsigned long,
> > + struct task_struct *, unsigned long);
> > +#else
> > extern int copy_thread(unsigned long, unsigned long, unsigned long,
> > struct task_struct *);
> > +
> > +/* Architectures that haven't opted into copy_thread_tls get the tls argument
> > + * via pt_regs, so ignore the tls argument passed via C. */
> > +static inline int copy_thread_tls(
> > + unsigned long clone_flags, unsigned long sp, unsigned long arg,
> > + struct task_struct *p, unsigned long tls)
> > +{
> > + return copy_thread(clone_flags, sp, arg, p);
> > +}
> > +#endif
>
> Is this detour really needed. Can we not update copy_thread() of all arches in one
> go and add the tls arg, w/o using it.
>
> And then arch maintainers can micro-optimize their code to use that arg vs.
> pt_regs->rxx version at their own leisure. The only downside I see with that is
> bigger churn (touches all arches), and a interim unused arg warning ?
In addition to the cleanup and simplification, the purpose of this patch
is specifically to make sure that any architecture opting into
HAVE_COPY_THREAD_TLS does *not* care how tls is passed in, and in
particular doesn't depend on it arriving in a specific syscall argument.
I have patches in flight (for CLONE_FD and clone4) that depend on that
assumption, by introducing additional syscalls (with tls passed
differently) that call down through these same code paths.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH 0/3] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-05-11 14:36 UTC (permalink / raw)
To: Andrew Morton
Cc: Shuah Khan, linux-alpha, linux-kernel, linux-mips, linux-parisc,
linuxppc-dev, sparclinux, linux-xtensa, linux-mm, linux-arch,
linux-api
In-Reply-To: <20150508131523.f970d13a213bca63bd6f2619@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]
On Fri, 08 May 2015, Andrew Morton wrote:
> On Fri, 8 May 2015 16:06:10 -0400 Eric B Munson <emunson@akamai.com> wrote:
>
> > On Fri, 08 May 2015, Andrew Morton wrote:
> >
> > > On Fri, 8 May 2015 15:33:43 -0400 Eric B Munson <emunson@akamai.com> wrote:
> > >
> > > > mlock() allows a user to control page out of program memory, but this
> > > > comes at the cost of faulting in the entire mapping when it is
> > > > allocated. For large mappings where the entire area is not necessary
> > > > this is not ideal.
> > > >
> > > > This series introduces new flags for mmap() and mlockall() that allow a
> > > > user to specify that the covered are should not be paged out, but only
> > > > after the memory has been used the first time.
> > >
> > > Please tell us much much more about the value of these changes: the use
> > > cases, the behavioural improvements and performance results which the
> > > patchset brings to those use cases, etc.
> > >
> >
> > The primary use case is for mmaping large files read only. The process
> > knows that some of the data is necessary, but it is unlikely that the
> > entire file will be needed. The developer only wants to pay the cost to
> > read the data in once. Unfortunately developer must choose between
> > allowing the kernel to page in the memory as needed and guaranteeing
> > that the data will only be read from disk once. The first option runs
> > the risk of having the memory reclaimed if the system is under memory
> > pressure, the second forces the memory usage and startup delay when
> > faulting in the entire file.
>
> Why can't the application mmap only those parts of the file which it
> wants and mlock those?
There are a number of problems with this approach. The first is it
presumes the program will know what portions are needed a head of time.
In many cases this is simply not true. The second problem is the number
of syscalls required. With my patches, a single mmap() or mlockall()
call is needed to setup the required locking. Without it, a separate
mmap call must be made for each piece of data that is needed. This also
opens up problems for data that is arranged assuming it is contiguous in
memory. With the single mmap call, the user gets a contiguous VMA
without having to know about it. mmap() with MAP_FIXED could address
the problem, but this introduces a new failure mode of your map
colliding with another that was placed by the kernel.
Another use case for the LOCKONFAULT flag is the security use of
mlock(). If an application will be using data that cannot be written
to swap, but the exact size is unknown until run time (all we have a
build time is the maximum size the buffer can be). The LOCKONFAULT flag
allows the developer to create the buffer and guarantee that the
contents are never written to swap without ever consuming more memory
than is actually needed.
>
> > I am working on getting startup times with and without this change for
> > an application, I will post them as soon as I have them.
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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