* Re: [PATCH v4 1/2] capabilities: Ambient capabilities
From: Andrew Morton @ 2015-07-17 20:50 UTC (permalink / raw)
To: Christoph Lameter
Cc: Andy Lutomirski, James Morris, Andy Lutomirski, Andrew Morton,
Serge E. Hallyn, Serge Hallyn, James Morris, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet
In-Reply-To: <alpine.DEB.2.11.1507171539590.13763-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org>
On Fri, 17 Jul 2015 15:40:57 -0500 (CDT) Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> Here is a test program that can be used to verify the functionality.
I slurped that into the changelog.
Adding a selftest into tools/testing/selftests/ would be appropriate.
^ permalink raw reply
* Re: [PATCH v4 1/2] capabilities: Ambient capabilities
From: Christoph Lameter @ 2015-07-17 20:40 UTC (permalink / raw)
To: Andy Lutomirski
Cc: James Morris, Andy Lutomirski, Andrew Morton, Serge E. Hallyn,
Serge Hallyn, James Morris, Jarkko Sakkinen, Ted Ts'o,
Andrew G. Morgan, Linux API, Mimi Zohar, Michael Kerrisk,
Austin S Hemmelgarn, linux-security-module, Aaron Jones,
Serge Hallyn, LKML, Markku Savela, Kees Cook, Jonathan Corbet
In-Reply-To: <CALCETrWMBZ0p7P1FvtrYp5ChJXvvRzy0JpjS8KwUUa5m9gqgQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Here is a test program that can be used to verify the functionality.
------ ambient_test.c -----------------------
/*
* Test program for the ambient capabilities. This program spawns a shell
* that allows running processes with a defined set of capabilities.
*
* (C) 2015 Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
* Released under: GPL v3 or later.
*
*
* Compile using:
*
* gcc -o ambient_test ambient_test.o -lcap-ng
*
* This program must have the following capabilities to run properly:
* Permissions for CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_NICE
*
* A command to equip the binary with the right caps is:
*
* setcap cap_net_raw,cap_net_admin,cap_sys_nice+p ambient_test
*
*
* To get a shell with additional caps that can be inherited by other processes:
*
* ./ambient_test /bin/bash
*
*
* Verifying that it works:
*
* From the bash spawed by ambient_test run
*
* cat /proc/$$/status
*
* and have a look at the capabilities.
*/
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <cap-ng.h>
#include <sys/prctl.h>
#include <linux/capability.h>
/*
* Definitions from the kernel header files. These are going to be removed
* when the /usr/include files have these defined.
*/
#define PR_CAP_AMBIENT 47
#define PR_CAP_AMBIENT_IS_SET 1
#define PR_CAP_AMBIENT_RAISE 2
#define PR_CAP_AMBIENT_LOWER 3
#define PR_CAP_AMBIENT_CLEAR_ALL 4
static void set_ambient_cap(int cap)
{
int rc;
capng_get_caps_process();
rc = capng_update(CAPNG_ADD, CAPNG_INHERITABLE, cap);
if (rc) {
printf("Cannot add inheritable cap\n");
exit(2);
}
capng_apply(CAPNG_SELECT_CAPS);
/* Note the two 0s at the end. Kernel checks for these */
if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0, 0)) {
perror("Cannot set cap");
exit(1);
}
}
int main(int argc, char **argv)
{
int rc;
set_ambient_cap(CAP_NET_RAW);
set_ambient_cap(CAP_NET_ADMIN);
set_ambient_cap(CAP_SYS_NICE);
printf("Ambient_test forking shell\n");
if (execv(argv[1], argv + 1))
perror("Cannot exec");
return 0;
}
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Linus Torvalds @ 2015-07-17 19:11 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mathieu Desnoyers, Ben Maurer, Paul Turner, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, rostedt, Paul E. McKenney,
Josh Triplett, Lai Jiangshan, Andrew Morton, linux-api,
libc-alpha
In-Reply-To: <CALCETrVNcLpZVATHOs-gZR9AMUSW_ScvXW_0oY=OnFHXXHLdaA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 17, 2015 at 11:55 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>
> I doubt I'll succeed, too. But I don't want anything resembling full
> per-cpu page tables -- per-cpu pgds would be plenty. Still kinda
> nasty to implement.
Per-cpu pgd's would have been trivial in the old 32-bit PAE
environment. There's only four entries at the top level, and they have
to be allocated at process startup anyway - and we wopuldn't even have
to do a per-cpu-and-VM allocation, we'd just have done one single
per-cpu entry, and when switching tasks we'd *copy* the VM entries to
the per-cpu one and re-load %cr3 with the same address. I thought
about it.
But I'm really happy we never went down that road. It's non-portable,
even on x86-32 (because it requires PAE). And even there it would be
limited to "the top 1GB of virtual address space ends up being
per-cpu", and then you have to get the vmalloc space right etc, so you
have that one PGE entry for the kernel mapping that you can make be
percpu and play tricks in. So you'd basically allocate one page per
CPU for the magic upper PGD entry that maps the top 1GB, and edit that
on-the-fly as you do task-switching. Very specialized, and the upside
was very dubious.
And that "simple" trick is not really doable with the x86-64 model any
more (you can't copy 4kB efficiently the way you could copy 32 _bytes_
efficiently). And you really don't want to pre-allocate the whole
top-level PGD either. So all the things that made it "easy" for 32-bit
PAE basically went away with x86-64.
No, I think the only thing that would make it possible is if there is
some architecture extension that replaces part of the page table
mappings with a percpu MSR describing a magic mapping or two. It would
be trivial to do such an addition in hardware (it's not even in the
critical path, it would be just a new magic special case for the TLB
fill code), but without hardware support it's just not a good idea.
(And I'm not claiming that the hw extension for per-cpu mappigns would
be a good idea either, although I think it would be an _interesting_
toy to play with ;)
Linus
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Andy Lutomirski @ 2015-07-17 18:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Mathieu Desnoyers, Ben Maurer, Paul Turner, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, rostedt, Paul E. McKenney,
Josh Triplett, Lai Jiangshan, Andrew Morton, linux-api,
libc-alpha
In-Reply-To: <CA+55aFz-VBnEKh0SPKgu8xV5=Zb+=6odybVUDoOYOknshbcFJA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 17, 2015 at 11:48 AM, Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> On Thu, Jul 16, 2015 at 12:27 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>
>> If we actually bit the bullet and implemented per-cpu mappings
>
> That's not ever going to happen.
>
> Per-cpu page tables are a complete disaster. It's a recipe for crazy
> race conditions, when you have CPUs that update things like
> dirty/accessed bits atomically etc, and you have fundamental races
> when multiple CPU's allocating page tables at the same time (remember:
> we have concurrent page faults, and the locking is not per-vm, it's at
> a finer granularity).
>
> It's also a big memory management problem when you have lots and lots of CPU's.
>
> So you can try to prove me wrong, but seriously, I doubt you'll succeed.
I doubt I'll succeed, too. But I don't want anything resembling full
per-cpu page tables -- per-cpu pgds would be plenty. Still kinda
nasty to implement. On the other hand, getting rid of swapgs would be
a nice win.
>
> On x86, if you want per-cpu memory areas, you should basically plan on
> using segment registers instead (although other odd state has been
> used - there's been the people who use segment limits etc rather than
> the *pointer* itself, preferring to use "lsl" to get percpu data. You
> could also imaging hiding things in the vector state somewhere if you
> control your environment well enough).
I do think we should implement per-cpu descriptor bases or gs bases,
and we should also implement rd/wrfsgsbase. We should do them
together, give them well-defined sematics, and write tests. The
current "segment register state kinda sorta context switches correctly
as long as no one looks carefully" approach is no good. And once we
do that, we don't need a cached cpu number.
Sigh, if we had clean per-cpu memory mappings and got rid of swapgs,
then implementing the fsgsbase stuff would be so much easier.
(Although -- we could plausibly use r15 or something as our percpu
pointer in the kernel without too much loss, which would also get rid
of the fsgsbase mess. Hmm. It would make paranoid entries much
faster, too.)
Anyway, I do intend to ask Intel for real per-cpu mappings of some
sort if they ever ask my opinion again. Maybe they'll give us
something in a few decades.
--Andy
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Linus Torvalds @ 2015-07-17 18:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mathieu Desnoyers, Ben Maurer, Paul Turner, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, rostedt, Paul E. McKenney,
Josh Triplett, Lai Jiangshan, Andrew Morton, linux-api,
libc-alpha
In-Reply-To: <CALCETrWEKE=mow3vVh7C4r8CuGy_d5VOEz7KkpijuR5cpBfFtg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Jul 16, 2015 at 12:27 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>
> If we actually bit the bullet and implemented per-cpu mappings
That's not ever going to happen.
Per-cpu page tables are a complete disaster. It's a recipe for crazy
race conditions, when you have CPUs that update things like
dirty/accessed bits atomically etc, and you have fundamental races
when multiple CPU's allocating page tables at the same time (remember:
we have concurrent page faults, and the locking is not per-vm, it's at
a finer granularity).
It's also a big memory management problem when you have lots and lots of CPU's.
So don't go there. The only way to do per-cpu virtual mappings is
hardware-specific, it if you have hardware that explicitly allows
inserting percpu TLB entries (while still sharing the page tables),
then that would be ok. And we don't have that on x86. MIPS has
explicit support for these kinds of TLB backs, and obviously on other
architectures you might be able to play games with the SW-fill TLB,
but on x86 there's no hardware support for per-CPU TLB filling.
And this is not just theory. We've seen what happens when people try
to do per-thread page tables. It's happened several times, and it's a
fundamental mistake. Plan-9 had "private mappings" because that's how
they did stacks (ie the stack mappings were thread-local), and it
means that thread switching is fundamentally broken. I think Mach did
too. And per-cpu page tables are less broken from a scheduling
standpoint than per-thread page tables, but still do share a lot of
the synchronization problems, and have some allocation issues all
their own.
The Linux VM model of "one page table per VM" is the right one.
Anything else sucks, and makes threading a disaster.
So you can try to prove me wrong, but seriously, I doubt you'll succeed.
On x86, if you want per-cpu memory areas, you should basically plan on
using segment registers instead (although other odd state has been
used - there's been the people who use segment limits etc rather than
the *pointer* itself, preferring to use "lsl" to get percpu data. You
could also imaging hiding things in the vector state somewhere if you
control your environment well enough).
Linus
^ permalink raw reply
* Re: [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Josh Triplett @ 2015-07-17 17:03 UTC (permalink / raw)
To: Ben Maurer
Cc: Mathieu Desnoyers, Paul Turner, linux-kernel@vger.kernel.org,
Andrew Hunter, Peter Zijlstra, Ingo Molnar, Steven Rostedt,
Paul E. McKenney, Linus Torvalds, Andrew Morton,
linux-api@vger.kernel.org
In-Reply-To: <5CDDBDF2D36D9F43B9F5E99003F6A0D48D616ED9@PRN-MBX02-1.TheFacebook.com>
On Fri, Jul 17, 2015 at 10:49:19AM +0000, Ben Maurer wrote:
> Mathieu Desnoyers wrote:
> > Expose a new system call allowing threads to register a userspace memory
> > area where to store the current CPU number. Scheduler migration sets the
>
> I really like that this approach makes it easier to add a per-thread interaction between userspace and the kernel in the future.
>
> >+ if (!tlap || t->thread_local_abi_len <
> >+ offsetof(struct thread_local_abi, cpu)
> >+ + sizeof(tlap->cpu))
>
> Could you save a branch here by enforcing that thread_local_abi_len = 0 if thread_local_abi = null?
"saving a branch" doesn't seem like a good reason to do that; however,
it *is* the convention across other calls: if you pass 0, the pointer
is ignored, but if you pass non-zero, the pointer must be valid or you
get -EFAULT (or an actual segfault).
- Josh Triplett
^ permalink raw reply
* Re: [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-17 16:23 UTC (permalink / raw)
To: Nikolay Borisov
Cc: Paul Turner, linux-kernel, Andrew Hunter, Peter Zijlstra,
Ingo Molnar, Ben Maurer, rostedt, Paul E. McKenney, Josh Triplett,
Linus Torvalds, Andrew Morton, linux-api
In-Reply-To: <55A8F9B2.2070008@siteground.com>
----- On Jul 17, 2015, at 8:48 AM, Nikolay Borisov n.borisov@siteground.com wrote:
> On 07/16/2015 11:00 PM, Mathieu Desnoyers wrote:
>> Expose a new system call allowing threads to register a userspace memory
>> area where to store the current CPU number. Scheduler migration sets the
>> TIF_NOTIFY_RESUME flag on the current thread. Upon return to user-space,
>> a notify-resume handler updates the current CPU value within that
>> user-space memory area.
>>
>> This getcpu cache is an alternative to the sched_getcpu() vdso which has
>> a few benefits:
>> - It is faster to do a memory read that to call a vDSO,
>> - This cache value can be read from within an inline assembly, which
>> makes it a useful building block for restartable sequences.
>>
>> This approach is inspired by Paul Turner and Andrew Hunter's work
>> on percpu atomics, which lets the kernel handle restart of critical
>> sections:
>> Ref.:
>> * https://lkml.org/lkml/2015/6/24/665
>> * https://lwn.net/Articles/650333/
>> *
>> http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
>>
>> Benchmarking sched_getcpu() vs tls cache approach. Getting the
>> current CPU number:
>>
>> - With Linux vdso: 12.7 ns
>> - With TLS-cached cpu number: 0.3 ns
>>
>> The system call can be extended by registering a larger structure in
>> the future.
>>
[...]
>> +/*
>> + * sys_thread_local_abi - setup thread-local ABI for caller thread
>> + */
>> +SYSCALL_DEFINE3(thread_local_abi, struct thread_local_abi __user *, tlap,
>> + size_t, len, int, flags)
>> +{
>> + size_t minlen;
>> +
>> + if (flags)
>> + return -EINVAL;
>> + if (current->thread_local_abi && tlap)
>> + return -EBUSY;
>> + /* Agree on the intersection of userspace and kernel features */
>> + minlen = min_t(size_t, len, sizeof(struct thread_local_abi));
>> + current->thread_local_abi_len = minlen;
>> + current->thread_local_abi = tlap;
>> + if (!tlap)
>> + return 0;
>> + /*
>> + * Migration checks ->thread_local_abi to see if notify_resume
>> + * flag should be set. Therefore, we need to ensure that
>> + * the scheduler sees ->thread_local_abi before we update its content.
>> + */
>> + barrier(); /* Store thread_local_abi before update content */
>> + if (getcpu_cache_active(current)) {
>
> Just checking whether my understanding of the code is correct, but this
> 'if' is necessary in case we have been moved to a different CPU after
> the store of the thread_local_abi?
No, this is not correct. Currently, only the getcpu_cache feature is
implemented, but if struct thread_local_abi eventually grows with more
fields, userspace could call the kernel with a "len" argument that does not
cover some of the features. Therefore, the generic way to check whether
getcpu_cache is implemented by the current thread is to call
"getcpu_cache_active()". If it is enabled, then we need to update the
getcpu_cache content for the current thread.
The barrier() above is required because we want to store thread_local_abi
(and thread_local_abi_len) before we get the current CPU number and store
it into the getcpu_cache, because we could be migrated by the scheduler
with CONFIG_PREEMPT=y at any point between the moment we read the current
CPU number within getcpu_cache_update() and resume userspace. Having
thread_local_abi and thread_local_abi_len set before fetching the current
CPU number ensures that the scheduler will succeed its own getcpu_cache_active()
check, and will therefore raise the resume notifier flag upon migration,
which will then fix the CPU number before resuming to userspace.
Thanks,
Mathieu
>
>> + if (getcpu_cache_update(current))
>> + return -EFAULT;
>> + }
>> + return minlen;
>> +}
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-17 16:12 UTC (permalink / raw)
To: Ben Maurer
Cc: Paul Turner, linux-kernel, Andrew Hunter, Peter Zijlstra,
Ingo Molnar, rostedt, Paul E. McKenney, Josh Triplett,
Linus Torvalds, Andrew Morton, linux-api
In-Reply-To: <5CDDBDF2D36D9F43B9F5E99003F6A0D48D616ED9@PRN-MBX02-1.TheFacebook.com>
----- On Jul 17, 2015, at 6:49 AM, Ben Maurer bmaurer@fb.com wrote:
> Mathieu Desnoyers wrote:
>> Expose a new system call allowing threads to register a userspace memory
>> area where to store the current CPU number. Scheduler migration sets the
>
> I really like that this approach makes it easier to add a per-thread interaction
> between userspace and the kernel in the future.
>
>>+ if (!tlap || t->thread_local_abi_len <
>>+ offsetof(struct thread_local_abi, cpu)
>>+ + sizeof(tlap->cpu))
>
> Could you save a branch here by enforcing that thread_local_abi_len = 0 if
> thread_local_abi = null?
Yes, good idea! Will do.
Thanks!
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-17 16:03 UTC (permalink / raw)
To: Ondřej Bílka
Cc: Ben Maurer, Paul Turner, Andrew Hunter, Peter Zijlstra,
Ingo Molnar, rostedt, Paul E. McKenney, Josh Triplett,
Lai Jiangshan, Linus Torvalds, Andrew Morton, linux-api,
libc-alpha
In-Reply-To: <20150717105859.GB21915@domone>
----- On Jul 17, 2015, at 6:58 AM, Ondřej Bílka neleai@seznam.cz wrote:
[...]
>> If we invoke this per-thread registration directly in the glibc NPTL
>> implementation,
>> in start_thread, do you think it would fit your requirements ?
>>
> A generic solution would be adding eager initialization of thread_local
> variables which would fix more performance problems.
Concretely, what code would be in charge of this initialization ?
What changes does "eager initialization" imply from the current
implementation ?
> Second would be write patch to libc adding function
> pthread_create_add_hook_np to register function that would be ran after
> each thread creation.
Do you suggest that this callback should be registered once per thread,
or somehow attached to pthread_attr_t ? As maintainer of lttng-ust,
where we need some control over our own threads, but where we want minimal
impact on the overall application, I would really prefer if we can attach
the callback to pthread_attr_t.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH 08/24] proc: pick out a function to iterate task children
From: Andrew Vagin @ 2015-07-17 15:57 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrey Vagin, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Cyrill Gorcunov,
Pavel Emelyanov, Roger Luethi, Arnd Bergmann,
Arnaldo Carvalho de Melo, David Ahern, Andy Lutomirski,
Pavel Odintsov
In-Reply-To: <20150714180235.GB8088-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Jul 14, 2015 at 08:02:35PM +0200, Oleg Nesterov wrote:
> On 07/06, Andrey Vagin wrote:
> >
> > -static struct pid *
> > -get_children_pid(struct inode *inode, struct pid *pid_prev, loff_t pos)
> > +static struct task_struct *
> > +task_next_child(struct task_struct *parent, struct task_struct *prev, unsigned int pos)
> > {
>
> I won't really argue, just a question...
>
> So this patch changes it to accept/return task_struct rather pid. Why?
> it is better to get/put "struct pid" only, not the whole task_struct.
>
> If another caller want task_struct, the necessary conversion is simple.
Another caller wants task_struct.
Currently this function receives pid and converts it into task_struct, then
gets the next child and returns its pid. So I try to avoid extra
conversion in task_diag code.
> But again, I won't argue if you think this will complicate the non-proc
> users of this helper.
>
> Oleg.
>
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Andy Lutomirski @ 2015-07-17 15:53 UTC (permalink / raw)
To: Ondřej Bílka
Cc: Mathieu Desnoyers, Ben Maurer, Paul Turner, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, rostedt, Paul E. McKenney,
Josh Triplett, Lai Jiangshan, Linus Torvalds, Andrew Morton,
linux-api, libc-alpha
In-Reply-To: <20150717102105.GA21915@domone>
On Fri, Jul 17, 2015 at 3:21 AM, Ondřej Bílka <neleai@seznam.cz> wrote:
> On Thu, Jul 16, 2015 at 12:27:10PM -0700, Andy Lutomirski wrote:
>> On Thu, Jul 16, 2015 at 11:08 AM, Mathieu Desnoyers
>> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> > ----- On Jul 14, 2015, at 5:34 AM, Ben Maurer bmaurer-b10kYP2dOMg@public.gmane.org wrote:
>> >>
>> >> That said, having the ability for the kernel to understand that TLS
>> >> implementation are laid out using the same offset on each thread seems like
>> >> something that could be valuable long term. Doing so makes it possible to build
>> >> other TLS-based features without forcing each thread to be registered.
>> >
>> > AFAIU, using a fixed hardcoded ABI between kernel and user-space might make
>> > transition from the pre-existing ABI (where this memory area is not
>> > reserved) a bit tricky without registering the area, or getting a "feature"
>> > flag, through a system call.
>> >
>> > The related question then becomes: should we issue this system call once
>> > per process, or once per thread at thread creation ? Issuing it once per
>> > thread is marginally more costly for thread creation, but seems to be
>> > easier to deal with internally within the kernel.
>> >
>> > We could however ensure that only a single system call is needed per new-coming
>> > thread, rather than one system call per feature. One way to do this would be
>> > to register an area that may contain more than just the CPU id. It could
>> > consist of an expandable structure with fixed offsets. When registered, we
>> > could pass the size of that structure as an argument to the system call, so
>> > the kernel knows which features are expected by user-space.
>>
>> If we actually bit the bullet and implemented per-cpu mappings, we
>> could have this be completely flexible because there would be no
>> format at all. Similarly, if we implemented per-cpu segments,
>> userspace would need to agree with *itself* how to arbitrate it, but
>> the kernel wouldn't need to be involved.
>>
>> With this kind of memory poking, it's definitely messier, which is unfortunate.
>>
> Could you recapitulate thread? On libc side we didn't read most of it so
> it would be appreciated.
>
> If per-cpu mappings mean that there is a single virtual page that is
> mapped to different virtual pages?
Single virtual page that's mapped to different physical pages on
different cpus. I believe that ARM has some hardware support for
this, but I'm not that familiar with ARM. x86 can fake it (at the
cost of some context switch overhead).
>
> I had in my todo list improving tls access. This would help tls
> implementations for older arms and in general architectures that dont
> store tcb in register.
>
> My proposal is modulo small constant equivalent of userspace accessing tid
> without syscall overhead, just use array of tcb's for first 32768 tids
> and do syscall only when tid exceeds that.
>
> On userspace my proposal would be use map that to fixed virtual address and store tcb in first eigth bytes. Kernel would on context switch along registers also
> save and restore these. That would make tls access cheap as it would
> need only extra load instruction versus static variable.
>
The problem is that having the kernel access userspace memory on
context switch, while doable, is a little bit unpleasant. We also
really need to get the ABI right the first time, because we don't
really get a second chance.
--Andy
^ permalink raw reply
* [PATCH nf-next v2] netfilter: nf_ct_sctp: minimal multihoming support
From: Michal Kubecek @ 2015-07-17 14:17 UTC (permalink / raw)
To: netfilter-devel-u79uwXL29TY76Z2rM5mHXA
Cc: coreteam-Cap9r6Oaw4JrovVCs/uTlw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Florian Westphal, Marcelo Ricardo Leitner
Currently nf_conntrack_proto_sctp module handles only packets between
primary addresses used to establish the connection. Any packets between
secondary addresses are classified as invalid so that usual firewall
configurations drop them. Allowing HEARTBEAT and HEARTBEAT-ACK chunks to
establish a new conntrack would allow traffic between secondary
addresses to pass through. A more sophisticated solution based on the
addresses advertised in the initial handshake (and possibly also later
dynamic address addition and removal) would be much harder to implement.
Moreover, in general we cannot assume to always see the initial
handshake as it can be routed through a different path.
The patch adds two new conntrack states:
SCTP_CONNTRACK_HEARTBEAT_SENT - a HEARTBEAT chunk seen but not acked
SCTP_CONNTRACK_HEARTBEAT_ACKED - a HEARTBEAT acked by HEARTBEAT-ACK
State transition rules:
- HEARTBEAT_SENT responds to usual chunks the same way as NONE (so that
the behaviour changes as little as possible)
- HEARTBEAT_ACKED responds to usual chunks the same way as ESTABLISHED
does, except the resulting state is HEARTBEAT_ACKED rather than
ESTABLISHED
- previously existing states except NONE are preserved when HEARTBEAT or
HEARTBEAT-ACK is seen
- NONE (in the initial direction) changes to HEARTBEAT_SENT on HEARTBEAT
and to CLOSED on HEARTBEAT-ACK
- HEARTBEAT_SENT changes to HEARTBEAT_ACKED on HEARTBEAT-ACK in the
reply direction
- HEARTBEAT_SENT and HEARTBEAT_ACKED are preserved on HEARTBEAT and
HEARTBEAT-ACK otherwise
Normally, vtag is set from the INIT chunk for the reply direction and
from the INIT-ACK chunk for the originating direction (i.e. each of
these defines vtag value for the opposite direction). For secondary
conntracks, we can't rely on seeing INIT/INIT-ACK and even if we have
seen them, we would need to connect two different conntracks. Therefore
simplified logic is applied: vtag of first packet in each direction
(HEARTBEAT in the originating and HEARTBEAT-ACK in reply direction) is
saved and all following packets in that direction are compared with this
saved value. While INIT and INIT-ACK define vtag for the opposite
direction, vtags extracted from HEARTBEAT and HEARTBEAT-ACK are always
for their direction.
Default timeout values for new states are
HEARTBEAT_SENT: 30 seconds (default hb_interval)
HEARTBEAT_ACKED: 210 seconds (hb_interval * path_max_retry + max_rto)
(We cannot expect to see the shutdown sequence so that, unlike
ESTABLISHED, the HEARTBEAT_ACKED timeout shouldn't be too long.)
Signed-off-by: Michal Kubecek <mkubecek-AlSwsSmVLrQ@public.gmane.org>
---
v2:
- add new timeouts to nla policy interface
- explain vtag handling in the commit message
- for consistency, rename *_HB_* constants to *_HEARTBEAT_*
include/uapi/linux/netfilter/nf_conntrack_sctp.h | 2 +
include/uapi/linux/netfilter/nfnetlink_cttimeout.h | 2 +
net/netfilter/nf_conntrack_proto_sctp.c | 115 ++++++++++++++++-----
3 files changed, 95 insertions(+), 24 deletions(-)
diff --git a/include/uapi/linux/netfilter/nf_conntrack_sctp.h b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
index ceeefe6681b5..ed4e776e1242 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_sctp.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
@@ -13,6 +13,8 @@ enum sctp_conntrack {
SCTP_CONNTRACK_SHUTDOWN_SENT,
SCTP_CONNTRACK_SHUTDOWN_RECD,
SCTP_CONNTRACK_SHUTDOWN_ACK_SENT,
+ SCTP_CONNTRACK_HEARTBEAT_SENT,
+ SCTP_CONNTRACK_HEARTBEAT_ACKED,
SCTP_CONNTRACK_MAX
};
diff --git a/include/uapi/linux/netfilter/nfnetlink_cttimeout.h b/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
index 1ab0b97b3a1e..f2c10dc140d6 100644
--- a/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
+++ b/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
@@ -92,6 +92,8 @@ enum ctattr_timeout_sctp {
CTA_TIMEOUT_SCTP_SHUTDOWN_SENT,
CTA_TIMEOUT_SCTP_SHUTDOWN_RECD,
CTA_TIMEOUT_SCTP_SHUTDOWN_ACK_SENT,
+ CTA_TIMEOUT_SCTP_HEARTBEAT_SENT,
+ CTA_TIMEOUT_SCTP_HEARTBEAT_ACKED,
__CTA_TIMEOUT_SCTP_MAX
};
#define CTA_TIMEOUT_SCTP_MAX (__CTA_TIMEOUT_SCTP_MAX - 1)
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index b45da90fad32..1aac57e45319 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -42,6 +42,8 @@ static const char *const sctp_conntrack_names[] = {
"SHUTDOWN_SENT",
"SHUTDOWN_RECD",
"SHUTDOWN_ACK_SENT",
+ "HEARTBEAT_SENT",
+ "HEARTBEAT_ACKED",
};
#define SECS * HZ
@@ -57,6 +59,8 @@ static unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] __read_mostly = {
[SCTP_CONNTRACK_SHUTDOWN_SENT] = 300 SECS / 1000,
[SCTP_CONNTRACK_SHUTDOWN_RECD] = 300 SECS / 1000,
[SCTP_CONNTRACK_SHUTDOWN_ACK_SENT] = 3 SECS,
+ [SCTP_CONNTRACK_HEARTBEAT_SENT] = 30 SECS,
+ [SCTP_CONNTRACK_HEARTBEAT_ACKED] = 210 SECS,
};
#define sNO SCTP_CONNTRACK_NONE
@@ -67,6 +71,8 @@ static unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] __read_mostly = {
#define sSS SCTP_CONNTRACK_SHUTDOWN_SENT
#define sSR SCTP_CONNTRACK_SHUTDOWN_RECD
#define sSA SCTP_CONNTRACK_SHUTDOWN_ACK_SENT
+#define sHS SCTP_CONNTRACK_HEARTBEAT_SENT
+#define sHA SCTP_CONNTRACK_HEARTBEAT_ACKED
#define sIV SCTP_CONNTRACK_MAX
/*
@@ -88,6 +94,10 @@ SHUTDOWN_ACK_SENT - We have seen a SHUTDOWN_ACK chunk in the direction opposite
to that of the SHUTDOWN chunk.
CLOSED - We have seen a SHUTDOWN_COMPLETE chunk in the direction of
the SHUTDOWN chunk. Connection is closed.
+HEARTBEAT_SENT - We have seen a HEARTBEAT in a new flow.
+HEARTBEAT_ACKED - We have seen a HEARTBEAT-ACK in the direction opposite to
+ that of the HEARTBEAT chunk. Secondary connection is
+ established.
*/
/* TODO
@@ -97,36 +107,40 @@ CLOSED - We have seen a SHUTDOWN_COMPLETE chunk in the direction of
- Check the error type in the reply dir before transitioning from
cookie echoed to closed.
- Sec 5.2.4 of RFC 2960
- - Multi Homing support.
+ - Full Multi Homing support.
*/
/* SCTP conntrack state transitions */
-static const u8 sctp_conntracks[2][9][SCTP_CONNTRACK_MAX] = {
+static const u8 sctp_conntracks[2][11][SCTP_CONNTRACK_MAX] = {
{
/* ORIGINAL */
-/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA */
-/* init */ {sCW, sCW, sCW, sCE, sES, sSS, sSR, sSA},
-/* init_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},
-/* abort */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
-/* shutdown */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA},
-/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA},
-/* error */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't have Stale cookie*/
-/* cookie_echo */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA},/* 5.2.4 - Big TODO */
-/* cookie_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't come in orig dir */
-/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL}
+/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
+/* init */ {sCW, sCW, sCW, sCE, sES, sSS, sSR, sSA, sCW, sHA},
+/* init_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},
+/* abort */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
+/* shutdown */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA, sCL, sSS},
+/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA, sSA, sHA},
+/* error */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't have Stale cookie*/
+/* cookie_echo */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* 5.2.4 - Big TODO */
+/* cookie_ack */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't come in orig dir */
+/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL, sCL, sHA},
+/* heartbeat */ {sHS, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
+/* heartbeat_ack*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA}
},
{
/* REPLY */
-/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA */
-/* init */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* INIT in sCL Big TODO */
-/* init_ack */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},
-/* abort */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
-/* shutdown */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA},
-/* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA},
-/* error */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA},
-/* cookie_echo */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA},/* Can't come in reply dir */
-/* cookie_ack */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA},
-/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL}
+/* sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
+/* init */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* INIT in sCL Big TODO */
+/* init_ack */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},
+/* abort */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sIV, sCL},
+/* shutdown */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA, sIV, sSR},
+/* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA, sIV, sHA},
+/* error */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA, sIV, sHA},
+/* cookie_echo */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* Can't come in reply dir */
+/* cookie_ack */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA, sIV, sHA},
+/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL, sIV, sHA},
+/* heartbeat */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
+/* heartbeat_ack*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHA, sHA}
}
};
@@ -278,9 +292,16 @@ static int sctp_new_state(enum ip_conntrack_dir dir,
pr_debug("SCTP_CID_SHUTDOWN_COMPLETE\n");
i = 8;
break;
+ case SCTP_CID_HEARTBEAT:
+ pr_debug("SCTP_CID_HEARTBEAT");
+ i = 9;
+ break;
+ case SCTP_CID_HEARTBEAT_ACK:
+ pr_debug("SCTP_CID_HEARTBEAT_ACK");
+ i = 10;
+ break;
default:
- /* Other chunks like DATA, SACK, HEARTBEAT and
- its ACK do not cause a change in state */
+ /* Other chunks like DATA or SACK do not change the state */
pr_debug("Unknown chunk type, Will stay in %s\n",
sctp_conntrack_names[cur_state]);
return cur_state;
@@ -329,6 +350,8 @@ static int sctp_packet(struct nf_conn *ct,
!test_bit(SCTP_CID_COOKIE_ECHO, map) &&
!test_bit(SCTP_CID_ABORT, map) &&
!test_bit(SCTP_CID_SHUTDOWN_ACK, map) &&
+ !test_bit(SCTP_CID_HEARTBEAT, map) &&
+ !test_bit(SCTP_CID_HEARTBEAT_ACK, map) &&
sh->vtag != ct->proto.sctp.vtag[dir]) {
pr_debug("Verification tag check failed\n");
goto out;
@@ -357,6 +380,16 @@ static int sctp_packet(struct nf_conn *ct,
/* Sec 8.5.1 (D) */
if (sh->vtag != ct->proto.sctp.vtag[dir])
goto out_unlock;
+ } else if (sch->type == SCTP_CID_HEARTBEAT ||
+ sch->type == SCTP_CID_HEARTBEAT_ACK) {
+ if (ct->proto.sctp.vtag[dir] == 0) {
+ pr_debug("Setting vtag %x for dir %d\n",
+ sh->vtag, dir);
+ ct->proto.sctp.vtag[dir] = sh->vtag;
+ } else if (sh->vtag != ct->proto.sctp.vtag[dir]) {
+ pr_debug("Verification tag check failed\n");
+ goto out_unlock;
+ }
}
old_state = ct->proto.sctp.state;
@@ -466,6 +499,10 @@ static bool sctp_new(struct nf_conn *ct, const struct sk_buff *skb,
/* Sec 8.5.1 (A) */
return false;
}
+ } else if (sch->type == SCTP_CID_HEARTBEAT) {
+ pr_debug("Setting vtag %x for secondary conntrack\n",
+ sh->vtag);
+ ct->proto.sctp.vtag[IP_CT_DIR_ORIGINAL] = sh->vtag;
}
/* If it is a shutdown ack OOTB packet, we expect a return
shutdown complete, otherwise an ABORT Sec 8.4 (5) and (8) */
@@ -610,6 +647,8 @@ sctp_timeout_nla_policy[CTA_TIMEOUT_SCTP_MAX+1] = {
[CTA_TIMEOUT_SCTP_SHUTDOWN_SENT] = { .type = NLA_U32 },
[CTA_TIMEOUT_SCTP_SHUTDOWN_RECD] = { .type = NLA_U32 },
[CTA_TIMEOUT_SCTP_SHUTDOWN_ACK_SENT] = { .type = NLA_U32 },
+ [CTA_TIMEOUT_SCTP_HEARTBEAT_SENT] = { .type = NLA_U32 },
+ [CTA_TIMEOUT_SCTP_HEARTBEAT_ACKED] = { .type = NLA_U32 },
};
#endif /* CONFIG_NF_CT_NETLINK_TIMEOUT */
@@ -658,6 +697,18 @@ static struct ctl_table sctp_sysctl_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_jiffies,
},
+ {
+ .procname = "nf_conntrack_sctp_timeout_heartbeat_sent",
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_jiffies,
+ },
+ {
+ .procname = "nf_conntrack_sctp_timeout_heartbeat_acked",
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_jiffies,
+ },
{ }
};
@@ -705,6 +756,18 @@ static struct ctl_table sctp_compat_sysctl_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_jiffies,
},
+ {
+ .procname = "ip_conntrack_sctp_timeout_heartbeat_sent",
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_jiffies,
+ },
+ {
+ .procname = "ip_conntrack_sctp_timeout_heartbeat_acked",
+ .maxlen = sizeof(unsigned int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_jiffies,
+ },
{ }
};
#endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
@@ -730,6 +793,8 @@ static int sctp_kmemdup_sysctl_table(struct nf_proto_net *pn,
pn->ctl_table[4].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_SENT];
pn->ctl_table[5].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_RECD];
pn->ctl_table[6].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_ACK_SENT];
+ pn->ctl_table[7].data = &sn->timeouts[SCTP_CONNTRACK_HEARTBEAT_SENT];
+ pn->ctl_table[8].data = &sn->timeouts[SCTP_CONNTRACK_HEARTBEAT_ACKED];
#endif
return 0;
}
@@ -752,6 +817,8 @@ static int sctp_kmemdup_compat_sysctl_table(struct nf_proto_net *pn,
pn->ctl_compat_table[4].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_SENT];
pn->ctl_compat_table[5].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_RECD];
pn->ctl_compat_table[6].data = &sn->timeouts[SCTP_CONNTRACK_SHUTDOWN_ACK_SENT];
+ pn->ctl_compat_table[7].data = &sn->timeouts[SCTP_CONNTRACK_HEARTBEAT_SENT];
+ pn->ctl_compat_table[8].data = &sn->timeouts[SCTP_CONNTRACK_HEARTBEAT_ACKED];
#endif
#endif
return 0;
--
2.4.5
^ permalink raw reply related
* Re: [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Nikolay Borisov @ 2015-07-17 12:48 UTC (permalink / raw)
To: Mathieu Desnoyers, Paul Turner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, Ben Maurer, Steven Rostedt,
Paul E. McKenney, Josh Triplett, Linus Torvalds, Andrew Morton,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1437076851-14848-1-git-send-email-mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On 07/16/2015 11:00 PM, Mathieu Desnoyers wrote:
> Expose a new system call allowing threads to register a userspace memory
> area where to store the current CPU number. Scheduler migration sets the
> TIF_NOTIFY_RESUME flag on the current thread. Upon return to user-space,
> a notify-resume handler updates the current CPU value within that
> user-space memory area.
>
> This getcpu cache is an alternative to the sched_getcpu() vdso which has
> a few benefits:
> - It is faster to do a memory read that to call a vDSO,
> - This cache value can be read from within an inline assembly, which
> makes it a useful building block for restartable sequences.
>
> This approach is inspired by Paul Turner and Andrew Hunter's work
> on percpu atomics, which lets the kernel handle restart of critical
> sections:
> Ref.:
> * https://lkml.org/lkml/2015/6/24/665
> * https://lwn.net/Articles/650333/
> * http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
>
> Benchmarking sched_getcpu() vs tls cache approach. Getting the
> current CPU number:
>
> - With Linux vdso: 12.7 ns
> - With TLS-cached cpu number: 0.3 ns
>
> The system call can be extended by registering a larger structure in
> the future.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> CC: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> CC: Andrew Hunter <ahh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> CC: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> CC: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> CC: Ben Maurer <bmaurer-b10kYP2dOMg@public.gmane.org>
> CC: Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>
> CC: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> CC: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
> CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> arch/x86/kernel/signal.c | 2 +
> arch/x86/syscalls/syscall_64.tbl | 1 +
> fs/exec.c | 1 +
> include/linux/sched.h | 35 ++++++++++++++
> include/uapi/asm-generic/unistd.h | 4 +-
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/thread_local_abi.h | 37 ++++++++++++++
> init/Kconfig | 9 ++++
> kernel/Makefile | 1 +
> kernel/fork.c | 2 +
> kernel/sched/core.c | 4 ++
> kernel/sched/sched.h | 2 +
> kernel/sys_ni.c | 3 ++
> kernel/thread_local_abi.c | 90 +++++++++++++++++++++++++++++++++++
> 14 files changed, 191 insertions(+), 1 deletion(-)
> create mode 100644 include/uapi/linux/thread_local_abi.h
> create mode 100644 kernel/thread_local_abi.c
>
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index e504246..157cec0 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -750,6 +750,8 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
> if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> clear_thread_flag(TIF_NOTIFY_RESUME);
> tracehook_notify_resume(regs);
> + if (getcpu_cache_active(current))
> + getcpu_cache_handle_notify_resume(current);
> }
> if (thread_info_flags & _TIF_USER_RETURN_NOTIFY)
> fire_user_return_notifiers();
> diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
> index 8d656fb..0eb2fc2 100644
> --- a/arch/x86/syscalls/syscall_64.tbl
> +++ b/arch/x86/syscalls/syscall_64.tbl
> @@ -329,6 +329,7 @@
> 320 common kexec_file_load sys_kexec_file_load
> 321 common bpf sys_bpf
> 322 64 execveat stub_execveat
> +323 common thread_local_abi sys_thread_local_abi
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/exec.c b/fs/exec.c
> index c7f9b73..e5acf80 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1555,6 +1555,7 @@ static int do_execveat_common(int fd, struct filename *filename,
> /* execve succeeded */
> current->fs->in_exec = 0;
> current->in_execve = 0;
> + thread_local_abi_execve(current);
> acct_update_integrals(current);
> task_numa_free(current);
> free_bprm(bprm);
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index a419b65..4a3fc52 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2,6 +2,7 @@
> #define _LINUX_SCHED_H
>
> #include <uapi/linux/sched.h>
> +#include <uapi/linux/thread_local_abi.h>
>
> #include <linux/sched/prio.h>
>
> @@ -1710,6 +1711,10 @@ struct task_struct {
> #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> unsigned long task_state_change;
> #endif
> +#ifdef CONFIG_THREAD_LOCAL_ABI
> + size_t thread_local_abi_len;
> + struct thread_local_abi __user *thread_local_abi;
> +#endif
> };
>
> /* Future-safe accessor for struct task_struct's cpus_allowed. */
> @@ -3090,4 +3095,34 @@ static inline unsigned long rlimit_max(unsigned int limit)
> return task_rlimit_max(current, limit);
> }
>
> +#ifdef CONFIG_THREAD_LOCAL_ABI
> +void thread_local_abi_fork(struct task_struct *t);
> +void thread_local_abi_execve(struct task_struct *t);
> +void getcpu_cache_handle_notify_resume(struct task_struct *t);
> +static inline bool getcpu_cache_active(struct task_struct *t)
> +{
> + struct thread_local_abi __user *tlap = t->thread_local_abi;
> +
> + if (!tlap || t->thread_local_abi_len <
> + offsetof(struct thread_local_abi, cpu)
> + + sizeof(tlap->cpu))
> + return false;
> + return true;
> +}
> +#else
> +static inline void thread_local_abi_fork(struct task_struct *t)
> +{
> +}
> +static inline void thread_local_abi_execve(struct task_struct *t)
> +{
> +}
> +static inline void getcpu_cache_handle_notify_resume(struct task_struct *t)
> +{
> +}
> +static inline bool getcpu_cache_active(struct task_struct *t)
> +{
> + return false;
> +}
> +#endif
> +
> #endif
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index e016bd9..50aa984 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
> __SYSCALL(__NR_bpf, sys_bpf)
> #define __NR_execveat 281
> __SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
> +#define __NR_thread_local_abi 282
> +__SYSCALL(__NR_thread_local_abi, sys_thread_local_abi)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 282
> +#define __NR_syscalls 283
>
> /*
> * All syscalls below here should go away really,
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 68ceb97..dfd6a30 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -389,6 +389,7 @@ header-y += tcp_metrics.h
> header-y += telephony.h
> header-y += termios.h
> header-y += thermal.h
> +header-y += thread_local_abi.h
> header-y += time.h
> header-y += times.h
> header-y += timex.h
> diff --git a/include/uapi/linux/thread_local_abi.h b/include/uapi/linux/thread_local_abi.h
> new file mode 100644
> index 0000000..6487c92
> --- /dev/null
> +++ b/include/uapi/linux/thread_local_abi.h
> @@ -0,0 +1,37 @@
> +#ifndef _UAPI_LINUX_THREAD_LOCAL_ABI_H
> +#define _UAPI_LINUX_THREAD_LOCAL_ABI_H
> +
> +/*
> + * linux/thread_local_abi.h
> + *
> + * thread_local_abi system call API
> + *
> + * Copyright (c) 2015 Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/types.h>
> +
> +/* This structure is an ABI that can only be extended. */
> +struct thread_local_abi {
> + int32_t cpu;
> +};
> +
> +#endif /* _UAPI_LINUX_THREAD_LOCAL_ABI_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index f5dbc6d..c8ff5fa 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1559,6 +1559,15 @@ config PCI_QUIRKS
> bugs/quirks. Disable this only if your target machine is
> unaffected by PCI quirks.
>
> +config THREAD_LOCAL_ABI
> + bool "Enable thread-local ABI" if EXPERT
> + default y
> + help
> + Enable the thread-local ABI system call. It provides a user-space
> + cache for the current CPU number value.
> +
> + If unsure, say Y.
> +
> config EMBEDDED
> bool "Embedded system"
> option allnoconfig_y
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1408b33..cc1f3d4 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -96,6 +96,7 @@ obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
> obj-$(CONFIG_TORTURE_TEST) += torture.o
> +obj-$(CONFIG_THREAD_LOCAL_ABI) += thread_local_abi.o
>
> $(obj)/configs.o: $(obj)/config_data.h
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index cf65139..e17bcb3 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1549,6 +1549,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> cgroup_post_fork(p);
> if (clone_flags & CLONE_THREAD)
> threadgroup_change_end(current);
> + if (!(clone_flags & CLONE_THREAD))
> + thread_local_abi_fork(p);
> perf_event_fork(p);
>
> trace_task_newtask(p, clone_flags);
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 62671f5..668a502 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1823,6 +1823,10 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
>
> p->numa_group = NULL;
> #endif /* CONFIG_NUMA_BALANCING */
> +#ifdef CONFIG_THREAD_LOCAL_ABI
> + p->thread_local_abi_len = 0;
> + p->thread_local_abi = NULL;
> +#endif
> }
>
> #ifdef CONFIG_NUMA_BALANCING
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index dc0f435..bf3e346 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -921,6 +921,8 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
> {
> set_task_rq(p, cpu);
> #ifdef CONFIG_SMP
> + if (getcpu_cache_active(p))
> + set_tsk_thread_flag(p, TIF_NOTIFY_RESUME);
> /*
> * After ->cpu is set up to a new value, task_rq_lock(p, ...) can be
> * successfuly executed on another CPU. We must ensure that updates of
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 5adcb0a..cadb903 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -229,3 +229,6 @@ cond_syscall(sys_bpf);
>
> /* execveat */
> cond_syscall(sys_execveat);
> +
> +/* thread-local ABI */
> +cond_syscall(sys_thread_local_abi);
> diff --git a/kernel/thread_local_abi.c b/kernel/thread_local_abi.c
> new file mode 100644
> index 0000000..681f06e
> --- /dev/null
> +++ b/kernel/thread_local_abi.c
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright (C) 2015 Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> + *
> + * thread_local_abi system call
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/sched.h>
> +#include <linux/uaccess.h>
> +#include <linux/syscalls.h>
> +
> +static int getcpu_cache_update(struct task_struct *t)
> +{
> + if (put_user(raw_smp_processor_id(), &t->thread_local_abi->cpu)) {
> + t->thread_local_abi_len = 0;
> + t->thread_local_abi = NULL;
> + return -1;
> + }
> + return 0;
> +}
> +
> +/*
> + * This resume handler should always be executed between a migration
> + * triggered by preemption and return to user-space.
> + */
> +void getcpu_cache_handle_notify_resume(struct task_struct *t)
> +{
> + BUG_ON(!getcpu_cache_active(t));
> + if (unlikely(t->flags & PF_EXITING))
> + return;
> + if (getcpu_cache_update(t))
> + force_sig(SIGSEGV, t);
> +}
> +
> +/*
> + * If parent process has a thread-local ABI, the child inherits. Only applies
> + * when forking a process, not a thread.
> + */
> +void thread_local_abi_fork(struct task_struct *t)
> +{
> + t->thread_local_abi_len = current->thread_local_abi_len;
> + t->thread_local_abi = current->thread_local_abi;
> +}
> +
> +void thread_local_abi_execve(struct task_struct *t)
> +{
> + t->thread_local_abi_len = 0;
> + t->thread_local_abi = NULL;
> +}
> +
> +/*
> + * sys_thread_local_abi - setup thread-local ABI for caller thread
> + */
> +SYSCALL_DEFINE3(thread_local_abi, struct thread_local_abi __user *, tlap,
> + size_t, len, int, flags)
> +{
> + size_t minlen;
> +
> + if (flags)
> + return -EINVAL;
> + if (current->thread_local_abi && tlap)
> + return -EBUSY;
> + /* Agree on the intersection of userspace and kernel features */
> + minlen = min_t(size_t, len, sizeof(struct thread_local_abi));
> + current->thread_local_abi_len = minlen;
> + current->thread_local_abi = tlap;
> + if (!tlap)
> + return 0;
> + /*
> + * Migration checks ->thread_local_abi to see if notify_resume
> + * flag should be set. Therefore, we need to ensure that
> + * the scheduler sees ->thread_local_abi before we update its content.
> + */
> + barrier(); /* Store thread_local_abi before update content */
> + if (getcpu_cache_active(current)) {
Just checking whether my understanding of the code is correct, but this
'if' is necessary in case we have been moved to a different CPU after
the store of the thread_local_abi?
> + if (getcpu_cache_update(current))
> + return -EFAULT;
> + }
> + return minlen;
> +}
>
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Ondřej Bílka @ 2015-07-17 10:58 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Ben Maurer, Paul Turner, Andrew Hunter, Peter Zijlstra,
Ingo Molnar, rostedt, Paul E. McKenney, Josh Triplett,
Lai Jiangshan, Linus Torvalds, Andrew Morton, linux-api,
libc-alpha-9JcytcrH/bA+uJoB2kUjGw
In-Reply-To: <587954201.31.1436808992876.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Mon, Jul 13, 2015 at 05:36:32PM +0000, Mathieu Desnoyers wrote:
> ----- On Jul 13, 2015, at 7:17 AM, Ben Maurer bmaurer-b10kYP2dOMg@public.gmane.org wrote:
>
> > At Facebook we already use getcpu in folly, our base C++ library, to provide
> > high performance concurrency algorithms. Folly includes an abstraction called
> > AccessSpreader which helps engineers write abstractions which shard themselves
> > across different cores to prevent cache contention
> > (https://github.com/facebook/folly/blob/master/folly/detail/CacheLocality.cpp).
Could you contribute your improvements/tips to libc? If these help for
c++ mutex then it would also improve c mutex.
> > We have used this primative to create faster reader writer locks
> > (https://github.com/facebook/folly/blob/master/folly/SharedMutex.h), as well as
> > in an abstraction that powers workqueues
> > (https://github.com/facebook/folly/blob/master/folly/IndexedMemPool.h). This
> > would be a great perf improvement for these types of abstractions and probably
> > encourage us to use the idea more widely.
> >
As libc rwlocks now are slow it gets speedup from that. Main problem
with this is that lock elission will give you bigger speedups that that.
Also from description you have wrong rwlock usecase, main application is
avoid blocking, when two readers take lock for long time having one wait
would be terrible.
> > One quick comment on the approach -- it'd be really great if we had a method
> > that didn't require users to register each thread. This can often lead to
> > requiring an additional branch in critical code to check if the appropriate
> > caches have been initialized. Also, one of the most interesting potential
> > applications of the restartable sequences concept is in malloc. having a brief
> > period at the beginning of the life of a thread where malloc didn't work would
> > be pretty tricky to program around.
>
> If we invoke this per-thread registration directly in the glibc NPTL implementation,
> in start_thread, do you think it would fit your requirements ?
>
A generic solution would be adding eager initialization of thread_local
variables which would fix more performance problems.
Second would be write patch to libc adding function
pthread_create_add_hook_np to register function that would be ran after
each thread cretion.
^ permalink raw reply
* RE: [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Ben Maurer @ 2015-07-17 10:49 UTC (permalink / raw)
To: Mathieu Desnoyers, Paul Turner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andrew Hunter, Peter Zijlstra, Ingo Molnar, Steven Rostedt,
Paul E. McKenney, Josh Triplett, Linus Torvalds, Andrew Morton,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1437076851-14848-1-git-send-email-mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
Mathieu Desnoyers wrote:
> Expose a new system call allowing threads to register a userspace memory
> area where to store the current CPU number. Scheduler migration sets the
I really like that this approach makes it easier to add a per-thread interaction between userspace and the kernel in the future.
>+ if (!tlap || t->thread_local_abi_len <
>+ offsetof(struct thread_local_abi, cpu)
>+ + sizeof(tlap->cpu))
Could you save a branch here by enforcing that thread_local_abi_len = 0 if thread_local_abi = null?
-b--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Ondřej Bílka @ 2015-07-17 10:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mathieu Desnoyers, Ben Maurer, Paul Turner, Andrew Hunter,
Peter Zijlstra, Ingo Molnar, rostedt, Paul E. McKenney,
Josh Triplett, Lai Jiangshan, Linus Torvalds, Andrew Morton,
linux-api, libc-alpha
In-Reply-To: <CALCETrWEKE=mow3vVh7C4r8CuGy_d5VOEz7KkpijuR5cpBfFtg@mail.gmail.com>
On Thu, Jul 16, 2015 at 12:27:10PM -0700, Andy Lutomirski wrote:
> On Thu, Jul 16, 2015 at 11:08 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> > ----- On Jul 14, 2015, at 5:34 AM, Ben Maurer bmaurer@fb.com wrote:
> >>
> >> That said, having the ability for the kernel to understand that TLS
> >> implementation are laid out using the same offset on each thread seems like
> >> something that could be valuable long term. Doing so makes it possible to build
> >> other TLS-based features without forcing each thread to be registered.
> >
> > AFAIU, using a fixed hardcoded ABI between kernel and user-space might make
> > transition from the pre-existing ABI (where this memory area is not
> > reserved) a bit tricky without registering the area, or getting a "feature"
> > flag, through a system call.
> >
> > The related question then becomes: should we issue this system call once
> > per process, or once per thread at thread creation ? Issuing it once per
> > thread is marginally more costly for thread creation, but seems to be
> > easier to deal with internally within the kernel.
> >
> > We could however ensure that only a single system call is needed per new-coming
> > thread, rather than one system call per feature. One way to do this would be
> > to register an area that may contain more than just the CPU id. It could
> > consist of an expandable structure with fixed offsets. When registered, we
> > could pass the size of that structure as an argument to the system call, so
> > the kernel knows which features are expected by user-space.
>
> If we actually bit the bullet and implemented per-cpu mappings, we
> could have this be completely flexible because there would be no
> format at all. Similarly, if we implemented per-cpu segments,
> userspace would need to agree with *itself* how to arbitrate it, but
> the kernel wouldn't need to be involved.
>
> With this kind of memory poking, it's definitely messier, which is unfortunate.
>
Could you recapitulate thread? On libc side we didn't read most of it so
it would be appreciated.
If per-cpu mappings mean that there is a single virtual page that is
mapped to different virtual pages?
I had in my todo list improving tls access. This would help tls
implementations for older arms and in general architectures that dont
store tcb in register.
My proposal is modulo small constant equivalent of userspace accessing tid
without syscall overhead, just use array of tcb's for first 32768 tids
and do syscall only when tid exceeds that.
On userspace my proposal would be use map that to fixed virtual address and store tcb in first eigth bytes. Kernel would on context switch along registers also
save and restore these. That would make tls access cheap as it would
need only extra load instruction versus static variable.
^ permalink raw reply
* Re: [PATCH -mm v8 4/7] proc: add kpagecgroup file
From: Vladimir Davydov @ 2015-07-17 9:27 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJu=L5_AUFv=Bh2WiWwOsMx41z_X0cAum_WkNikSE4Bo0r+wfQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Jul 16, 2015 at 12:04:59PM -0700, Andres Lagar-Cavilla wrote:
> On Thu, Jul 16, 2015 at 2:28 AM, Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> wrote:
>
> > On Wed, Jul 15, 2015 at 12:03:18PM -0700, Andres Lagar-Cavilla wrote:
> > > For both /proc/kpage* interfaces you add (and more critically for the
> > > rmap-causing one, kpageidle):
> > >
> > > It's a good idea to do cond_sched(). Whether after each pfn, each Nth
> > > pfn, each put_user, I leave to you, but a reasonable cadence is
> > > needed, because user-space can call this on the entire physical
> > > address space, and that's a lot of work to do without re-scheduling.
> >
> > I really don't think it's necessary. These files can only be
> > read/written by the root, who has plenty ways to kill the system anyway.
> > The program that is allowed to read/write these files must be conscious
> > and do it in batches of reasonable size. AFAICS the same reasoning
> > already lays behind /proc/kpagecount and /proc/kpageflag, which also do
> > not thrust the "right" batch size on their readers.
> >
>
> Beg to disagree. You're conflating intended use with system health. A
> cond_sched() is a one-liner.
I would still prefer not to clutter the code with cond_resched's, but I
don't think it's a matter worth arguing upon, so I'll prepare a patch
that makes all /proc/kapge* files issue cond_resched periodically and
leave it up to Andrew to decide if it should be applied or not.
Thanks,
Vladimir
^ permalink raw reply
* Re: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add
From: Naoya Horiguchi @ 2015-07-17 9:02 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-api@vger.kernel.org, Dave Hansen, David Rientjes,
Hugh Dickins, Davidlohr Bueso, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Andrew Morton, Michal Hocko
In-Reply-To: <1436761268-6397-2-git-send-email-mike.kravetz@oracle.com>
On Sun, Jul 12, 2015 at 09:20:59PM -0700, Mike Kravetz wrote:
> fallocate hole punch will want to remove a specific range of
> pages. When pages are removed, their associated entries in
> the region/reserve map will also be removed. This will break
> an assumption in the region_chg/region_add calling sequence.
> If a new region descriptor must be allocated, it is done as
> part of the region_chg processing. In this way, region_add
> can not fail because it does not need to attempt an allocation.
>
> To prepare for fallocate hole punch, create a "cache" of
> descriptors that can be used by region_add if necessary.
> region_chg will ensure there are sufficient entries in the
> cache. It will be necessary to track the number of in progress
> add operations to know a sufficient number of descriptors
> reside in the cache. A new routine region_abort is added to
> adjust this in progress count when add operations are aborted.
> vma_abort_reservation is also added for callers creating
> reservations with vma_needs_reservation/vma_commit_reservation.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> include/linux/hugetlb.h | 3 +
> mm/hugetlb.c | 169 ++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 153 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index d891f94..667cf44 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -35,6 +35,9 @@ struct resv_map {
> struct kref refs;
> spinlock_t lock;
> struct list_head regions;
> + long adds_in_progress;
> + struct list_head rgn_cache;
> + long rgn_cache_count;
> };
> extern struct resv_map *resv_map_alloc(void);
> void resv_map_release(struct kref *ref);
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a8c3087..241d16d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -240,11 +240,14 @@ struct file_region {
>
> /*
> * Add the huge page range represented by [f, t) to the reserve
> - * map. Existing regions will be expanded to accommodate the
> - * specified range. We know only existing regions need to be
> - * expanded, because region_add is only called after region_chg
> - * with the same range. If a new file_region structure must
> - * be allocated, it is done in region_chg.
> + * map. In the normal case, existing regions will be expanded
> + * to accommodate the specified range. Sufficient regions should
> + * exist for expansion due to the previous call to region_chg
> + * with the same range. However, it is possible that region_del
> + * could have been called after region_chg and modifed the map
> + * in such a way that no region exists to be expanded. In this
> + * case, pull a region descriptor from the cache associated with
> + * the map and use that for the new range.
> *
> * Return the number of new huge pages added to the map. This
> * number is greater than or equal to zero.
> @@ -261,6 +264,27 @@ static long region_add(struct resv_map *resv, long f, long t)
> if (f <= rg->to)
> break;
>
> + if (&rg->link == head || t < rg->from) {
> + /*
> + * No region exists which can be expanded to include the
> + * specified range. Pull a region descriptor from the
> + * cache, and use it for this range.
> + */
This comment mentions this if-block, not the VM_BUG_ON below, so it had
better be put the above if-line.
> + VM_BUG_ON(!resv->rgn_cache_count);
resv->rgn_cache_count <= 0 might be safer.
...
> @@ -3236,11 +3360,14 @@ retry:
> * any allocations necessary to record that reservation occur outside
> * the spinlock.
> */
> - if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED))
> + if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
> if (vma_needs_reservation(h, vma, address) < 0) {
> ret = VM_FAULT_OOM;
> goto backout_unlocked;
> }
> + /* Just decrements count, does not deallocate */
> + vma_abort_reservation(h, vma, address);
> + }
This is not "abort reservation" operation, but you use "abort reservation"
routine, which might confusing and makes future maintenance hard. I think
this should be done in a simplified variant of vma_commit_reservation()
(maybe just an alias of your vma_abort_reservation()) or fast path in
vma_commit_reservation().
Thanks,
Naoya Horiguchi
>
> ptl = huge_pte_lockptr(h, mm, ptep);
> spin_lock(ptl);
> @@ -3387,6 +3514,8 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> ret = VM_FAULT_OOM;
> goto out_mutex;
> }
> + /* Just decrements count, does not deallocate */
> + vma_abort_reservation(h, vma, address);
>
> if (!(vma->vm_flags & VM_MAYSHARE))
> pagecache_page = hugetlbfs_pagecache_page(h,
> @@ -3726,6 +3855,8 @@ int hugetlb_reserve_pages(struct inode *inode,
> }
> return 0;
> out_err:
> + if (!vma || vma->vm_flags & VM_MAYSHARE)
> + region_abort(resv_map, from, to);
> if (vma && is_vma_resv_set(vma, HPAGE_RESV_OWNER))
> kref_put(&resv_map->refs, resv_map_release);
> return ret;
> --
> 2.1.0
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 00/10] hugetlbfs: add fallocate support
From: Naoya Horiguchi @ 2015-07-17 9:01 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-api@vger.kernel.org, Dave Hansen, David Rientjes,
Hugh Dickins, Davidlohr Bueso, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Andrew Morton, Michal Hocko
In-Reply-To: <1436761268-6397-1-git-send-email-mike.kravetz@oracle.com>
On Sun, Jul 12, 2015 at 09:20:58PM -0700, Mike Kravetz wrote:
> Only change in this revision is the fix to the self-discovered
> issue in region_chg(). Functional and stress tests passing.
> Full changelog below.
>
> As suggested during the RFC process, tests have been proposed to
> libhugetlbfs as described at:
> http://librelist.com/browser//libhugetlbfs/2015/6/25/patch-tests-add-tests-for-fallocate-system-call/
> fallocate(2) man page modifications are also necessary to specify
> that fallocate for hugetlbfs only operates on whole pages. This
> change will be submitted once the code has stabilized and been
> proposed for merging.
>
> hugetlbfs is used today by applications that want a high degree of
> control over huge page usage. Often, large hugetlbfs files are used
> to map a large number huge pages into the application processes.
> The applications know when page ranges within these large files will
> no longer be used, and ideally would like to release them back to
> the subpool or global pools for other uses. The fallocate() system
> call provides an interface for preallocation and hole punching within
> files. This patch set adds fallocate functionality to hugetlbfs.
>
> v3:
> Fixed issue with region_chg to recheck if there are sufficient
> entries in the cache after acquiring lock.
> v2:
> Fixed leak in resv_map_release discovered by Hillf Danton.
> Used LONG_MAX as indicator of truncate function for region_del.
> v1:
> Add a cache of region descriptors to the resv_map for use by
> region_add in case hole punch deletes entries necessary for
> a successful operation.
> RFC v4:
> Removed alloc_huge_page/hugetlb_reserve_pages race patches as already
> in mmotm
> Moved hugetlb_fix_reserve_counts in series as suggested by Naoya Horiguchi
> Inline'ed hugetlb_fault_mutex routines as suggested by Davidlohr Bueso and
> existing code changed to use new interfaces as suggested by Naoya
> fallocate preallocation code cleaned up and made simpler
> Modified alloc_huge_page to handle special case where allocation is
> for a hole punched area with spool reserves
> RFC v3:
> Folded in patch for alloc_huge_page/hugetlb_reserve_pages race
> in current code
> fallocate allocation and hole punch is synchronized with page
> faults via existing mutex table
> hole punch uses existing hugetlb_vmtruncate_list instead of more
> generic unmap_mapping_range for unmapping
> Error handling for the case when region_del() fauils
> RFC v2:
> Addressed alignment and error handling issues noticed by Hillf Danton
> New region_del() routine for region tracking/resv_map of ranges
> Fixed several issues found during more extensive testing
> Error handling in region_del() when kmalloc() fails stills needs
> to be addressed
> madvise remove support remains
>
> Mike Kravetz (10):
> mm/hugetlb: add cache of descriptors to resv_map for region_add
> mm/hugetlb: add region_del() to delete a specific range of entries
> mm/hugetlb: expose hugetlb fault mutex for use by fallocate
> hugetlbfs: hugetlb_vmtruncate_list() needs to take a range to delete
> hugetlbfs: truncate_hugepages() takes a range of pages
> mm/hugetlb: vma_has_reserves() needs to handle fallocate hole punch
> mm/hugetlb: alloc_huge_page handle areas hole punched by fallocate
> hugetlbfs: New huge_add_to_page_cache helper routine
> hugetlbfs: add hugetlbfs_fallocate()
> mm: madvise allow remove operation for hugetlbfs
>
> fs/hugetlbfs/inode.c | 281 +++++++++++++++++++++++++++++---
> include/linux/hugetlb.h | 17 +-
> mm/hugetlb.c | 423 ++++++++++++++++++++++++++++++++++++++----------
> mm/madvise.c | 2 +-
> 4 files changed, 619 insertions(+), 104 deletions(-)
I've read through this series and it looks good to me.
I'll send a comment later for 1/10, but it's kind of nitpicks.
Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Thanks,
Naoya Horiguchi
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] toshiba_acpi: Add /dev/toshiba_acpi device
From: Darren Hart @ 2015-07-17 6:42 UTC (permalink / raw)
To: Azael Avalos
Cc: Jonathan Corbet, Jonathan Buzzard, linux-doc, linux-kernel,
platform-driver-x86, linux-api
In-Reply-To: <1437089564-4242-1-git-send-email-coproscefalo@gmail.com>
On Thu, Jul 16, 2015 at 05:32:44PM -0600, Azael Avalos wrote:
> There were previous attempts to "merge" the toshiba SMM module to the
> toshiba_acpi one, they were trying to imitate what the old toshiba
> module does, however, some models (TOS1900 devices) come with a
> "crippled" implementation and do not provide all the "features" a
> "genuine" Toshiba BIOS does.
>
> This patch adds a new device called toshiba_acpi, which aim is to
> enable userspace to access the SMM on Toshiba laptops via ACPI calls.
>
> Creating a new convenience _IOWR command to access the SCI functions
> by opening/closing the SCI internally to avoid buggy BIOS, while at
> the same time providing backwards compatibility.
>
> Older programs who wish to access the SMM on newer models can do it
> without much code change, as the toshiba.h header was modified to
> reflect these changes as well as adds all the toshiba_acpi paths
> and commands.
>
> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
This and the other 2 are queued to for-review. They will remain there for a few
days to give others a chance to review. After that, I will provide a thorough
review and move them into for-next for 4.3
They do pass all my preliminary tests, checkpatch, build, and ninja-check.
As this is the first series that I've received since enabling a lot of new
automation, I wanted to say thanks for clean series that is ready review. Nice.
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply
* [PATCH] toshiba_acpi: Add /dev/toshiba_acpi device
From: Azael Avalos @ 2015-07-16 23:32 UTC (permalink / raw)
To: Jonathan Corbet, Darren Hart, Jonathan Buzzard,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Azael Avalos
There were previous attempts to "merge" the toshiba SMM module to the
toshiba_acpi one, they were trying to imitate what the old toshiba
module does, however, some models (TOS1900 devices) come with a
"crippled" implementation and do not provide all the "features" a
"genuine" Toshiba BIOS does.
This patch adds a new device called toshiba_acpi, which aim is to
enable userspace to access the SMM on Toshiba laptops via ACPI calls.
Creating a new convenience _IOWR command to access the SCI functions
by opening/closing the SCI internally to avoid buggy BIOS, while at
the same time providing backwards compatibility.
Older programs who wish to access the SMM on newer models can do it
without much code change, as the toshiba.h header was modified to
reflect these changes as well as adds all the toshiba_acpi paths
and commands.
Signed-off-by: Azael Avalos <coproscefalo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/platform/x86/toshiba_acpi.c | 91 ++++++++++++++++++++++++++++++++++++
include/uapi/linux/toshiba.h | 32 +++++++++++--
3 files changed, 121 insertions(+), 4 deletions(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 51f4221..18babaf 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -263,7 +263,7 @@ Code Seq#(hex) Include File Comments
's' all linux/cdk.h
't' 00-7F linux/ppp-ioctl.h
't' 80-8F linux/isdn_ppp.h
-'t' 90 linux/toshiba.h
+'t' 90-91 linux/toshiba.h toshiba and toshiba_acpi SMM
'u' 00-1F linux/smb_fs.h gone
'u' 20-3F linux/uvcvideo.h USB video class host driver
'v' 00-1F linux/ext2_fs.h conflict!
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
index 58d6119..dc4255b 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -50,6 +50,8 @@
#include <linux/acpi.h>
#include <linux/dmi.h>
#include <linux/uaccess.h>
+#include <linux/miscdevice.h>
+#include <linux/toshiba.h>
#include <acpi/video.h>
MODULE_AUTHOR("John Belmonte");
@@ -170,6 +172,7 @@ struct toshiba_acpi_dev {
struct led_classdev led_dev;
struct led_classdev kbd_led;
struct led_classdev eco_led;
+ struct miscdevice miscdev;
int force_fan;
int last_key_event;
@@ -2262,6 +2265,81 @@ static struct attribute_group toshiba_attr_group = {
};
/*
+ * Misc device
+ */
+static int toshiba_acpi_smm_bridge(SMMRegisters *regs)
+{
+ u32 in[TCI_WORDS] = { regs->eax, regs->ebx, regs->ecx,
+ regs->edx, regs->esi, regs->edi };
+ u32 out[TCI_WORDS];
+ acpi_status status;
+
+ status = tci_raw(toshiba_acpi, in, out);
+ if (ACPI_FAILURE(status)) {
+ pr_err("ACPI call to query TCI registers failed\n");
+ return -EIO;
+ }
+
+ /* Fillout the SMM struct with the TCI call results */
+ regs->eax = out[0];
+ regs->ebx = out[1];
+ regs->ecx = out[2];
+ regs->edx = out[3];
+ regs->esi = out[4];
+ regs->edi = out[5];
+
+ return 0;
+}
+
+static long toshiba_acpi_ioctl(struct file *fp, unsigned int cmd,
+ unsigned long arg)
+{
+ SMMRegisters __user *argp = (SMMRegisters __user *)arg;
+ SMMRegisters regs;
+ int ret;
+
+ if (!argp)
+ return -EINVAL;
+
+ switch (cmd) {
+ case TOSH_SMM:
+ if (copy_from_user(®s, argp, sizeof(SMMRegisters)))
+ return -EFAULT;
+ ret = toshiba_acpi_smm_bridge(®s);
+ if (ret)
+ return ret;
+ if (copy_to_user(argp, ®s, sizeof(SMMRegisters)))
+ return -EFAULT;
+ break;
+ case TOSHIBA_ACPI_SCI:
+ if (copy_from_user(®s, argp, sizeof(SMMRegisters)))
+ return -EFAULT;
+ /* Ensure we are being called with a SCI_{GET, SET} register */
+ if (regs.eax != SCI_GET && regs.eax != SCI_SET)
+ return -EINVAL;
+ if (!sci_open(toshiba_acpi))
+ return -EIO;
+ ret = toshiba_acpi_smm_bridge(®s);
+ sci_close(toshiba_acpi);
+ if (ret)
+ return ret;
+ if (copy_to_user(argp, ®s, sizeof(SMMRegisters)))
+ return -EFAULT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct file_operations toshiba_acpi_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = toshiba_acpi_ioctl,
+ .llseek = noop_llseek,
+};
+
+/*
* Hotkeys
*/
static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
@@ -2559,6 +2637,8 @@ static int toshiba_acpi_remove(struct acpi_device *acpi_dev)
{
struct toshiba_acpi_dev *dev = acpi_driver_data(acpi_dev);
+ misc_deregister(&dev->miscdev);
+
remove_toshiba_proc_entries(dev);
if (dev->sysfs_created)
@@ -2630,6 +2710,17 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
return -ENOMEM;
dev->acpi_dev = acpi_dev;
dev->method_hci = hci_method;
+ dev->miscdev.minor = MISC_DYNAMIC_MINOR;
+ dev->miscdev.name = "toshiba_acpi";
+ dev->miscdev.fops = &toshiba_acpi_fops;
+
+ ret = misc_register(&dev->miscdev);
+ if (ret) {
+ pr_err("Failed to register miscdevice\n");
+ kfree(dev);
+ return ret;
+ }
+
acpi_dev->driver_data = dev;
dev_set_drvdata(&acpi_dev->dev, dev);
diff --git a/include/uapi/linux/toshiba.h b/include/uapi/linux/toshiba.h
index e9bef5b..c58bf4b 100644
--- a/include/uapi/linux/toshiba.h
+++ b/include/uapi/linux/toshiba.h
@@ -1,6 +1,7 @@
/* toshiba.h -- Linux driver for accessing the SMM on Toshiba laptops
*
* Copyright (c) 1996-2000 Jonathan A. Buzzard (jonathan-kE54VaY6i3ADXYZnReoRVg@public.gmane.org)
+ * Copyright (c) 2015 Azael Avalos <coproscefalo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
*
* Thanks to Juergen Heinzl <juergen-f29ozuUdGaZwQVkoCiQKlSp2UmYkHbXO@public.gmane.org> for the pointers
* on making sure the structure is aligned and packed.
@@ -20,9 +21,18 @@
#ifndef _UAPI_LINUX_TOSHIBA_H
#define _UAPI_LINUX_TOSHIBA_H
-#define TOSH_PROC "/proc/toshiba"
-#define TOSH_DEVICE "/dev/toshiba"
-#define TOSH_SMM _IOWR('t', 0x90, int) /* broken: meant 24 bytes */
+/*
+ * Toshiba modules paths
+ */
+
+#define TOSH_PROC "/proc/toshiba"
+#define TOSH_DEVICE "/dev/toshiba"
+#define TOSHIBA_ACPI_PROC "/proc/acpi/toshiba"
+#define TOSHIBA_ACPI_DEVICE "/dev/toshiba_acpi"
+
+/*
+ * Toshiba SMM structure
+ */
typedef struct {
unsigned int eax;
@@ -33,5 +43,21 @@ typedef struct {
unsigned int edi __attribute__ ((packed));
} SMMRegisters;
+/*
+ * IOCTLs (0x90 - 0x91)
+ */
+
+#define TOSH_SMM _IOWR('t', 0x90, SMMRegisters)
+/*
+ * Convenience toshiba_acpi command.
+ *
+ * The System Configuration Interface (SCI) is opened/closed internally
+ * to avoid userspace of buggy BIOSes.
+ *
+ * The toshiba_acpi module checks whether the eax register is set with
+ * SCI_GET (0xf300) or SCI_SET (0xf400), returning -EINVAL if not.
+ */
+#define TOSHIBA_ACPI_SCI _IOWR('t', 0x91, SMMRegisters)
+
#endif /* _UAPI_LINUX_TOSHIBA_H */
--
2.4.2
^ permalink raw reply related
* [RFC PATCH] thread_local_abi system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-16 20:00 UTC (permalink / raw)
To: Paul Turner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mathieu Desnoyers,
Andrew Hunter, Peter Zijlstra, Ingo Molnar, Ben Maurer,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Linus Torvalds,
Andrew Morton, linux-api-u79uwXL29TY76Z2rM5mHXA
Expose a new system call allowing threads to register a userspace memory
area where to store the current CPU number. Scheduler migration sets the
TIF_NOTIFY_RESUME flag on the current thread. Upon return to user-space,
a notify-resume handler updates the current CPU value within that
user-space memory area.
This getcpu cache is an alternative to the sched_getcpu() vdso which has
a few benefits:
- It is faster to do a memory read that to call a vDSO,
- This cache value can be read from within an inline assembly, which
makes it a useful building block for restartable sequences.
This approach is inspired by Paul Turner and Andrew Hunter's work
on percpu atomics, which lets the kernel handle restart of critical
sections:
Ref.:
* https://lkml.org/lkml/2015/6/24/665
* https://lwn.net/Articles/650333/
* http://www.linuxplumbersconf.org/2013/ocw/system/presentations/1695/original/LPC%20-%20PerCpu%20Atomics.pdf
Benchmarking sched_getcpu() vs tls cache approach. Getting the
current CPU number:
- With Linux vdso: 12.7 ns
- With TLS-cached cpu number: 0.3 ns
The system call can be extended by registering a larger structure in
the future.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
CC: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
CC: Andrew Hunter <ahh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
CC: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
CC: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
CC: Ben Maurer <bmaurer-b10kYP2dOMg@public.gmane.org>
CC: Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>
CC: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
CC: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
CC: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
arch/x86/kernel/signal.c | 2 +
arch/x86/syscalls/syscall_64.tbl | 1 +
fs/exec.c | 1 +
include/linux/sched.h | 35 ++++++++++++++
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/thread_local_abi.h | 37 ++++++++++++++
init/Kconfig | 9 ++++
kernel/Makefile | 1 +
kernel/fork.c | 2 +
kernel/sched/core.c | 4 ++
kernel/sched/sched.h | 2 +
kernel/sys_ni.c | 3 ++
kernel/thread_local_abi.c | 90 +++++++++++++++++++++++++++++++++++
14 files changed, 191 insertions(+), 1 deletion(-)
create mode 100644 include/uapi/linux/thread_local_abi.h
create mode 100644 kernel/thread_local_abi.c
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index e504246..157cec0 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -750,6 +750,8 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
if (thread_info_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
+ if (getcpu_cache_active(current))
+ getcpu_cache_handle_notify_resume(current);
}
if (thread_info_flags & _TIF_USER_RETURN_NOTIFY)
fire_user_return_notifiers();
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index 8d656fb..0eb2fc2 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -329,6 +329,7 @@
320 common kexec_file_load sys_kexec_file_load
321 common bpf sys_bpf
322 64 execveat stub_execveat
+323 common thread_local_abi sys_thread_local_abi
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/exec.c b/fs/exec.c
index c7f9b73..e5acf80 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1555,6 +1555,7 @@ static int do_execveat_common(int fd, struct filename *filename,
/* execve succeeded */
current->fs->in_exec = 0;
current->in_execve = 0;
+ thread_local_abi_execve(current);
acct_update_integrals(current);
task_numa_free(current);
free_bprm(bprm);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a419b65..4a3fc52 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2,6 +2,7 @@
#define _LINUX_SCHED_H
#include <uapi/linux/sched.h>
+#include <uapi/linux/thread_local_abi.h>
#include <linux/sched/prio.h>
@@ -1710,6 +1711,10 @@ struct task_struct {
#ifdef CONFIG_DEBUG_ATOMIC_SLEEP
unsigned long task_state_change;
#endif
+#ifdef CONFIG_THREAD_LOCAL_ABI
+ size_t thread_local_abi_len;
+ struct thread_local_abi __user *thread_local_abi;
+#endif
};
/* Future-safe accessor for struct task_struct's cpus_allowed. */
@@ -3090,4 +3095,34 @@ static inline unsigned long rlimit_max(unsigned int limit)
return task_rlimit_max(current, limit);
}
+#ifdef CONFIG_THREAD_LOCAL_ABI
+void thread_local_abi_fork(struct task_struct *t);
+void thread_local_abi_execve(struct task_struct *t);
+void getcpu_cache_handle_notify_resume(struct task_struct *t);
+static inline bool getcpu_cache_active(struct task_struct *t)
+{
+ struct thread_local_abi __user *tlap = t->thread_local_abi;
+
+ if (!tlap || t->thread_local_abi_len <
+ offsetof(struct thread_local_abi, cpu)
+ + sizeof(tlap->cpu))
+ return false;
+ return true;
+}
+#else
+static inline void thread_local_abi_fork(struct task_struct *t)
+{
+}
+static inline void thread_local_abi_execve(struct task_struct *t)
+{
+}
+static inline void getcpu_cache_handle_notify_resume(struct task_struct *t)
+{
+}
+static inline bool getcpu_cache_active(struct task_struct *t)
+{
+ return false;
+}
+#endif
+
#endif
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index e016bd9..50aa984 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
__SYSCALL(__NR_bpf, sys_bpf)
#define __NR_execveat 281
__SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
+#define __NR_thread_local_abi 282
+__SYSCALL(__NR_thread_local_abi, sys_thread_local_abi)
#undef __NR_syscalls
-#define __NR_syscalls 282
+#define __NR_syscalls 283
/*
* All syscalls below here should go away really,
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 68ceb97..dfd6a30 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -389,6 +389,7 @@ header-y += tcp_metrics.h
header-y += telephony.h
header-y += termios.h
header-y += thermal.h
+header-y += thread_local_abi.h
header-y += time.h
header-y += times.h
header-y += timex.h
diff --git a/include/uapi/linux/thread_local_abi.h b/include/uapi/linux/thread_local_abi.h
new file mode 100644
index 0000000..6487c92
--- /dev/null
+++ b/include/uapi/linux/thread_local_abi.h
@@ -0,0 +1,37 @@
+#ifndef _UAPI_LINUX_THREAD_LOCAL_ABI_H
+#define _UAPI_LINUX_THREAD_LOCAL_ABI_H
+
+/*
+ * linux/thread_local_abi.h
+ *
+ * thread_local_abi system call API
+ *
+ * Copyright (c) 2015 Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/types.h>
+
+/* This structure is an ABI that can only be extended. */
+struct thread_local_abi {
+ int32_t cpu;
+};
+
+#endif /* _UAPI_LINUX_THREAD_LOCAL_ABI_H */
diff --git a/init/Kconfig b/init/Kconfig
index f5dbc6d..c8ff5fa 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1559,6 +1559,15 @@ config PCI_QUIRKS
bugs/quirks. Disable this only if your target machine is
unaffected by PCI quirks.
+config THREAD_LOCAL_ABI
+ bool "Enable thread-local ABI" if EXPERT
+ default y
+ help
+ Enable the thread-local ABI system call. It provides a user-space
+ cache for the current CPU number value.
+
+ If unsure, say Y.
+
config EMBEDDED
bool "Embedded system"
option allnoconfig_y
diff --git a/kernel/Makefile b/kernel/Makefile
index 1408b33..cc1f3d4 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o
obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
obj-$(CONFIG_TORTURE_TEST) += torture.o
+obj-$(CONFIG_THREAD_LOCAL_ABI) += thread_local_abi.o
$(obj)/configs.o: $(obj)/config_data.h
diff --git a/kernel/fork.c b/kernel/fork.c
index cf65139..e17bcb3 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1549,6 +1549,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
cgroup_post_fork(p);
if (clone_flags & CLONE_THREAD)
threadgroup_change_end(current);
+ if (!(clone_flags & CLONE_THREAD))
+ thread_local_abi_fork(p);
perf_event_fork(p);
trace_task_newtask(p, clone_flags);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 62671f5..668a502 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1823,6 +1823,10 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
p->numa_group = NULL;
#endif /* CONFIG_NUMA_BALANCING */
+#ifdef CONFIG_THREAD_LOCAL_ABI
+ p->thread_local_abi_len = 0;
+ p->thread_local_abi = NULL;
+#endif
}
#ifdef CONFIG_NUMA_BALANCING
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index dc0f435..bf3e346 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -921,6 +921,8 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
{
set_task_rq(p, cpu);
#ifdef CONFIG_SMP
+ if (getcpu_cache_active(p))
+ set_tsk_thread_flag(p, TIF_NOTIFY_RESUME);
/*
* After ->cpu is set up to a new value, task_rq_lock(p, ...) can be
* successfuly executed on another CPU. We must ensure that updates of
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 5adcb0a..cadb903 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -229,3 +229,6 @@ cond_syscall(sys_bpf);
/* execveat */
cond_syscall(sys_execveat);
+
+/* thread-local ABI */
+cond_syscall(sys_thread_local_abi);
diff --git a/kernel/thread_local_abi.c b/kernel/thread_local_abi.c
new file mode 100644
index 0000000..681f06e
--- /dev/null
+++ b/kernel/thread_local_abi.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2015 Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
+ *
+ * thread_local_abi system call
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/uaccess.h>
+#include <linux/syscalls.h>
+
+static int getcpu_cache_update(struct task_struct *t)
+{
+ if (put_user(raw_smp_processor_id(), &t->thread_local_abi->cpu)) {
+ t->thread_local_abi_len = 0;
+ t->thread_local_abi = NULL;
+ return -1;
+ }
+ return 0;
+}
+
+/*
+ * This resume handler should always be executed between a migration
+ * triggered by preemption and return to user-space.
+ */
+void getcpu_cache_handle_notify_resume(struct task_struct *t)
+{
+ BUG_ON(!getcpu_cache_active(t));
+ if (unlikely(t->flags & PF_EXITING))
+ return;
+ if (getcpu_cache_update(t))
+ force_sig(SIGSEGV, t);
+}
+
+/*
+ * If parent process has a thread-local ABI, the child inherits. Only applies
+ * when forking a process, not a thread.
+ */
+void thread_local_abi_fork(struct task_struct *t)
+{
+ t->thread_local_abi_len = current->thread_local_abi_len;
+ t->thread_local_abi = current->thread_local_abi;
+}
+
+void thread_local_abi_execve(struct task_struct *t)
+{
+ t->thread_local_abi_len = 0;
+ t->thread_local_abi = NULL;
+}
+
+/*
+ * sys_thread_local_abi - setup thread-local ABI for caller thread
+ */
+SYSCALL_DEFINE3(thread_local_abi, struct thread_local_abi __user *, tlap,
+ size_t, len, int, flags)
+{
+ size_t minlen;
+
+ if (flags)
+ return -EINVAL;
+ if (current->thread_local_abi && tlap)
+ return -EBUSY;
+ /* Agree on the intersection of userspace and kernel features */
+ minlen = min_t(size_t, len, sizeof(struct thread_local_abi));
+ current->thread_local_abi_len = minlen;
+ current->thread_local_abi = tlap;
+ if (!tlap)
+ return 0;
+ /*
+ * Migration checks ->thread_local_abi to see if notify_resume
+ * flag should be set. Therefore, we need to ensure that
+ * the scheduler sees ->thread_local_abi before we update its content.
+ */
+ barrier(); /* Store thread_local_abi before update content */
+ if (getcpu_cache_active(current)) {
+ if (getcpu_cache_update(current))
+ return -EFAULT;
+ }
+ return minlen;
+}
--
2.1.4
^ permalink raw reply related
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Andy Lutomirski @ 2015-07-16 19:27 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Ben Maurer, Paul Turner, Andrew Hunter, Peter Zijlstra,
Ingo Molnar, rostedt, Paul E. McKenney, Josh Triplett,
Lai Jiangshan, Linus Torvalds, Andrew Morton, linux-api,
libc-alpha
In-Reply-To: <549319255.383.1437070088597.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Thu, Jul 16, 2015 at 11:08 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> ----- On Jul 14, 2015, at 5:34 AM, Ben Maurer bmaurer-b10kYP2dOMg@public.gmane.org wrote:
>
>> Mathieu Desnoyers wrote:
>>> If we invoke this per-thread registration directly in the glibc NPTL
>>> implementation,
>>> in start_thread, do you think it would fit your requirements ?
>>
>> I guess this would basically be transparent to the user -- we'd just need to
>> make sure that the registration happens very early, before any chance of
>> calling malloc.
>
> Yes, this is my thinking too.
>
>>
>> That said, having the ability for the kernel to understand that TLS
>> implementation are laid out using the same offset on each thread seems like
>> something that could be valuable long term. Doing so makes it possible to build
>> other TLS-based features without forcing each thread to be registered.
>
> AFAIU, using a fixed hardcoded ABI between kernel and user-space might make
> transition from the pre-existing ABI (where this memory area is not
> reserved) a bit tricky without registering the area, or getting a "feature"
> flag, through a system call.
>
> The related question then becomes: should we issue this system call once
> per process, or once per thread at thread creation ? Issuing it once per
> thread is marginally more costly for thread creation, but seems to be
> easier to deal with internally within the kernel.
>
> We could however ensure that only a single system call is needed per new-coming
> thread, rather than one system call per feature. One way to do this would be
> to register an area that may contain more than just the CPU id. It could
> consist of an expandable structure with fixed offsets. When registered, we
> could pass the size of that structure as an argument to the system call, so
> the kernel knows which features are expected by user-space.
If we actually bit the bullet and implemented per-cpu mappings, we
could have this be completely flexible because there would be no
format at all. Similarly, if we implemented per-cpu segments,
userspace would need to agree with *itself* how to arbitrate it, but
the kernel wouldn't need to be involved.
With this kind of memory poking, it's definitely messier, which is unfortunate.
--Andy
^ permalink raw reply
* Re: [PATCH -mm v8 4/7] proc: add kpagecgroup file
From: Andres Lagar-Cavilla @ 2015-07-16 19:04 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Minchan Kim, Raghavendra K T, Johannes Weiner,
Michal Hocko, Greg Thelen, Michel Lespinasse, David Rientjes,
Pavel Emelyanov, Cyrill Gorcunov, Jonathan Corbet, linux-api,
linux-doc, linux-mm, cgroups, linux-kernel
In-Reply-To: <20150716092841.GA2001@esperanza>
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
On Thu, Jul 16, 2015 at 2:28 AM, Vladimir Davydov <vdavydov@parallels.com>
wrote:
> On Wed, Jul 15, 2015 at 12:03:18PM -0700, Andres Lagar-Cavilla wrote:
> > For both /proc/kpage* interfaces you add (and more critically for the
> > rmap-causing one, kpageidle):
> >
> > It's a good idea to do cond_sched(). Whether after each pfn, each Nth
> > pfn, each put_user, I leave to you, but a reasonable cadence is
> > needed, because user-space can call this on the entire physical
> > address space, and that's a lot of work to do without re-scheduling.
>
> I really don't think it's necessary. These files can only be
> read/written by the root, who has plenty ways to kill the system anyway.
> The program that is allowed to read/write these files must be conscious
> and do it in batches of reasonable size. AFAICS the same reasoning
> already lays behind /proc/kpagecount and /proc/kpageflag, which also do
> not thrust the "right" batch size on their readers.
>
Beg to disagree. You're conflating intended use with system health. A
cond_sched() is a one-liner.
Andres
>
> Thanks,
> Vladimir
>
--
Andres Lagar-Cavilla | Google Kernel Team | andreslc@google.com
[-- Attachment #2: Type: text/html, Size: 2520 bytes --]
^ permalink raw reply
* [PATCH] pagemap: update documentation
From: Konstantin Khlebnikov @ 2015-07-16 18:47 UTC (permalink / raw)
To: linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Andrew Morton, Naoya Horiguchi
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150714152516.29844.69929.stgit@buzz>
Notes about recent changes.
Signed-off-by: Konstantin Khlebnikov <khlebnikov-XoJtRXgx1JseBXzfvpsJ4g@public.gmane.org>
---
Documentation/vm/pagemap.txt | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index 3cfbbb333ea1..aab39aa7dd8f 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -16,12 +16,17 @@ There are three components to pagemap:
* Bits 0-4 swap type if swapped
* Bits 5-54 swap offset if swapped
* Bit 55 pte is soft-dirty (see Documentation/vm/soft-dirty.txt)
- * Bit 56 page exlusively mapped
+ * Bit 56 page exclusively mapped (since 4.2)
* Bits 57-60 zero
- * Bit 61 page is file-page or shared-anon
+ * Bit 61 page is file-page or shared-anon (since 3.5)
* Bit 62 page swapped
* Bit 63 page present
+ Since Linux 4.0 only users with the CAP_SYS_ADMIN capability can get PFNs:
+ for unprivileged users from 4.0 till 4.2 open fails with -EPERM, starting
+ from from 4.2 PFN field is zeroed if user has no CAP_SYS_ADMIN capability.
+ Reason: information about PFNs helps in exploiting Rowhammer vulnerability.
+
If the page is not present but in swap, then the PFN contains an
encoding of the swap file number and the page's offset into the
swap. Unmapped pages return a null PFN. This allows determining
@@ -160,3 +165,8 @@ Other notes:
Reading from any of the files will return -EINVAL if you are not starting
the read on an 8-byte boundary (e.g., if you sought an odd number of bytes
into the file), or if the size of the read is not a multiple of 8 bytes.
+
+Before Linux 3.11 pagemap bits 55-60 were used for "page-shift" (which is
+always 12 at most architectures). Since Linux 3.11 their meaning changes
+after first clear of soft-dirty bits. Since Linux 4.2 they are used for
+flags unconditionally.
^ permalink raw reply related
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