linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Kui-Feng Lee <thinker.li@gmail.com>,
	 mhiramat@kernel.org,  martin.lau@linux.dev,
	 kernel-team@meta.com,  andrii@kernel.org,
	 linux-trace-kernel@vger.kernel.org,  bpf@vger.kernel.org
Cc: sinquersw@gmail.com,  kuifeng@meta.com,
	 Kui-Feng Lee <thinker.li@gmail.com>
Subject: RE: [PATCH bpf-next] rethook: Remove warning messages printed for finding return address of a frame.
Date: Wed, 03 Apr 2024 15:15:29 -0700	[thread overview]
Message-ID: <660dd501ee0da_21448208d5@john.notmuch> (raw)
In-Reply-To: <20240401191621.758056-1-thinker.li@gmail.com>

Kui-Feng Lee wrote:
> rethook_find_ret_addr() prints a warning message and returns 0 when the
> target task is running and not the "current" task to prevent returning an
> incorrect return address. However, this check is incomplete as the target
> task can still transition to the running state when finding the return
> address, although it is safe with RCU.
> 
> The issue we encounter is that the kernel frequently prints warning
> messages when BPF profiling programs call to bpf_get_task_stack() on
> running tasks.
> 
> The callers should be aware and willing to take the risk of receiving an
> incorrect return address from a task that is currently running other than
> the "current" one. A warning is not needed here as the callers are intent
> on it.
> 
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>  kernel/trace/rethook.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index fa03094e9e69..4297a132a7ae 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -248,7 +248,7 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
>  	if (WARN_ON_ONCE(!cur))
>  		return 0;
>  
> -	if (WARN_ON_ONCE(tsk != current && task_is_running(tsk)))
> +	if (tsk != current && task_is_running(tsk))
>  		return 0;
>  
>  	do {
> -- 
> 2.34.1
> 
> 

Acked-by: John Fastabend <john.fastabend@gmail.com>

      parent reply	other threads:[~2024-04-03 22:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01 19:16 [PATCH bpf-next] rethook: Remove warning messages printed for finding return address of a frame Kui-Feng Lee
2024-04-02 16:58 ` Andrii Nakryiko
2024-04-03 14:36   ` Daniel Borkmann
2024-04-08  1:13     ` Masami Hiramatsu
2024-04-08 17:16       ` Kui-Feng Lee
2024-04-03 22:15 ` John Fastabend [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=660dd501ee0da_21448208d5@john.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=kernel-team@meta.com \
    --cc=kuifeng@meta.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mhiramat@kernel.org \
    --cc=sinquersw@gmail.com \
    --cc=thinker.li@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).