Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1482782 > unrolled thread

[PATCH 00/12] thread_info cleanups and stack caching

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] thread_info cleanups and stack caching Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:30 +0200
    [PATCH 03/12] um: Stop conflating task_struct::stack with thread_info Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:30 +0200
      Re: [PATCH 03/12] um: Stop conflating task_struct::stack with  thread_info Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:30 +0200
      [tip:x86/asm] um/Stop conflating task_struct::stack with  thread_info tip-bot for Linus Torvalds <tipbot@zytor.com> - 2016-09-15 12:50 +0200
    [PATCH 12/12] fork: Cache two thread stacks per cpu if CONFIG_VMAP_STACK is set Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [PATCH 01/12] x86/asm: Move 'status' from struct thread_info to struct thread_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      [tip:x86/asm] x86/asm: Move the thread_info::status field to  thread_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
    [PATCH 10/12] lib/syscall: Pin the task stack in collect_syscall() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-14 17:00 +0200
        Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-14 20:30 +0200
          Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-14 20:40 +0200
            Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-15 20:10 +0200
              Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-15 20:40 +0200
                Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-15 20:50 +0200
                  Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Josh Poimboeuf <jpoimboe@redhat.com> - 2016-09-15 21:20 +0200
                    Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 09:50 +0200
                      Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@amacapital.net> - 2016-09-16 17:20 +0200
                        Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk() Andy Lutomirski <luto@kernel.org> - 2016-09-16 17:40 +0200
                          Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 18:40 +0200
                        Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Peter Zijlstra <peterz@infradead.org> - 2016-09-16 17:40 +0200
      Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:40 +0200
        Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in  save_stack_trace_tsk() Ingo Molnar <mingo@kernel.org> - 2016-09-15 11:30 +0200
    [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Jann Horn <jann@thejh.net> - 2016-09-17 04:10 +0200
        Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@amacapital.net> - 2016-09-23 00:50 +0200
          Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Andy Lutomirski <luto@amacapital.net> - 2016-09-23 01:00 +0200
          Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Jann Horn <jann@thejh.net> - 2016-09-23 09:50 +0200
            Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Kees Cook <keescook@chromium.org> - 2016-09-23 20:30 +0200
              Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Jann Horn <jann@thejh.net> - 2016-09-23 20:40 +0200
                Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan() Tycho Andersen <tycho.andersen@canonical.com> - 2016-09-26 07:20 +0200
    [PATCH 06/12] sched: Add try_get_task_stack() and put_task_stack() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      Re: [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info() Ingo Molnar <mingo@kernel.org> - 2016-09-15 08:30 +0200
      [tip:x86/asm] x86/entry: Get rid of pt_regs_to_thread_info() tip-bot for Linus Torvalds <tipbot@zytor.com> - 2016-09-15 12:50 +0200
    [PATCH 05/12] x86: Move thread_info into task_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      [tip:x86/asm] x86: Move thread_info into task_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
    [PATCH 04/12] sched: Allow putting thread_info into task_struct Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
      [tip:x86/asm] sched/core: Allow putting thread_info into  task_struct tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-09-15 12:50 +0200
    [PATCH 11/12] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200
    [PATCH 07/12] kthread: to_live_kthread() needs try_get_task_stack() Andy Lutomirski <luto@kernel.org> - 2016-09-13 23:40 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1485127 — Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()

On Fri, Sep 16, 2016 at 08:12:40AM -0700, Andy Lutomirski wrote:
> On Fri, Sep 16, 2016 at 12:47 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Thu, Sep 15, 2016 at 02:19:38PM -0500, Josh Poimboeuf wrote:

> >> My idea was to use task_rq_lock() to lock the runqueue and then check
> >> tsk->on_cpu.  I think Peter wasn't too keen on it.
> >
> > That basically allows a DoS on the scheduler, since a user can run tasks
> > on every cpu (through sys_sched_setaffinity()). Then doing while (1) cat
> > /proc/$PID/stack would saturate the rq->lock on every CPU.
> >
> > The more tasks the merrier.
> 
> Is this worse than it would be if this code used preempt_disable()
> (which I think it did until very recently)?

Much worse, since the proposed task_rq_lock() not only disables
preemption, it also disables IRQs and takes 2 locks. And hogging the
rq->lock affects other tasks their ability to schedule.

[toc] | [prev] | [next] | [standalone]

#1483877 — Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()

* Andy Lutomirski <luto@kernel.org> wrote:

> This will prevent a crash if the target task dies before or while
> dumping its stack once we start freeing task stacks early.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/kernel/stacktrace.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
> index 4738f5e0f2ab..b3f32fbe3ba4 100644
> --- a/arch/x86/kernel/stacktrace.c
> +++ b/arch/x86/kernel/stacktrace.c
> @@ -79,9 +79,14 @@ void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace)
>  
>  void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
>  {
> +	if (!try_get_task_stack(tsk))
> +		return;
> +
>  	dump_trace(tsk, NULL, NULL, 0, &save_stack_ops_nosched, trace);
>  	if (trace->nr_entries < trace->max_entries)
>  		trace->entries[trace->nr_entries++] = ULONG_MAX;
> +
> +	put_task_stack(tsk);
>  }
>  EXPORT_SYMBOL_GPL(save_stack_trace_tsk);

So I very much like the first half of the series, the thread_info merge into 
task_struct (yay!) and I am in the process of applying those bits to -tip.

So the above not (yet) fully correct patch is in preparation to a later change you 
are doing in this series:

    [PATCH 11/12] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK

But I am having serious second thoughts about the fact that we now have to sprinke 
non-obvious try_get_task_stack()/put_task_stack() pairs into debugging code that 
never really needed anything like that, which pairs are easy to get wrong, easy to 
miss - and generally if we miss them will it will result in a slightly buggy, very 
unpleasant, racy, crashy behavior of the kernel.

Can we just ... not do the delayed freeing?

I mean, the cache hotness arguments don't look overly convincing to me: if we just 
start not using a piece of formerly cache hot memory then those cache lines will 
eventually decay naturally in whatever LRU scheme the CPU is using and will be 
reused without much harm done. It's just "another day in CPU land" that happens 
all the time. Yes, we could reduce the latency of the freeing and thus slightly 
improve the locality of other bits ... but is it really measurable or noticeable 
in any fashion. It's like task/thread fork/clone/exit is _that_ much of a fast 
path.

And then there's also the whole CONFIG_THREAD_INFO_IN_TASK #ifdeffery spreading.

I.e I just don't see the justification for this kind of object life time assymetry 
and hard to debug fragility.

Am I missing something?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1483932 — Re: [PATCH 08/12] x86/dumpstack: Pin the target stack in save_stack_trace_tsk()

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> It absolutely is - compared to the vmalloc costs. Andy has quite noticeable 
> slowdowns without the stack reuse - and the stack reuse requires the delayed 
> freeing. With the stack reuse, fork/exit actually sped up.

Ok, great, that's convincing!

I'm still applying it two-phase and I'll wait for v2 of the second half of the 
series, to have the fix Josh alluded to.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1482790 — [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

This will prevent a crash if get_wchan() runs after the task stack
is freed.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/process.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 0b9ed8ec5226..4002b475171c 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -532,15 +532,18 @@ unsigned long thread_saved_pc(struct task_struct *tsk)
  */
 unsigned long get_wchan(struct task_struct *p)
 {
-	unsigned long start, bottom, top, sp, fp, ip;
+	unsigned long start, bottom, top, sp, fp, ip, ret = 0;
 	int count = 0;
 
 	if (!p || p == current || p->state == TASK_RUNNING)
 		return 0;
 
+	if (!try_get_task_stack(p))
+		return 0;
+
 	start = (unsigned long)task_stack_page(p);
 	if (!start)
-		return 0;
+		goto out;
 
 	/*
 	 * Layout of the stack page:
@@ -564,16 +567,21 @@ unsigned long get_wchan(struct task_struct *p)
 
 	sp = READ_ONCE(p->thread.sp);
 	if (sp < bottom || sp > top)
-		return 0;
+		goto out;
 
 	fp = READ_ONCE_NOCHECK(((struct inactive_task_frame *)sp)->bp);
 	do {
 		if (fp < bottom || fp > top)
-			return 0;
+			goto out;
 		ip = READ_ONCE_NOCHECK(*(unsigned long *)(fp + sizeof(unsigned long)));
-		if (!in_sched_functions(ip))
-			return ip;
+		if (!in_sched_functions(ip)) {
+			ret = ip;
+			goto out;
+		}
 		fp = READ_ONCE_NOCHECK(*(unsigned long *)fp);
 	} while (count++ < 16 && p->state != TASK_RUNNING);
-	return 0;
+
+out:
+	put_task_stack(p);
+	return ret;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1485465 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

[Multipart message — attachments visible in raw view] — view raw

On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
> This will prevent a crash if get_wchan() runs after the task stack
> is freed.

I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
they read from the saved userspace registers area at the top of the kernel stack?

Used on remote processes in:
  vma_is_stack_for_task() (via /proc/$pid/maps)
  do_task_stat() (/proc/$pid/stat)

[toc] | [prev] | [next] | [standalone]

#1489629 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

On Fri, Sep 16, 2016 at 7:00 PM, Jann Horn <jann@thejh.net> wrote:
> On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
>> This will prevent a crash if get_wchan() runs after the task stack
>> is freed.
>
> I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
> they read from the saved userspace registers area at the top of the kernel stack?
>
> Used on remote processes in:
>   vma_is_stack_for_task() (via /proc/$pid/maps)

This isn't used in /proc/$pid/maps -- it's only used in
/proc/$pid/task/$tid/maps.  I wonder if anyone actually cares about it
-- it certainly won't work reliably.

I could pin the stack in vma_is_stack_for_task, but it seems
potentially better to me to change it to vma_is_stack_for_current()
and remove the offending caller in /proc, replacing it with "return
0".  Thoughts?

>   do_task_stat() (/proc/$pid/stat)

Like this:

        mm = get_task_mm(task);
        if (mm) {
                vsize = task_vsize(mm);
                if (permitted) {
                        eip = KSTK_EIP(task);
                        esp = KSTK_ESP(task);
                }
        }

Can we just delete this outright?  It seems somewhere between mostly
and entirely useless, and it also seems dangerous.  Until very
recently, on x86_64, this would have been a potential info leak, as
SYSCALL followed closely by a hardware interrupt would cause *kernel*
values to land in task_pt_regs().  I don't even want to think about
what this code does if the task is in vm86 mode.  I wouldn't be at all
surprised if non-x86 architectures have all kinds of interesting
thinks happen if you do this to a task that isn't running normal
non-atomic kernel code at the time.

I would advocate for unconditionally returning zeros in these two stat fields.

[toc] | [prev] | [next] | [standalone]

#1489634 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

On Thu, Sep 22, 2016 at 3:44 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Sep 16, 2016 at 7:00 PM, Jann Horn <jann@thejh.net> wrote:
>> On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
>>> This will prevent a crash if get_wchan() runs after the task stack
>>> is freed.
>>
>> I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
>> they read from the saved userspace registers area at the top of the kernel stack?
>>
>> Used on remote processes in:
>>   vma_is_stack_for_task() (via /proc/$pid/maps)
>
> This isn't used in /proc/$pid/maps -- it's only used in
> /proc/$pid/task/$tid/maps.  I wonder if anyone actually cares about it
> -- it certainly won't work reliably.
>
> I could pin the stack in vma_is_stack_for_task, but it seems
> potentially better to me to change it to vma_is_stack_for_current()
> and remove the offending caller in /proc, replacing it with "return
> 0".  Thoughts?

The history here is strange:

Before March 2012, we used to only consider the "stack" of the mm --
that is, the VMA that the mm actually treats as a stack.  Then:

commit b76437579d1344b612cf1851ae610c636cec7db0
Author: Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>
Date:   Wed Mar 21 16:34:04 2012 -0700

    procfs: mark thread stack correctly in proc/<pid>/maps

and we did something extra horrible to try to find out whose stack was
where.  This got partially reverted by:

commit 65376df582174ffcec9e6471bf5b0dd79ba05e4a
Author: Johannes Weiner <hannes@cmpxchg.org>
Date:   Tue Feb 2 16:57:29 2016 -0800

    proc: revert /proc/<pid>/maps [stack:TID] annotation

and now we're in the current situation where it's fast but still racy.

Any objection if I finish reverting the patch and restore the pre-2012
behavior?  Frankly, I wouldn't mind trying to excise KSTK_EIP and
KSTK_ESP from the kernel entirely, to be replaced with
current_user_sp() and current_user_ip().  Having those macros around
seems likely to make people think they're safe to use.

>
>>   do_task_stat() (/proc/$pid/stat)
>
> Like this:
>
>         mm = get_task_mm(task);
>         if (mm) {
>                 vsize = task_vsize(mm);
>                 if (permitted) {
>                         eip = KSTK_EIP(task);
>                         esp = KSTK_ESP(task);
>                 }
>         }
>
> Can we just delete this outright?  It seems somewhere between mostly
> and entirely useless, and it also seems dangerous.  Until very
> recently, on x86_64, this would have been a potential info leak, as
> SYSCALL followed closely by a hardware interrupt would cause *kernel*
> values to land in task_pt_regs().  I don't even want to think about
> what this code does if the task is in vm86 mode.  I wouldn't be at all
> surprised if non-x86 architectures have all kinds of interesting
> thinks happen if you do this to a task that isn't running normal
> non-atomic kernel code at the time.
>
> I would advocate for unconditionally returning zeros in these two stat fields.

--Andy

[toc] | [prev] | [next] | [standalone]

#1489813 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 22, 2016 at 03:44:37PM -0700, Andy Lutomirski wrote:
> On Fri, Sep 16, 2016 at 7:00 PM, Jann Horn <jann@thejh.net> wrote:
> > On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
> >> This will prevent a crash if get_wchan() runs after the task stack
> >> is freed.
> >
> > I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
> > they read from the saved userspace registers area at the top of the kernel stack?
> >
> > Used on remote processes in:
> >   vma_is_stack_for_task() (via /proc/$pid/maps)
> 
> This isn't used in /proc/$pid/maps -- it's only used in
> /proc/$pid/task/$tid/maps.  I wonder if anyone actually cares about it
> -- it certainly won't work reliably.
> 
> I could pin the stack in vma_is_stack_for_task, but it seems
> potentially better to me to change it to vma_is_stack_for_current()
> and remove the offending caller in /proc, replacing it with "return
> 0".  Thoughts?

I just scrolled through the debian codesearch results for "\[stack\]" -
there seem to only be 105 across all of debian's packages, many of them
duplicates - and I didn't see any that looked like they used the tid map.
So I think this might work.

( https://codesearch.debian.net/search?q=%22%5C%5Bstack%5C%5D%22 )


> >   do_task_stat() (/proc/$pid/stat)
> 
> Like this:
> 
>         mm = get_task_mm(task);
>         if (mm) {
>                 vsize = task_vsize(mm);
>                 if (permitted) {
>                         eip = KSTK_EIP(task);
>                         esp = KSTK_ESP(task);
>                 }
>         }
> 
> Can we just delete this outright?  It seems somewhere between mostly
> and entirely useless, and it also seems dangerous.  Until very
> recently, on x86_64, this would have been a potential info leak, as
> SYSCALL followed closely by a hardware interrupt would cause *kernel*
> values to land in task_pt_regs().  I don't even want to think about
> what this code does if the task is in vm86 mode.  I wouldn't be at all
> surprised if non-x86 architectures have all kinds of interesting
> thinks happen if you do this to a task that isn't running normal
> non-atomic kernel code at the time.
> 
> I would advocate for unconditionally returning zeros in these two stat fields.

I'd like that a lot.

I guess the two things that might theoretically use it are ptrace users
and (very theoretically) sampling profiling stuff or so?

In gdb, the only code I can find that reads this is in gdb/linux-nat.c, but
it's behind an "#ifdef 0":

  #if 0   /* Don't know how architecture-dependent the rest is...
             Anyway the signal bitmap info is available from "status".  */
            if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
              printf_filtered (_("Kernel stack pointer: 0x%lx\n"), ltmp);
            if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
              printf_filtered (_("Kernel instr pointer: 0x%lx\n"), ltmp);
  [...]

strace and ltrace don't seem to be using it.

[toc] | [prev] | [next] | [standalone]

#1490376 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

On Fri, Sep 23, 2016 at 12:43 AM, Jann Horn <jann@thejh.net> wrote:
> On Thu, Sep 22, 2016 at 03:44:37PM -0700, Andy Lutomirski wrote:
>> On Fri, Sep 16, 2016 at 7:00 PM, Jann Horn <jann@thejh.net> wrote:
>> > On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
>> >> This will prevent a crash if get_wchan() runs after the task stack
>> >> is freed.
>> >
>> > I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
>> > they read from the saved userspace registers area at the top of the kernel stack?
>> >
>> > Used on remote processes in:
>> >   vma_is_stack_for_task() (via /proc/$pid/maps)
>>
>> This isn't used in /proc/$pid/maps -- it's only used in
>> /proc/$pid/task/$tid/maps.  I wonder if anyone actually cares about it
>> -- it certainly won't work reliably.
>>
>> I could pin the stack in vma_is_stack_for_task, but it seems
>> potentially better to me to change it to vma_is_stack_for_current()
>> and remove the offending caller in /proc, replacing it with "return
>> 0".  Thoughts?
>
> I just scrolled through the debian codesearch results for "\[stack\]" -
> there seem to only be 105 across all of debian's packages, many of them
> duplicates - and I didn't see any that looked like they used the tid map.
> So I think this might work.
>
> ( https://codesearch.debian.net/search?q=%22%5C%5Bstack%5C%5D%22 )
>
>
>> >   do_task_stat() (/proc/$pid/stat)
>>
>> Like this:
>>
>>         mm = get_task_mm(task);
>>         if (mm) {
>>                 vsize = task_vsize(mm);
>>                 if (permitted) {
>>                         eip = KSTK_EIP(task);
>>                         esp = KSTK_ESP(task);
>>                 }
>>         }
>>
>> Can we just delete this outright?  It seems somewhere between mostly
>> and entirely useless, and it also seems dangerous.  Until very
>> recently, on x86_64, this would have been a potential info leak, as
>> SYSCALL followed closely by a hardware interrupt would cause *kernel*
>> values to land in task_pt_regs().  I don't even want to think about
>> what this code does if the task is in vm86 mode.  I wouldn't be at all
>> surprised if non-x86 architectures have all kinds of interesting
>> thinks happen if you do this to a task that isn't running normal
>> non-atomic kernel code at the time.
>>
>> I would advocate for unconditionally returning zeros in these two stat fields.
>
> I'd like that a lot.
>
> I guess the two things that might theoretically use it are ptrace users
> and (very theoretically) sampling profiling stuff or so?
>
> In gdb, the only code I can find that reads this is in gdb/linux-nat.c, but
> it's behind an "#ifdef 0":
>
>   #if 0   /* Don't know how architecture-dependent the rest is...
>              Anyway the signal bitmap info is available from "status".  */
>             if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
>               printf_filtered (_("Kernel stack pointer: 0x%lx\n"), ltmp);
>             if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
>               printf_filtered (_("Kernel instr pointer: 0x%lx\n"), ltmp);
>   [...]
>
> strace and ltrace don't seem to be using it.

Does CRIU use this? I wouldn't expect so, since they're using ptrace,
IIUC, to freeze/restore.

-Kees

-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

#1490382 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

[Multipart message — attachments visible in raw view] — view raw

On Fri, Sep 23, 2016 at 11:28:26AM -0700, Kees Cook wrote:
> On Fri, Sep 23, 2016 at 12:43 AM, Jann Horn <jann@thejh.net> wrote:
> > On Thu, Sep 22, 2016 at 03:44:37PM -0700, Andy Lutomirski wrote:
> >> On Fri, Sep 16, 2016 at 7:00 PM, Jann Horn <jann@thejh.net> wrote:
> >> > On Tue, Sep 13, 2016 at 02:29:29PM -0700, Andy Lutomirski wrote:
> >> >> This will prevent a crash if get_wchan() runs after the task stack
> >> >> is freed.
> >> >
> >> > I think I found some more stuff. Have a look at KSTK_EIP() and KSTK_ESP(), I think
> >> > they read from the saved userspace registers area at the top of the kernel stack?
> >> >
> >> > Used on remote processes in:
> >> >   vma_is_stack_for_task() (via /proc/$pid/maps)
> >>
> >> This isn't used in /proc/$pid/maps -- it's only used in
> >> /proc/$pid/task/$tid/maps.  I wonder if anyone actually cares about it
> >> -- it certainly won't work reliably.
> >>
> >> I could pin the stack in vma_is_stack_for_task, but it seems
> >> potentially better to me to change it to vma_is_stack_for_current()
> >> and remove the offending caller in /proc, replacing it with "return
> >> 0".  Thoughts?
> >
> > I just scrolled through the debian codesearch results for "\[stack\]" -
> > there seem to only be 105 across all of debian's packages, many of them
> > duplicates - and I didn't see any that looked like they used the tid map.
> > So I think this might work.
> >
> > ( https://codesearch.debian.net/search?q=%22%5C%5Bstack%5C%5D%22 )
> >
> >
> >> >   do_task_stat() (/proc/$pid/stat)
> >>
> >> Like this:
> >>
> >>         mm = get_task_mm(task);
> >>         if (mm) {
> >>                 vsize = task_vsize(mm);
> >>                 if (permitted) {
> >>                         eip = KSTK_EIP(task);
> >>                         esp = KSTK_ESP(task);
> >>                 }
> >>         }
> >>
> >> Can we just delete this outright?  It seems somewhere between mostly
> >> and entirely useless, and it also seems dangerous.  Until very
> >> recently, on x86_64, this would have been a potential info leak, as
> >> SYSCALL followed closely by a hardware interrupt would cause *kernel*
> >> values to land in task_pt_regs().  I don't even want to think about
> >> what this code does if the task is in vm86 mode.  I wouldn't be at all
> >> surprised if non-x86 architectures have all kinds of interesting
> >> thinks happen if you do this to a task that isn't running normal
> >> non-atomic kernel code at the time.
> >>
> >> I would advocate for unconditionally returning zeros in these two stat fields.
> >
> > I'd like that a lot.
> >
> > I guess the two things that might theoretically use it are ptrace users
> > and (very theoretically) sampling profiling stuff or so?
> >
> > In gdb, the only code I can find that reads this is in gdb/linux-nat.c, but
> > it's behind an "#ifdef 0":
> >
> >   #if 0   /* Don't know how architecture-dependent the rest is...
> >              Anyway the signal bitmap info is available from "status".  */
> >             if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
> >               printf_filtered (_("Kernel stack pointer: 0x%lx\n"), ltmp);
> >             if (fscanf (procfile, "%lu ", &ltmp) > 0)     /* FIXME arch?  */
> >               printf_filtered (_("Kernel instr pointer: 0x%lx\n"), ltmp);
> >   [...]
> >
> > strace and ltrace don't seem to be using it.
> 
> Does CRIU use this? I wouldn't expect so, since they're using ptrace,
> IIUC, to freeze/restore.

As far as I can tell:

parse_pid_stat() parses them into a struct proc_pid_stat as "esp" and "eip",
but those struct members are never used (like, probably, most other members
of that struct).

child_opened_proc.c just opens /proc/%d/stat and then closes it again
immediately.

So in summary: I don't think so.

[toc] | [prev] | [next] | [standalone]

#1491071 — Re: [PATCH 09/12] x86/process: Pin the target stack in get_wchan()

On Fri, Sep 23, 2016 at 08:34:43PM +0200, Jann Horn wrote:
> On Fri, Sep 23, 2016 at 11:28:26AM -0700, Kees Cook wrote:
> > Does CRIU use this? I wouldn't expect so, since they're using ptrace,
> > IIUC, to freeze/restore.
> 
> As far as I can tell:
> 
> parse_pid_stat() parses them into a struct proc_pid_stat as "esp" and "eip",
> but those struct members are never used (like, probably, most other members
> of that struct).

Yes, that's my reading of it too.

> child_opened_proc.c just opens /proc/%d/stat and then closes it again
> immediately.

This is just a test for ordering of things that are restored, and it
could use any file in /proc, stat was just convenient.

> So in summary: I don't think so.

Yep, agreed.

Tycho

[toc] | [prev] | [next] | [standalone]

#1482791 — [PATCH 06/12] sched: Add try_get_task_stack() and put_task_stack()

There are a few places in the kernel that access stack memory
belonging to a different task.  Before we can start freeing task
stacks before the task_struct is freed, we need a way for those code
paths to pin the stack.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/sched.h | 16 ++++++++++++++++
 init/Kconfig          |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a287e8b13549..a95867267e9f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3094,11 +3094,19 @@ static inline struct thread_info *task_thread_info(struct task_struct *task)
 {
 	return &task->thread_info;
 }
+
+/*
+ * When accessing the stack of a non-current task that might exit, use
+ * try_get_task_stack() instead.  task_stack_page will return a pointer
+ * that could get freed out from under you.
+ */
 static inline void *task_stack_page(const struct task_struct *task)
 {
 	return task->stack;
 }
+
 #define setup_thread_stack(new,old)	do { } while(0)
+
 static inline unsigned long *end_of_stack(const struct task_struct *task)
 {
 	return task->stack;
@@ -3134,6 +3142,14 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
 }
 
 #endif
+
+static inline void *try_get_task_stack(struct task_struct *tsk)
+{
+	return task_stack_page(tsk);
+}
+
+static inline void put_task_stack(struct task_struct *tsk) {}
+
 #define task_stack_end_corrupted(task) \
 		(*(end_of_stack(task)) != STACK_END_MAGIC)
 
diff --git a/init/Kconfig b/init/Kconfig
index ec8d43894b02..3b9a47fe843b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -33,6 +33,9 @@ config THREAD_INFO_IN_TASK
 	  make this work, an arch will need to remove all thread_info fields
 	  except flags and fix any runtime bugs.
 
+	  One subtle change that will be needed is to use try_get_task_stack()
+	  and put_task_stack() in save_thread_stack_tsk() and get_wchan().
+
 menu "General setup"
 
 config BROKEN
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1482792 — [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info()

From: Linus Torvalds <torvalds@linux-foundation.org>

It was a nice optimization while it lasted, but thread_info is moving
and this optimization will no longer work.

Quoting Linus:

    Oh Gods, Andy. That pt_regs_to_thread_info() thing made me want
    to do unspeakable acts on a poor innocent wax figure that looked
    _exactly_ like you.

[changelog written by Andy]
Message-Id: <CA+55aFxvZhBu9U1cqpVm4frv0p5mqu=0TxsSqE-=95ft8HvCVA@mail.gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/common.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 871bbf975d4c..bdd9cc59d20f 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -31,13 +31,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
 
-static struct thread_info *pt_regs_to_thread_info(struct pt_regs *regs)
-{
-	unsigned long top_of_stack =
-		(unsigned long)(regs + 1) + TOP_OF_KERNEL_STACK_PADDING;
-	return (struct thread_info *)(top_of_stack - THREAD_SIZE);
-}
-
 #ifdef CONFIG_CONTEXT_TRACKING
 /* Called on entry from user mode with IRQs off. */
 __visible inline void enter_from_user_mode(void)
@@ -71,7 +64,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
 {
 	u32 arch = in_ia32_syscall() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
 
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned long ret = 0;
 	bool emulated = false;
 	u32 work;
@@ -173,18 +166,17 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
 		/* Disable IRQs and retry */
 		local_irq_disable();
 
-		cached_flags = READ_ONCE(pt_regs_to_thread_info(regs)->flags);
+		cached_flags = READ_ONCE(current_thread_info()->flags);
 
 		if (!(cached_flags & EXIT_TO_USERMODE_LOOP_FLAGS))
 			break;
-
 	}
 }
 
 /* Called with IRQs disabled. */
 __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	u32 cached_flags;
 
 	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
@@ -247,7 +239,7 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
  */
 __visible inline void syscall_return_slowpath(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	u32 cached_flags = READ_ONCE(ti->flags);
 
 	CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
@@ -270,7 +262,7 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
 #ifdef CONFIG_X86_64
 __visible void do_syscall_64(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned long nr = regs->orig_ax;
 
 	enter_from_user_mode();
@@ -303,7 +295,7 @@ __visible void do_syscall_64(struct pt_regs *regs)
  */
 static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned int nr = (unsigned int)regs->orig_ax;
 
 #ifdef CONFIG_IA32_EMULATION
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1483872 — Re: [PATCH 02/12] x86/entry: Get rid of pt_regs_to_thread_info()

* Andy Lutomirski <luto@kernel.org> wrote:

> From: Linus Torvalds <torvalds@linux-foundation.org>
> 
> It was a nice optimization while it lasted, but thread_info is moving
> and this optimization will no longer work.
> 
> Quoting Linus:
> 
>     Oh Gods, Andy. That pt_regs_to_thread_info() thing made me want
>     to do unspeakable acts on a poor innocent wax figure that looked
>     _exactly_ like you.
> 
> [changelog written by Andy]

Linus, since this came from you I presume this is still fine, but I'm adding your 
Signed-off-by as well to make the SOB chain neat:

  Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Is this ok?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1483966 — [tip:x86/asm] x86/entry: Get rid of pt_regs_to_thread_info()

Commit-ID:  97245d00585d82540f4538cf72d92a1e853c7b0e
Gitweb:     http://git.kernel.org/tip/97245d00585d82540f4538cf72d92a1e853c7b0e
Author:     Linus Torvalds <torvalds@linux-foundation.org>
AuthorDate: Tue, 13 Sep 2016 14:29:22 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 15 Sep 2016 08:25:12 +0200

x86/entry: Get rid of pt_regs_to_thread_info()

It was a nice optimization while it lasted, but thread_info is moving
and this optimization will no longer work.

Quoting Linus:

    Oh Gods, Andy. That pt_regs_to_thread_info() thing made me want
    to do unspeakable acts on a poor innocent wax figure that looked
    _exactly_ like you.

[ Changelog written by Andy. ]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jann Horn <jann@thejh.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/6376aa81c68798cc81631673f52bd91a3e078944.1473801993.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/common.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 871bbf9..bdd9cc5 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -31,13 +31,6 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
 
-static struct thread_info *pt_regs_to_thread_info(struct pt_regs *regs)
-{
-	unsigned long top_of_stack =
-		(unsigned long)(regs + 1) + TOP_OF_KERNEL_STACK_PADDING;
-	return (struct thread_info *)(top_of_stack - THREAD_SIZE);
-}
-
 #ifdef CONFIG_CONTEXT_TRACKING
 /* Called on entry from user mode with IRQs off. */
 __visible inline void enter_from_user_mode(void)
@@ -71,7 +64,7 @@ static long syscall_trace_enter(struct pt_regs *regs)
 {
 	u32 arch = in_ia32_syscall() ? AUDIT_ARCH_I386 : AUDIT_ARCH_X86_64;
 
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned long ret = 0;
 	bool emulated = false;
 	u32 work;
@@ -173,18 +166,17 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
 		/* Disable IRQs and retry */
 		local_irq_disable();
 
-		cached_flags = READ_ONCE(pt_regs_to_thread_info(regs)->flags);
+		cached_flags = READ_ONCE(current_thread_info()->flags);
 
 		if (!(cached_flags & EXIT_TO_USERMODE_LOOP_FLAGS))
 			break;
-
 	}
 }
 
 /* Called with IRQs disabled. */
 __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	u32 cached_flags;
 
 	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
@@ -247,7 +239,7 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
  */
 __visible inline void syscall_return_slowpath(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	u32 cached_flags = READ_ONCE(ti->flags);
 
 	CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
@@ -270,7 +262,7 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
 #ifdef CONFIG_X86_64
 __visible void do_syscall_64(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned long nr = regs->orig_ax;
 
 	enter_from_user_mode();
@@ -303,7 +295,7 @@ __visible void do_syscall_64(struct pt_regs *regs)
  */
 static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)
 {
-	struct thread_info *ti = pt_regs_to_thread_info(regs);
+	struct thread_info *ti = current_thread_info();
 	unsigned int nr = (unsigned int)regs->orig_ax;
 
 #ifdef CONFIG_IA32_EMULATION

[toc] | [prev] | [next] | [standalone]

#1482793 — [PATCH 05/12] x86: Move thread_info into task_struct

Now that most of the thread_info users have been cleaned up,
this is straightforward.

Most of this code was written by Linus.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/Kconfig                   |  1 +
 arch/x86/entry/entry_64.S          |  7 ++++--
 arch/x86/include/asm/thread_info.h | 46 --------------------------------------
 arch/x86/kernel/asm-offsets.c      |  4 +---
 arch/x86/kernel/irq_64.c           |  3 +--
 arch/x86/kernel/process.c          |  6 ++---
 6 files changed, 10 insertions(+), 57 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ce8860cccc34..452412cbd6c1 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -158,6 +158,7 @@ config X86
 	select SPARSE_IRQ
 	select SRCU
 	select SYSCTL_EXCEPTION_TRACE
+	select THREAD_INFO_IN_TASK
 	select USER_STACKTRACE_SUPPORT
 	select VIRT_TO_BUS
 	select X86_DEV_DMA_OPS			if X86_64
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index c0373d667674..0cfc665d6716 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -179,7 +179,8 @@ GLOBAL(entry_SYSCALL_64_after_swapgs)
 	 * If we need to do entry work or if we guess we'll need to do
 	 * exit work, go straight to the slow path.
 	 */
-	testl	$_TIF_WORK_SYSCALL_ENTRY|_TIF_ALLWORK_MASK, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
+	movq	PER_CPU_VAR(current_task), %r11
+	testl	$_TIF_WORK_SYSCALL_ENTRY|_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
 	jnz	entry_SYSCALL64_slow_path
 
 entry_SYSCALL_64_fastpath:
@@ -217,7 +218,8 @@ entry_SYSCALL_64_fastpath:
 	 */
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
-	testl	$_TIF_ALLWORK_MASK, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
+	movq	PER_CPU_VAR(current_task), %r11
+	testl	$_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
 	jnz	1f
 
 	LOCKDEP_SYS_EXIT
@@ -370,6 +372,7 @@ END(ptregs_\func)
 /*
  * %rdi: prev task
  * %rsi: next task
+ * rsi: task we're switching to
  */
 ENTRY(__switch_to_asm)
 	/*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index c9dcfe7c7e4b..2aaca53c0974 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -52,20 +52,6 @@ struct task_struct;
 #include <asm/cpufeature.h>
 #include <linux/atomic.h>
 
-struct thread_info {
-	struct task_struct	*task;		/* main task structure */
-	__u32			flags;		/* low level flags */
-	__u32			cpu;		/* current CPU */
-};
-
-#define INIT_THREAD_INFO(tsk)			\
-{						\
-	.task		= &tsk,			\
-	.flags		= 0,			\
-	.cpu		= 0,			\
-}
-
-#define init_thread_info	(init_thread_union.thread_info)
 #define init_stack		(init_thread_union.stack)
 
 #else /* !__ASSEMBLY__ */
@@ -157,11 +143,6 @@ struct thread_info {
  */
 #ifndef __ASSEMBLY__
 
-static inline struct thread_info *current_thread_info(void)
-{
-	return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
-}
-
 static inline unsigned long current_stack_pointer(void)
 {
 	unsigned long sp;
@@ -223,33 +204,6 @@ static inline int arch_within_stack_frames(const void * const stack,
 # define cpu_current_top_of_stack (cpu_tss + TSS_sp0)
 #endif
 
-/*
- * ASM operand which evaluates to a 'thread_info' address of
- * the current task, if it is known that "reg" is exactly "off"
- * bytes below the top of the stack currently.
- *
- * ( The kernel stack's size is known at build time, it is usually
- *   2 or 4 pages, and the bottom  of the kernel stack contains
- *   the thread_info structure. So to access the thread_info very
- *   quickly from assembly code we can calculate down from the
- *   top of the kernel stack to the bottom, using constant,
- *   build-time calculations only. )
- *
- * For example, to fetch the current thread_info->flags value into %eax
- * on x86-64 defconfig kernels, in syscall entry code where RSP is
- * currently at exactly SIZEOF_PTREGS bytes away from the top of the
- * stack:
- *
- *      mov ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS), %eax
- *
- * will translate to:
- *
- *      8b 84 24 b8 c0 ff ff      mov    -0x3f48(%rsp), %eax
- *
- * which is below the current RSP by almost 16K.
- */
-#define ASM_THREAD_INFO(field, reg, off) ((field)+(off)-THREAD_SIZE)(reg)
-
 #endif
 
 #ifdef CONFIG_COMPAT
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index add5f90b93d4..c62e015b126c 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -35,9 +35,7 @@ void common(void) {
 #endif
 
 	BLANK();
-	OFFSET(TI_flags, thread_info, flags);
-
-	BLANK();
+	OFFSET(TASK_TI_flags, task_struct, thread_info.flags);
 	OFFSET(TASK_addr_limit, task_struct, thread.addr_limit);
 
 	BLANK();
diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
index 4a7903714065..9ebd0b0e73d9 100644
--- a/arch/x86/kernel/irq_64.c
+++ b/arch/x86/kernel/irq_64.c
@@ -40,8 +40,7 @@ static inline void stack_overflow_check(struct pt_regs *regs)
 	if (user_mode(regs))
 		return;
 
-	if (regs->sp >= curbase + sizeof(struct thread_info) +
-				  sizeof(struct pt_regs) + STACK_TOP_MARGIN &&
+	if (regs->sp >= curbase + sizeof(struct pt_regs) + STACK_TOP_MARGIN &&
 	    regs->sp <= curbase + THREAD_SIZE)
 		return;
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index c1fa790c81cd..0b9ed8ec5226 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -549,9 +549,7 @@ unsigned long get_wchan(struct task_struct *p)
 	 * PADDING
 	 * ----------- top = topmax - TOP_OF_KERNEL_STACK_PADDING
 	 * stack
-	 * ----------- bottom = start + sizeof(thread_info)
-	 * thread_info
-	 * ----------- start
+	 * ----------- bottom = start
 	 *
 	 * The tasks stack pointer points at the location where the
 	 * framepointer is stored. The data on the stack is:
@@ -562,7 +560,7 @@ unsigned long get_wchan(struct task_struct *p)
 	 */
 	top = start + THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING;
 	top -= 2 * sizeof(unsigned long);
-	bottom = start + sizeof(struct thread_info);
+	bottom = start;
 
 	sp = READ_ONCE(p->thread.sp);
 	if (sp < bottom || sp > top)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1483970 — [tip:x86/asm] x86: Move thread_info into task_struct

Commit-ID:  15f4eae70d365bba26854c90b6002aaabb18c8aa
Gitweb:     http://git.kernel.org/tip/15f4eae70d365bba26854c90b6002aaabb18c8aa
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 13 Sep 2016 14:29:25 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 15 Sep 2016 08:25:13 +0200

x86: Move thread_info into task_struct

Now that most of the thread_info users have been cleaned up,
this is straightforward.

Most of this code was written by Linus.

Originally-from: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jann Horn <jann@thejh.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/a50eab40abeaec9cb9a9e3cbdeafd32190206654.1473801993.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/Kconfig                   |  1 +
 arch/x86/entry/entry_64.S          |  7 ++++--
 arch/x86/include/asm/thread_info.h | 46 --------------------------------------
 arch/x86/kernel/asm-offsets.c      |  4 +---
 arch/x86/kernel/irq_64.c           |  3 +--
 arch/x86/kernel/process.c          |  6 ++---
 6 files changed, 10 insertions(+), 57 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4c39728..2a83bc8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -157,6 +157,7 @@ config X86
 	select SPARSE_IRQ
 	select SRCU
 	select SYSCTL_EXCEPTION_TRACE
+	select THREAD_INFO_IN_TASK
 	select USER_STACKTRACE_SUPPORT
 	select VIRT_TO_BUS
 	select X86_DEV_DMA_OPS			if X86_64
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index e7fba58..2b46384 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -179,7 +179,8 @@ GLOBAL(entry_SYSCALL_64_after_swapgs)
 	 * If we need to do entry work or if we guess we'll need to do
 	 * exit work, go straight to the slow path.
 	 */
-	testl	$_TIF_WORK_SYSCALL_ENTRY|_TIF_ALLWORK_MASK, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
+	movq	PER_CPU_VAR(current_task), %r11
+	testl	$_TIF_WORK_SYSCALL_ENTRY|_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
 	jnz	entry_SYSCALL64_slow_path
 
 entry_SYSCALL_64_fastpath:
@@ -217,7 +218,8 @@ entry_SYSCALL_64_fastpath:
 	 */
 	DISABLE_INTERRUPTS(CLBR_NONE)
 	TRACE_IRQS_OFF
-	testl	$_TIF_ALLWORK_MASK, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
+	movq	PER_CPU_VAR(current_task), %r11
+	testl	$_TIF_ALLWORK_MASK, TASK_TI_flags(%r11)
 	jnz	1f
 
 	LOCKDEP_SYS_EXIT
@@ -370,6 +372,7 @@ END(ptregs_\func)
 /*
  * %rdi: prev task
  * %rsi: next task
+ * rsi: task we're switching to
  */
 ENTRY(__switch_to_asm)
 	/*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index c9dcfe7..2aaca53 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -52,20 +52,6 @@ struct task_struct;
 #include <asm/cpufeature.h>
 #include <linux/atomic.h>
 
-struct thread_info {
-	struct task_struct	*task;		/* main task structure */
-	__u32			flags;		/* low level flags */
-	__u32			cpu;		/* current CPU */
-};
-
-#define INIT_THREAD_INFO(tsk)			\
-{						\
-	.task		= &tsk,			\
-	.flags		= 0,			\
-	.cpu		= 0,			\
-}
-
-#define init_thread_info	(init_thread_union.thread_info)
 #define init_stack		(init_thread_union.stack)
 
 #else /* !__ASSEMBLY__ */
@@ -157,11 +143,6 @@ struct thread_info {
  */
 #ifndef __ASSEMBLY__
 
-static inline struct thread_info *current_thread_info(void)
-{
-	return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
-}
-
 static inline unsigned long current_stack_pointer(void)
 {
 	unsigned long sp;
@@ -223,33 +204,6 @@ static inline int arch_within_stack_frames(const void * const stack,
 # define cpu_current_top_of_stack (cpu_tss + TSS_sp0)
 #endif
 
-/*
- * ASM operand which evaluates to a 'thread_info' address of
- * the current task, if it is known that "reg" is exactly "off"
- * bytes below the top of the stack currently.
- *
- * ( The kernel stack's size is known at build time, it is usually
- *   2 or 4 pages, and the bottom  of the kernel stack contains
- *   the thread_info structure. So to access the thread_info very
- *   quickly from assembly code we can calculate down from the
- *   top of the kernel stack to the bottom, using constant,
- *   build-time calculations only. )
- *
- * For example, to fetch the current thread_info->flags value into %eax
- * on x86-64 defconfig kernels, in syscall entry code where RSP is
- * currently at exactly SIZEOF_PTREGS bytes away from the top of the
- * stack:
- *
- *      mov ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS), %eax
- *
- * will translate to:
- *
- *      8b 84 24 b8 c0 ff ff      mov    -0x3f48(%rsp), %eax
- *
- * which is below the current RSP by almost 16K.
- */
-#define ASM_THREAD_INFO(field, reg, off) ((field)+(off)-THREAD_SIZE)(reg)
-
 #endif
 
 #ifdef CONFIG_COMPAT
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index add5f90..c62e015 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -35,9 +35,7 @@ void common(void) {
 #endif
 
 	BLANK();
-	OFFSET(TI_flags, thread_info, flags);
-
-	BLANK();
+	OFFSET(TASK_TI_flags, task_struct, thread_info.flags);
 	OFFSET(TASK_addr_limit, task_struct, thread.addr_limit);
 
 	BLANK();
diff --git a/arch/x86/kernel/irq_64.c b/arch/x86/kernel/irq_64.c
index 4a79037..9ebd0b0 100644
--- a/arch/x86/kernel/irq_64.c
+++ b/arch/x86/kernel/irq_64.c
@@ -40,8 +40,7 @@ static inline void stack_overflow_check(struct pt_regs *regs)
 	if (user_mode(regs))
 		return;
 
-	if (regs->sp >= curbase + sizeof(struct thread_info) +
-				  sizeof(struct pt_regs) + STACK_TOP_MARGIN &&
+	if (regs->sp >= curbase + sizeof(struct pt_regs) + STACK_TOP_MARGIN &&
 	    regs->sp <= curbase + THREAD_SIZE)
 		return;
 
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index c1fa790..0b9ed8e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -549,9 +549,7 @@ unsigned long get_wchan(struct task_struct *p)
 	 * PADDING
 	 * ----------- top = topmax - TOP_OF_KERNEL_STACK_PADDING
 	 * stack
-	 * ----------- bottom = start + sizeof(thread_info)
-	 * thread_info
-	 * ----------- start
+	 * ----------- bottom = start
 	 *
 	 * The tasks stack pointer points at the location where the
 	 * framepointer is stored. The data on the stack is:
@@ -562,7 +560,7 @@ unsigned long get_wchan(struct task_struct *p)
 	 */
 	top = start + THREAD_SIZE - TOP_OF_KERNEL_STACK_PADDING;
 	top -= 2 * sizeof(unsigned long);
-	bottom = start + sizeof(struct thread_info);
+	bottom = start;
 
 	sp = READ_ONCE(p->thread.sp);
 	if (sp < bottom || sp > top)

[toc] | [prev] | [next] | [standalone]

#1482794 — [PATCH 04/12] sched: Allow putting thread_info into task_struct

If an arch opts in by setting CONFIG_THREAD_INFO_IN_TASK_STRUCT,
then thread_info is defined as a single 'u32 flags' and is the first
entry of task_struct.  thread_info::task is removed (it serves no
purpose if thread_info is embedded in task_struct), and
thread_info::cpu gets its own slot in task_struct.

This is heavily based on a patch written by Linus.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/init_task.h   |  9 +++++++++
 include/linux/sched.h       | 36 ++++++++++++++++++++++++++++++++++--
 include/linux/thread_info.h | 15 +++++++++++++++
 init/Kconfig                |  7 +++++++
 init/init_task.c            |  7 +++++--
 kernel/sched/sched.h        |  4 ++++
 6 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index f8834f820ec2..9c04d44eeb3c 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -15,6 +15,8 @@
 #include <net/net_namespace.h>
 #include <linux/sched/rt.h>
 
+#include <asm/thread_info.h>
+
 #ifdef CONFIG_SMP
 # define INIT_PUSHABLE_TASKS(tsk)					\
 	.pushable_tasks = PLIST_NODE_INIT(tsk.pushable_tasks, MAX_PRIO),
@@ -183,12 +185,19 @@ extern struct task_group root_task_group;
 # define INIT_KASAN(tsk)
 #endif
 
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+# define INIT_TASK_TI(tsk) .thread_info = INIT_THREAD_INFO(tsk),
+#else
+# define INIT_TASK_TI(tsk)
+#endif
+
 /*
  *  INIT_TASK is used to set up the first task table, touch at
  * your own risk!. Base=0, limit=0x1fffff (=2MB)
  */
 #define INIT_TASK(tsk)	\
 {									\
+	INIT_TASK_TI(tsk)						\
 	.state		= 0,						\
 	.stack		= init_stack,					\
 	.usage		= ATOMIC_INIT(2),				\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 20f9f47bcfd0..a287e8b13549 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1458,6 +1458,13 @@ struct tlbflush_unmap_batch {
 };
 
 struct task_struct {
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	/*
+	 * For reasons of header soup (see current_thread_info()), this
+	 * must be the first element of task_struct.
+	 */
+	struct thread_info thread_info;
+#endif
 	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
 	void *stack;
 	atomic_t usage;
@@ -1467,6 +1474,9 @@ struct task_struct {
 #ifdef CONFIG_SMP
 	struct llist_node wake_entry;
 	int on_cpu;
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	unsigned int cpu;	/* current CPU */
+#endif
 	unsigned int wakee_flips;
 	unsigned long wakee_flip_decay_ts;
 	struct task_struct *last_wakee;
@@ -2588,7 +2598,9 @@ extern void set_curr_task(int cpu, struct task_struct *p);
 void yield(void);
 
 union thread_union {
+#ifndef CONFIG_THREAD_INFO_IN_TASK
 	struct thread_info thread_info;
+#endif
 	unsigned long stack[THREAD_SIZE/sizeof(long)];
 };
 
@@ -3076,10 +3088,26 @@ static inline void threadgroup_change_end(struct task_struct *tsk)
 	cgroup_threadgroup_change_end(tsk);
 }
 
-#ifndef __HAVE_THREAD_FUNCTIONS
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+
+static inline struct thread_info *task_thread_info(struct task_struct *task)
+{
+	return &task->thread_info;
+}
+static inline void *task_stack_page(const struct task_struct *task)
+{
+	return task->stack;
+}
+#define setup_thread_stack(new,old)	do { } while(0)
+static inline unsigned long *end_of_stack(const struct task_struct *task)
+{
+	return task->stack;
+}
+
+#elif !defined(__HAVE_THREAD_FUNCTIONS)
 
 #define task_thread_info(task)	((struct thread_info *)(task)->stack)
-#define task_stack_page(task)	((task)->stack)
+#define task_stack_page(task)	((void *)(task)->stack)
 
 static inline void setup_thread_stack(struct task_struct *p, struct task_struct *org)
 {
@@ -3379,7 +3407,11 @@ static inline void ptrace_signal_wake_up(struct task_struct *t, bool resume)
 
 static inline unsigned int task_cpu(const struct task_struct *p)
 {
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	return p->cpu;
+#else
 	return task_thread_info(p)->cpu;
+#endif
 }
 
 static inline int task_node(const struct task_struct *p)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cbd8990e2e77..cb0ed342d6b1 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -13,6 +13,21 @@
 struct timespec;
 struct compat_timespec;
 
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+struct thread_info {
+	u32			flags;		/* low level flags */
+};
+
+#define INIT_THREAD_INFO(tsk)			\
+{						\
+	.flags		= 0,			\
+}
+#endif
+
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+#define current_thread_info() ((struct thread_info *)current)
+#endif
+
 /*
  * System call restart block.
  */
diff --git a/init/Kconfig b/init/Kconfig
index cac3f096050d..ec8d43894b02 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -26,6 +26,13 @@ config IRQ_WORK
 config BUILDTIME_EXTABLE_SORT
 	bool
 
+config THREAD_INFO_IN_TASK
+	bool
+	help
+	  Select this to move thread_info off the stack into task_struct.  To
+	  make this work, an arch will need to remove all thread_info fields
+	  except flags and fix any runtime bugs.
+
 menu "General setup"
 
 config BROKEN
diff --git a/init/init_task.c b/init/init_task.c
index ba0a7f362d9e..11f83be1fa79 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -22,5 +22,8 @@ EXPORT_SYMBOL(init_task);
  * Initial thread structure. Alignment of this is handled by a special
  * linker map entry.
  */
-union thread_union init_thread_union __init_task_data =
-	{ INIT_THREAD_INFO(init_task) };
+union thread_union init_thread_union __init_task_data = {
+#ifndef CONFIG_THREAD_INFO_IN_TASK
+	INIT_THREAD_INFO(init_task)
+#endif
+};
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c64fc5114004..3655c9625e5b 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1000,7 +1000,11 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
 	 * per-task data have been completed by this moment.
 	 */
 	smp_wmb();
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	p->cpu = cpu;
+#else
 	task_thread_info(p)->cpu = cpu;
+#endif
 	p->wake_cpu = cpu;
 #endif
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1483968 — [tip:x86/asm] sched/core: Allow putting thread_info into task_struct

Commit-ID:  c65eacbe290b8141554c71b2c94489e73ade8c8d
Gitweb:     http://git.kernel.org/tip/c65eacbe290b8141554c71b2c94489e73ade8c8d
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Tue, 13 Sep 2016 14:29:24 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 15 Sep 2016 08:25:13 +0200

sched/core: Allow putting thread_info into task_struct

If an arch opts in by setting CONFIG_THREAD_INFO_IN_TASK_STRUCT,
then thread_info is defined as a single 'u32 flags' and is the first
entry of task_struct.  thread_info::task is removed (it serves no
purpose if thread_info is embedded in task_struct), and
thread_info::cpu gets its own slot in task_struct.

This is heavily based on a patch written by Linus.

Originally-from: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jann Horn <jann@thejh.net>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/a0898196f0476195ca02713691a5037a14f2aac5.1473801993.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/init_task.h   |  9 +++++++++
 include/linux/sched.h       | 36 ++++++++++++++++++++++++++++++++++--
 include/linux/thread_info.h | 15 +++++++++++++++
 init/Kconfig                |  7 +++++++
 init/init_task.c            |  7 +++++--
 kernel/sched/sched.h        |  4 ++++
 6 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index f8834f8..9c04d44 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -15,6 +15,8 @@
 #include <net/net_namespace.h>
 #include <linux/sched/rt.h>
 
+#include <asm/thread_info.h>
+
 #ifdef CONFIG_SMP
 # define INIT_PUSHABLE_TASKS(tsk)					\
 	.pushable_tasks = PLIST_NODE_INIT(tsk.pushable_tasks, MAX_PRIO),
@@ -183,12 +185,19 @@ extern struct task_group root_task_group;
 # define INIT_KASAN(tsk)
 #endif
 
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+# define INIT_TASK_TI(tsk) .thread_info = INIT_THREAD_INFO(tsk),
+#else
+# define INIT_TASK_TI(tsk)
+#endif
+
 /*
  *  INIT_TASK is used to set up the first task table, touch at
  * your own risk!. Base=0, limit=0x1fffff (=2MB)
  */
 #define INIT_TASK(tsk)	\
 {									\
+	INIT_TASK_TI(tsk)						\
 	.state		= 0,						\
 	.stack		= init_stack,					\
 	.usage		= ATOMIC_INIT(2),				\
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 20f9f47..a287e8b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1458,6 +1458,13 @@ struct tlbflush_unmap_batch {
 };
 
 struct task_struct {
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	/*
+	 * For reasons of header soup (see current_thread_info()), this
+	 * must be the first element of task_struct.
+	 */
+	struct thread_info thread_info;
+#endif
 	volatile long state;	/* -1 unrunnable, 0 runnable, >0 stopped */
 	void *stack;
 	atomic_t usage;
@@ -1467,6 +1474,9 @@ struct task_struct {
 #ifdef CONFIG_SMP
 	struct llist_node wake_entry;
 	int on_cpu;
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	unsigned int cpu;	/* current CPU */
+#endif
 	unsigned int wakee_flips;
 	unsigned long wakee_flip_decay_ts;
 	struct task_struct *last_wakee;
@@ -2588,7 +2598,9 @@ extern void set_curr_task(int cpu, struct task_struct *p);
 void yield(void);
 
 union thread_union {
+#ifndef CONFIG_THREAD_INFO_IN_TASK
 	struct thread_info thread_info;
+#endif
 	unsigned long stack[THREAD_SIZE/sizeof(long)];
 };
 
@@ -3076,10 +3088,26 @@ static inline void threadgroup_change_end(struct task_struct *tsk)
 	cgroup_threadgroup_change_end(tsk);
 }
 
-#ifndef __HAVE_THREAD_FUNCTIONS
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+
+static inline struct thread_info *task_thread_info(struct task_struct *task)
+{
+	return &task->thread_info;
+}
+static inline void *task_stack_page(const struct task_struct *task)
+{
+	return task->stack;
+}
+#define setup_thread_stack(new,old)	do { } while(0)
+static inline unsigned long *end_of_stack(const struct task_struct *task)
+{
+	return task->stack;
+}
+
+#elif !defined(__HAVE_THREAD_FUNCTIONS)
 
 #define task_thread_info(task)	((struct thread_info *)(task)->stack)
-#define task_stack_page(task)	((task)->stack)
+#define task_stack_page(task)	((void *)(task)->stack)
 
 static inline void setup_thread_stack(struct task_struct *p, struct task_struct *org)
 {
@@ -3379,7 +3407,11 @@ static inline void ptrace_signal_wake_up(struct task_struct *t, bool resume)
 
 static inline unsigned int task_cpu(const struct task_struct *p)
 {
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	return p->cpu;
+#else
 	return task_thread_info(p)->cpu;
+#endif
 }
 
 static inline int task_node(const struct task_struct *p)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 2b5b10e..e2d0fd8 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -13,6 +13,21 @@
 struct timespec;
 struct compat_timespec;
 
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+struct thread_info {
+	u32			flags;		/* low level flags */
+};
+
+#define INIT_THREAD_INFO(tsk)			\
+{						\
+	.flags		= 0,			\
+}
+#endif
+
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+#define current_thread_info() ((struct thread_info *)current)
+#endif
+
 /*
  * System call restart block.
  */
diff --git a/init/Kconfig b/init/Kconfig
index cac3f09..ec8d438 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -26,6 +26,13 @@ config IRQ_WORK
 config BUILDTIME_EXTABLE_SORT
 	bool
 
+config THREAD_INFO_IN_TASK
+	bool
+	help
+	  Select this to move thread_info off the stack into task_struct.  To
+	  make this work, an arch will need to remove all thread_info fields
+	  except flags and fix any runtime bugs.
+
 menu "General setup"
 
 config BROKEN
diff --git a/init/init_task.c b/init/init_task.c
index ba0a7f36..11f83be1 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -22,5 +22,8 @@ EXPORT_SYMBOL(init_task);
  * Initial thread structure. Alignment of this is handled by a special
  * linker map entry.
  */
-union thread_union init_thread_union __init_task_data =
-	{ INIT_THREAD_INFO(init_task) };
+union thread_union init_thread_union __init_task_data = {
+#ifndef CONFIG_THREAD_INFO_IN_TASK
+	INIT_THREAD_INFO(init_task)
+#endif
+};
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c64fc51..3655c96 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1000,7 +1000,11 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu)
 	 * per-task data have been completed by this moment.
 	 */
 	smp_wmb();
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	p->cpu = cpu;
+#else
 	task_thread_info(p)->cpu = cpu;
+#endif
 	p->wake_cpu = cpu;
 #endif
 }

[toc] | [prev] | [next] | [standalone]

#1482795 — [PATCH 11/12] sched: Free the stack early if CONFIG_THREAD_INFO_IN_TASK

We currently keep every task's stack around until the task_struct
itself is freed.  This means that we keep the stack allocation alive
for longer than necessary and that, under load, we free stacks in
big batches whenever RCU drops the last task reference.  Neither of
these is good for reuse of cache-hot memory, and freeing in batches
prevents us from usefully caching small numbers of vmalloced stacks.

On architectures that have thread_info on the stack, we can't easily
change this, but on architectures that set THREAD_INFO_IN_TASK, we
can free it as soon as the task is dead.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 include/linux/init_task.h |  4 +++-
 include/linux/sched.h     | 14 ++++++++++++++
 kernel/fork.c             | 35 ++++++++++++++++++++++++++++++++++-
 kernel/sched/core.c       |  4 ++++
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 9c04d44eeb3c..325f649d77ff 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -186,7 +186,9 @@ extern struct task_group root_task_group;
 #endif
 
 #ifdef CONFIG_THREAD_INFO_IN_TASK
-# define INIT_TASK_TI(tsk) .thread_info = INIT_THREAD_INFO(tsk),
+# define INIT_TASK_TI(tsk)			\
+	.thread_info = INIT_THREAD_INFO(tsk),	\
+	.stack_refcount = ATOMIC_INIT(1),
 #else
 # define INIT_TASK_TI(tsk)
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a95867267e9f..abb795afc823 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1936,6 +1936,10 @@ struct task_struct {
 #ifdef CONFIG_VMAP_STACK
 	struct vm_struct *stack_vm_area;
 #endif
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	/* A live task holds one reference. */
+	atomic_t stack_refcount;
+#endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
 /*
@@ -3143,12 +3147,22 @@ static inline unsigned long *end_of_stack(struct task_struct *p)
 
 #endif
 
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+static inline void *try_get_task_stack(struct task_struct *tsk)
+{
+	return atomic_inc_not_zero(&tsk->stack_refcount) ?
+		task_stack_page(tsk) : NULL;
+}
+
+extern void put_task_stack(struct task_struct *tsk);
+#else
 static inline void *try_get_task_stack(struct task_struct *tsk)
 {
 	return task_stack_page(tsk);
 }
 
 static inline void put_task_stack(struct task_struct *tsk) {}
+#endif
 
 #define task_stack_end_corrupted(task) \
 		(*(end_of_stack(task)) != STACK_END_MAGIC)
diff --git a/kernel/fork.c b/kernel/fork.c
index 9b85f6b2cdcd..e0fd5446df58 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -269,11 +269,40 @@ static void account_kernel_stack(struct task_struct *tsk, int account)
 	}
 }
 
-void free_task(struct task_struct *tsk)
+static void release_task_stack(struct task_struct *tsk)
 {
 	account_kernel_stack(tsk, -1);
 	arch_release_thread_stack(tsk->stack);
 	free_thread_stack(tsk);
+	tsk->stack = NULL;
+#ifdef CONFIG_VMAP_STACK
+	tsk->stack_vm_area = NULL;
+#endif
+}
+
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+void put_task_stack(struct task_struct *tsk)
+{
+	if (atomic_dec_and_test(&tsk->stack_refcount))
+		release_task_stack(tsk);
+}
+#endif
+
+void free_task(struct task_struct *tsk)
+{
+#ifndef CONFIG_THREAD_INFO_IN_TASK
+	/*
+	 * The task is finally done with both the stack and thread_info,
+	 * so free both.
+	 */
+	release_task_stack(tsk);
+#else
+	/*
+	 * If the task had a separate stack allocation, it should be gone
+	 * by now.
+	 */
+	WARN_ON_ONCE(atomic_read(&tsk->stack_refcount) != 0);
+#endif
 	rt_mutex_debug_task_free(tsk);
 	ftrace_graph_exit_task(tsk);
 	put_seccomp_filter(tsk);
@@ -411,6 +440,9 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 #ifdef CONFIG_VMAP_STACK
 	tsk->stack_vm_area = stack_vm_area;
 #endif
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+	atomic_set(&tsk->stack_refcount, 1);
+#endif
 
 	if (err)
 		goto free_stack;
@@ -1750,6 +1782,7 @@ bad_fork_cleanup_count:
 	atomic_dec(&p->cred->user->processes);
 	exit_creds(p);
 bad_fork_free:
+	put_task_stack(p);
 	free_task(p);
 fork_out:
 	return ERR_PTR(retval);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3d91b63dd2f6..d61bfae91093 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2750,6 +2750,10 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 		 * task and put them back on the free list.
 		 */
 		kprobe_flush_task(prev);
+
+		/* Task is done with its stack. */
+		put_task_stack(prev);
+
 		put_task_struct(prev);
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web