Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965740AbdLSHud (ORCPT ); Tue, 19 Dec 2017 02:50:33 -0500 Received: from mail-wr0-f175.google.com ([209.85.128.175]:33796 "EHLO mail-wr0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935623AbdLSHua (ORCPT ); Tue, 19 Dec 2017 02:50:30 -0500 X-Google-Smtp-Source: ACJfBosldQ7raB+X/tA/+cnB8nGDqUNKTrqZcPw51SSTTWCh5Wkry1c6AZOpmPZvuJS7dVFtPcR2Qg== Date: Tue, 19 Dec 2017 08:50:26 +0100 From: Ingo Molnar To: Miroslav Benes Cc: Josh Poimboeuf , Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , live-patching@vger.kernel.org Subject: Re: stack traces and zombie tasks Message-ID: <20171219075026.hlczgvqnsfligmg7@gmail.com> References:

<20171218033408.whh7bkqfuaolrldm@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2876 Lines: 74 * Miroslav Benes 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 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//stack', they would get > > > > something like this > > > > > > > > [] do_exit+0x6f7/0xa80 > > > > [] do_group_exit+0x39/0xa0 > > > > [] __wake_up_parent+0x0/0x30 > > > > [] system_call_fastpath+0x16/0x1b > > > > [<00007fd128f9c4f9>] 0x7fd128f9c4f9 > > > > [] 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 Great. Josh, mind sending a changelogged version, or should I distill a commit out of this discussion, for tip:x86/urgent? Thanks, Ingo