* stack traces and zombie tasks
@ 2017-12-15 12:54 Miroslav Benes
2017-12-15 15:51 ` Andy Lutomirski
0 siblings, 1 reply; 6+ messages in thread
From: Miroslav Benes @ 2017-12-15 12:54 UTC (permalink / raw)
To: luto, jpoimboe; +Cc: x86, linux-kernel, live-patching
Hi,
commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping
it") slightly changed the behaviour of stack traces dumping for zombie
tasks.
Before the commit (well, this is older SLE12 kernel, but that should not
matter), if one called 'cat /proc/<zombie pid>/stack', they would get
something like this
[<ffffffff8105b877>] do_exit+0x6f7/0xa80
[<ffffffff8105bc79>] do_group_exit+0x39/0xa0
[<ffffffff8105bcf0>] __wake_up_parent+0x0/0x30
[<ffffffff8152dd09>] system_call_fastpath+0x16/0x1b
[<00007fd128f9c4f9>] 0x7fd128f9c4f9
[<ffffffffffffffff>] 0xffffffffffffffff
After, one gets nothing. The trace is empty. try_get_task_stack() contains
atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on
x86_64) and because stack_refcount is 0 for a zombie task, it returns
NULL. Therefore, all save_stack_trace_*() functions return immediately.
I guess that no one has cared about it so far. There is a problem for
live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for
the zombie task and its stack is deemed unreliable. It could block our
transition for quite a long time.
We can skip those tasks in kernel/livepatch/ with a simple test we have in
kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0).
But you may want to change it generally, so better to ask first.
Regards,
Miroslav
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: stack traces and zombie tasks 2017-12-15 12:54 stack traces and zombie tasks Miroslav Benes @ 2017-12-15 15:51 ` Andy Lutomirski 2017-12-18 3:34 ` Josh Poimboeuf 0 siblings, 1 reply; 6+ messages in thread From: Andy Lutomirski @ 2017-12-15 15:51 UTC (permalink / raw) To: Miroslav Benes Cc: Andrew Lutomirski, Josh Poimboeuf, X86 ML, linux-kernel, live-patching On Fri, Dec 15, 2017 at 4:54 AM, Miroslav Benes <mbenes@suse.cz> wrote: > Hi, > > commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping > it") slightly changed the behaviour of stack traces dumping for zombie > tasks. > > Before the commit (well, this is older SLE12 kernel, but that should not > matter), if one called 'cat /proc/<zombie pid>/stack', they would get > something like this > > [<ffffffff8105b877>] do_exit+0x6f7/0xa80 > [<ffffffff8105bc79>] do_group_exit+0x39/0xa0 > [<ffffffff8105bcf0>] __wake_up_parent+0x0/0x30 > [<ffffffff8152dd09>] system_call_fastpath+0x16/0x1b > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > [<ffffffffffffffff>] 0xffffffffffffffff > > After, one gets nothing. The trace is empty. try_get_task_stack() contains > atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on > x86_64) and because stack_refcount is 0 for a zombie task, it returns > NULL. Therefore, all save_stack_trace_*() functions return immediately. > > I guess that no one has cared about it so far. There is a problem for > live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for > the zombie task and its stack is deemed unreliable. It could block our > transition for quite a long time. > > We can skip those tasks in kernel/livepatch/ with a simple test we have in > kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0). > But you may want to change it generally, so better to ask first. > Sounds like a bug in save_stack_trace_tsk_reliable() to me: if the task has no stack, then the trace is 100% definitely empty :) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: stack traces and zombie tasks 2017-12-15 15:51 ` Andy Lutomirski @ 2017-12-18 3:34 ` Josh Poimboeuf 2017-12-18 13:07 ` Miroslav Benes 0 siblings, 1 reply; 6+ messages in thread From: Josh Poimboeuf @ 2017-12-18 3:34 UTC (permalink / raw) To: Andy Lutomirski; +Cc: Miroslav Benes, X86 ML, linux-kernel, live-patching On Fri, Dec 15, 2017 at 07:51:45AM -0800, Andy Lutomirski wrote: > On Fri, Dec 15, 2017 at 4:54 AM, Miroslav Benes <mbenes@suse.cz> wrote: > > Hi, > > > > commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping > > it") slightly changed the behaviour of stack traces dumping for zombie > > tasks. > > > > Before the commit (well, this is older SLE12 kernel, but that should not > > matter), if one called 'cat /proc/<zombie pid>/stack', they would get > > something like this > > > > [<ffffffff8105b877>] do_exit+0x6f7/0xa80 > > [<ffffffff8105bc79>] do_group_exit+0x39/0xa0 > > [<ffffffff8105bcf0>] __wake_up_parent+0x0/0x30 > > [<ffffffff8152dd09>] system_call_fastpath+0x16/0x1b > > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > > [<ffffffffffffffff>] 0xffffffffffffffff > > > > After, one gets nothing. The trace is empty. try_get_task_stack() contains > > atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on > > x86_64) and because stack_refcount is 0 for a zombie task, it returns > > NULL. Therefore, all save_stack_trace_*() functions return immediately. > > > > I guess that no one has cared about it so far. There is a problem for > > live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for > > the zombie task and its stack is deemed unreliable. It could block our > > transition for quite a long time. > > > > We can skip those tasks in kernel/livepatch/ with a simple test we have in > > kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0). > > But you may want to change it generally, so better to ask first. > > > > Sounds like a bug in save_stack_trace_tsk_reliable() to me: if the > task has no stack, then the trace is 100% definitely empty :) I would agree with that, something like the following should fix it? diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 77835bc021c7..20161ef53537 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -164,8 +164,12 @@ int save_stack_trace_tsk_reliable(struct task_struct *tsk, { int ret; + /* + * If the task doesn't have a stack (e.g., a zombie), the stack is + * "reliably" empty. + */ if (!try_get_task_stack(tsk)) - return -EINVAL; + return 0; ret = __save_stack_trace_reliable(trace, tsk); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: stack traces and zombie tasks 2017-12-18 3:34 ` Josh Poimboeuf @ 2017-12-18 13:07 ` Miroslav Benes 2017-12-19 7:50 ` Ingo Molnar 0 siblings, 1 reply; 6+ messages in thread From: Miroslav Benes @ 2017-12-18 13:07 UTC (permalink / raw) To: Josh Poimboeuf; +Cc: Andy Lutomirski, X86 ML, linux-kernel, live-patching On Sun, 17 Dec 2017, Josh Poimboeuf wrote: > On Fri, Dec 15, 2017 at 07:51:45AM -0800, Andy Lutomirski wrote: > > On Fri, Dec 15, 2017 at 4:54 AM, Miroslav Benes <mbenes@suse.cz> wrote: > > > Hi, > > > > > > commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping > > > it") slightly changed the behaviour of stack traces dumping for zombie > > > tasks. > > > > > > Before the commit (well, this is older SLE12 kernel, but that should not > > > matter), if one called 'cat /proc/<zombie pid>/stack', they would get > > > something like this > > > > > > [<ffffffff8105b877>] do_exit+0x6f7/0xa80 > > > [<ffffffff8105bc79>] do_group_exit+0x39/0xa0 > > > [<ffffffff8105bcf0>] __wake_up_parent+0x0/0x30 > > > [<ffffffff8152dd09>] system_call_fastpath+0x16/0x1b > > > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > > > [<ffffffffffffffff>] 0xffffffffffffffff > > > > > > After, one gets nothing. The trace is empty. try_get_task_stack() contains > > > atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on > > > x86_64) and because stack_refcount is 0 for a zombie task, it returns > > > NULL. Therefore, all save_stack_trace_*() functions return immediately. > > > > > > I guess that no one has cared about it so far. There is a problem for > > > live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for > > > the zombie task and its stack is deemed unreliable. It could block our > > > transition for quite a long time. > > > > > > We can skip those tasks in kernel/livepatch/ with a simple test we have in > > > kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0). > > > But you may want to change it generally, so better to ask first. > > > > > > > Sounds like a bug in save_stack_trace_tsk_reliable() to me: if the > > task has no stack, then the trace is 100% definitely empty :) > > I would agree with that, something like the following should fix it? > > diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c > index 77835bc021c7..20161ef53537 100644 > --- a/arch/x86/kernel/stacktrace.c > +++ b/arch/x86/kernel/stacktrace.c > @@ -164,8 +164,12 @@ int save_stack_trace_tsk_reliable(struct task_struct *tsk, > { > int ret; > > + /* > + * If the task doesn't have a stack (e.g., a zombie), the stack is > + * "reliably" empty. > + */ > if (!try_get_task_stack(tsk)) > - return -EINVAL; > + return 0; > > ret = __save_stack_trace_reliable(trace, tsk); This obviously fixes the problem, so you can add Reported-and-tested-by: Miroslav Benes <mbenes@suse.cz> Thanks Miroslav ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: stack traces and zombie tasks 2017-12-18 13:07 ` Miroslav Benes @ 2017-12-19 7:50 ` Ingo Molnar 2017-12-19 8:00 ` Ingo Molnar 0 siblings, 1 reply; 6+ messages in thread From: Ingo Molnar @ 2017-12-19 7:50 UTC (permalink / raw) To: Miroslav Benes Cc: Josh Poimboeuf, Andy Lutomirski, X86 ML, linux-kernel, live-patching * Miroslav Benes <mbenes@suse.cz> wrote: > On Sun, 17 Dec 2017, Josh Poimboeuf wrote: > > > On Fri, Dec 15, 2017 at 07:51:45AM -0800, Andy Lutomirski wrote: > > > On Fri, Dec 15, 2017 at 4:54 AM, Miroslav Benes <mbenes@suse.cz> wrote: > > > > Hi, > > > > > > > > commit 1959a60182f4 ("x86/dumpstack: Pin the target stack when dumping > > > > it") slightly changed the behaviour of stack traces dumping for zombie > > > > tasks. > > > > > > > > Before the commit (well, this is older SLE12 kernel, but that should not > > > > matter), if one called 'cat /proc/<zombie pid>/stack', they would get > > > > something like this > > > > > > > > [<ffffffff8105b877>] do_exit+0x6f7/0xa80 > > > > [<ffffffff8105bc79>] do_group_exit+0x39/0xa0 > > > > [<ffffffff8105bcf0>] __wake_up_parent+0x0/0x30 > > > > [<ffffffff8152dd09>] system_call_fastpath+0x16/0x1b > > > > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > > > > [<ffffffffffffffff>] 0xffffffffffffffff > > > > > > > > After, one gets nothing. The trace is empty. try_get_task_stack() contains > > > > atomic_inc_not_zero() (CONFIG_THREAD_INFO_IN_TASK is now default on > > > > x86_64) and because stack_refcount is 0 for a zombie task, it returns > > > > NULL. Therefore, all save_stack_trace_*() functions return immediately. > > > > > > > > I guess that no one has cared about it so far. There is a problem for > > > > live patching though. save_stack_trace_tsk_reliable() returns -EINVAL for > > > > the zombie task and its stack is deemed unreliable. It could block our > > > > transition for quite a long time. > > > > > > > > We can skip those tasks in kernel/livepatch/ with a simple test we have in > > > > kGraft. Skip the task if (task->state == TASK_DEAD && task->on_cpu == 0). > > > > But you may want to change it generally, so better to ask first. > > > > > > > > > > Sounds like a bug in save_stack_trace_tsk_reliable() to me: if the > > > task has no stack, then the trace is 100% definitely empty :) > > > > I would agree with that, something like the following should fix it? > > > > diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c > > index 77835bc021c7..20161ef53537 100644 > > --- a/arch/x86/kernel/stacktrace.c > > +++ b/arch/x86/kernel/stacktrace.c > > @@ -164,8 +164,12 @@ int save_stack_trace_tsk_reliable(struct task_struct *tsk, > > { > > int ret; > > > > + /* > > + * If the task doesn't have a stack (e.g., a zombie), the stack is > > + * "reliably" empty. > > + */ > > if (!try_get_task_stack(tsk)) > > - return -EINVAL; > > + return 0; > > > > ret = __save_stack_trace_reliable(trace, tsk); > > This obviously fixes the problem, so you can add > > Reported-and-tested-by: Miroslav Benes <mbenes@suse.cz> Great. Josh, mind sending a changelogged version, or should I distill a commit out of this discussion, for tip:x86/urgent? Thanks, Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: stack traces and zombie tasks 2017-12-19 7:50 ` Ingo Molnar @ 2017-12-19 8:00 ` Ingo Molnar 0 siblings, 0 replies; 6+ messages in thread From: Ingo Molnar @ 2017-12-19 8:00 UTC (permalink / raw) To: Miroslav Benes Cc: Josh Poimboeuf, Andy Lutomirski, X86 ML, linux-kernel, live-patching * Ingo Molnar <mingo@kernel.org> wrote: > > This obviously fixes the problem, so you can add > > > > Reported-and-tested-by: Miroslav Benes <mbenes@suse.cz> > > Great. > > Josh, mind sending a changelogged version, or should I distill a commit out of > this discussion, for tip:x86/urgent? ... never mind, later in my mbox I found the patch that Josh already submitted! Thanks, Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-19 8:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-15 12:54 stack traces and zombie tasks Miroslav Benes 2017-12-15 15:51 ` Andy Lutomirski 2017-12-18 3:34 ` Josh Poimboeuf 2017-12-18 13:07 ` Miroslav Benes 2017-12-19 7:50 ` Ingo Molnar 2017-12-19 8:00 ` Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome