From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755182AbdIGNH3 (ORCPT ); Thu, 7 Sep 2017 09:07:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53326 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754005AbdIGNH2 (ORCPT ); Thu, 7 Sep 2017 09:07:28 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 46F72828 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jpoimboe@redhat.com Date: Thu, 7 Sep 2017 08:07:26 -0500 From: Josh Poimboeuf To: Vlastimil Babka Cc: Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , x86@kernel.org, Miroslav Benes , linux-kernel@vger.kernel.org Subject: Re: [PATCH] x86, stacktrace: avoid recording save_stack_trace wrappers Message-ID: <20170907130726.c34xz7aszy3uiuug@treble> References: <20170907075736.11551-1-vbabka@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170907075736.11551-1-vbabka@suse.cz> User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 07 Sep 2017 13:07:28 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 07, 2017 at 09:57:36AM +0200, Vlastimil Babka wrote: > The save_stack_trace() and save_stack_trace_tsk() wrappers of > __save_stack_trace() add themselves to the call stack, and thus appear in the > recorded stacktraces. This is redundant and wasteful when we have limited space > to record the useful part of the backtrace with e.g. page_owner functionality. > > Fix this by making sure __save_stack_trace() is noinline (which matches the > current gcc decision) and bumping the skip in the wrappers. This is similar > to what was done for arm in 3683f44c42e9 ("ARM: stacktrace: avoid listing > stacktrace functions in stacktrace") and is pending for arm64. > > Signed-off-by: Vlastimil Babka > --- > arch/x86/kernel/stacktrace.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c > index 8dabd7bf1673..4b2fd6092739 100644 > --- a/arch/x86/kernel/stacktrace.c > +++ b/arch/x86/kernel/stacktrace.c > @@ -30,7 +30,7 @@ static int save_stack_address(struct stack_trace *trace, unsigned long addr, > return 0; > } > > -static void __save_stack_trace(struct stack_trace *trace, > +static void noinline __save_stack_trace(struct stack_trace *trace, > struct task_struct *task, struct pt_regs *regs, > bool nosched) > { > @@ -56,6 +56,7 @@ static void __save_stack_trace(struct stack_trace *trace, > */ > void save_stack_trace(struct stack_trace *trace) > { > + trace->skip++; > __save_stack_trace(trace, current, NULL, false); > } > EXPORT_SYMBOL_GPL(save_stack_trace); > @@ -70,6 +71,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > if (!try_get_task_stack(tsk)) > return; > > + trace->skip++; > __save_stack_trace(trace, tsk, NULL, true); > > put_task_stack(tsk); save_stack_trace_tsk() is usually called for other tasks, in which case these functions aren't on the stack. So the skip should only be done when task == current. Also, I think __save_stack_trace_reliable() should be made __always_inline (which matches its current GCC behavior) so that it doesn't accidentally get this problem in the future. -- Josh