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

Groups > linux.kernel > #1727998 > unrolled thread

[PATCH] x86, stacktrace: avoid recording save_stack_trace wrappers

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86, stacktrace: avoid recording save_stack_trace wrappers Vlastimil Babka <vbabka@suse.cz> - 2017-09-07 10:00 +0200
    Re: [PATCH] x86, stacktrace: avoid recording save_stack_trace  wrappers Josh Poimboeuf <jpoimboe@redhat.com> - 2017-09-07 15:10 +0200

#1727998 — [PATCH] x86, stacktrace: avoid recording save_stack_trace wrappers

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 <vbabka@suse.cz>
---
 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);
-- 
2.14.1

[toc] | [next] | [standalone]

#1728214 — Re: [PATCH] x86, stacktrace: avoid recording save_stack_trace wrappers

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 <vbabka@suse.cz>
> ---
>  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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web