bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Tao Chen <chen.dylane@linux.dev>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	 martin.lau@linux.dev, eddyz87@gmail.com, song@kernel.org,
	 yonghong.song@linux.dev, john.fastabend@gmail.com,
	kpsingh@kernel.org,  sdf@fomichev.me, haoluo@google.com,
	jolsa@kernel.org,  mattbobrowski@google.com, rostedt@goodmis.org,
	mhiramat@kernel.org,  mathieu.desnoyers@efficios.com,
	bpf@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v2] bpf: Remove migrate_disable in kprobe_multi_link_prog_run
Date: Tue, 12 Aug 2025 15:05:40 -0700	[thread overview]
Message-ID: <CAEf4BzZduEdBCzm56zwgrHpzV=CsMbzfVi5oR9w3H4vUQL6FYw@mail.gmail.com> (raw)
In-Reply-To: <20250805162732.1896687-1-chen.dylane@linux.dev>

On Tue, Aug 5, 2025 at 9:28 AM Tao Chen <chen.dylane@linux.dev> wrote:
>
> bpf program should run under migration disabled, kprobe_multi_link_prog_run
> called all the way from graph tracer, which disables preemption in
> function_graph_enter_regs, as Jiri and Yonghong suggested, there is no
> need to use migrate_disable. As a result, some overhead maybe will be
> reduced.
>
> Fixes: 0dcac2725406 ("bpf: Add multi kprobe link")
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> ---
>  kernel/trace/bpf_trace.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> Change list:
>  v1 -> v2:
>   - s/called the way/called all the way/.(Jiri)
>  v1: https://lore.kernel.org/bpf/f7acfd22-bcf3-4dff-9a87-7c1e6f84ce9c@linux.dev
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 3ae52978cae..5701791e3cb 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2734,14 +2734,19 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,

even though bpf_prog_run() eventually calls cant_migrate(), we should
add it before that __this_cpu_inc_return() call as well, because that
one is relying on that non-migration independently from bpf_prog_run()

>                 goto out;
>         }
>
> -       migrate_disable();
> +       /*
> +        * bpf program should run under migration disabled, kprobe_multi_link_prog_run
> +        * called all the way from graph tracer, which disables preemption in
> +        * function_graph_enter_regs, so there is no need to use migrate_disable.
> +        * Accessing the above percpu data bpf_prog_active is also safe for the same
> +        * reason.
> +        */

let's shorten this a bit to something like:

/* graph tracer framework ensures we won't migrate */
cant_migrate();

all the other stuff in the comment can become outdated way too easily
and/or is sort of general BPF implementation knowledge

pw-bot: cr


>         rcu_read_lock();
>         regs = ftrace_partial_regs(fregs, bpf_kprobe_multi_pt_regs_ptr());
>         old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
>         err = bpf_prog_run(link->link.prog, regs);
>         bpf_reset_run_ctx(old_run_ctx);
>         rcu_read_unlock();
> -       migrate_enable();
>
>   out:
>         __this_cpu_dec(bpf_prog_active);
> --
> 2.48.1
>

  reply	other threads:[~2025-08-12 22:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-05 16:27 [PATCH bpf-next v2] bpf: Remove migrate_disable in kprobe_multi_link_prog_run Tao Chen
2025-08-12 22:05 ` Andrii Nakryiko [this message]
2025-08-13 11:54   ` Tao Chen
2025-08-14 11:35   ` Tao Chen

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='CAEf4BzZduEdBCzm56zwgrHpzV=CsMbzfVi5oR9w3H4vUQL6FYw@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chen.dylane@linux.dev \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mattbobrowski@google.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /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).