From: Yafang Shao <laoar.shao@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
kafai@fb.com, songliubraving@fb.com, yhs@fb.com,
john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
haoluo@google.com, jolsa@kernel.org, rostedt@goodmis.org,
mhiramat@kernel.org, bpf@vger.kernel.org,
linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next 5/6] bpf: Improve tracing recursion prevention mechanism
Date: Tue, 18 Apr 2023 09:49:14 +0800 [thread overview]
Message-ID: <CALOAHbC4Bz_VX52zmv=sScBf0hzscMAC4+EwMCpnd1BcaSVJSw@mail.gmail.com> (raw)
In-Reply-To: <20230417201457.c43xfcukjzm4u6vx@dhcp-172-26-102-232.dhcp.thefacebook.com>
On Tue, Apr 18, 2023 at 4:15 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Apr 17, 2023 at 03:47:36PM +0000, Yafang Shao wrote:
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > index f61d513..3df39a5 100644
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -842,15 +842,21 @@ static __always_inline u64 notrace bpf_prog_start_time(void)
> > static u64 notrace __bpf_prog_enter_recur(struct bpf_prog *prog, struct bpf_tramp_run_ctx *run_ctx)
> > __acquires(RCU)
> > {
> > - rcu_read_lock();
> > - migrate_disable();
> > -
> > - run_ctx->saved_run_ctx = bpf_set_run_ctx(&run_ctx->run_ctx);
> > + int bit;
> >
> > - if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
> > + rcu_read_lock();
> > + bit = test_recursion_try_acquire(_THIS_IP_, _RET_IP_);
>
> and bpf will prevent ftrace to run and vice versa.
> Not a good idea.
>
> One bpf prog will prevent different bpf prog to run since they share current task.
> Not a good idea either.
That shouldn't happen. test_recursion_try_acquire() uses a
per-task_struct value. One single task_struct can't run in parallel,
right?
Note that the bpf program running in softirq or irq context won't be
prevented by it.
IIUC, the bpf program should run in serial in one single task, right?
That said, one bpf program can only run after another bpf program
finished in the same task?
--
Regards
Yafang
next prev parent reply other threads:[~2023-04-18 1:51 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-17 15:47 [PATCH bpf-next 0/6] bpf: Tracing recursion prevention mechanism improvement Yafang Shao
2023-04-17 15:47 ` [PATCH bpf-next 1/6] bpf: Add __rcu_read_{lock,unlock} into btf id deny list Yafang Shao
2023-04-17 15:47 ` [PATCH bpf-next 2/6] tracing: Add generic test_recursion_try_acquire() Yafang Shao
2023-04-20 6:51 ` Masami Hiramatsu
2023-04-17 15:47 ` [PATCH bpf-next 3/6] tracing: Add the comment for allowing one single recursion in process context Yafang Shao
2023-04-17 15:47 ` [PATCH bpf-next 4/6] selftests/bpf: Allow one single recursion in fentry recursion test Yafang Shao
2023-04-17 15:47 ` [PATCH bpf-next 5/6] bpf: Improve tracing recursion prevention mechanism Yafang Shao
2023-04-17 20:14 ` Alexei Starovoitov
2023-04-18 1:49 ` Yafang Shao [this message]
2023-04-18 15:38 ` Alexei Starovoitov
2023-04-19 11:46 ` Yafang Shao
[not found] ` <CAADnVQ+FO-+1OALTtgVkcpH3Adc6xS9qjzORyq2vwVtwY2UoxQ@mail.gmail.com>
2023-04-24 21:40 ` Steven Rostedt
2023-04-27 9:57 ` Yafang Shao
2023-04-27 12:15 ` Yafang Shao
2023-04-27 12:35 ` Yafang Shao
2023-04-17 23:29 ` kernel test robot
2023-04-27 13:26 ` Steven Rostedt
2023-04-27 14:22 ` Yafang Shao
2023-04-27 15:18 ` Steven Rostedt
2023-04-27 15:23 ` Yafang Shao
2023-04-27 15:36 ` Steven Rostedt
2023-04-27 15:39 ` Alexei Starovoitov
2023-04-27 15:43 ` Yafang Shao
2023-04-27 15:46 ` Steven Rostedt
2023-04-17 15:47 ` [PATCH bpf-next 6/6] bpf: Remove some denied functions from the btf id deny list Yafang Shao
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='CALOAHbC4Bz_VX52zmv=sScBf0hzscMAC4+EwMCpnd1BcaSVJSw@mail.gmail.com' \
--to=laoar.shao@gmail.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=sdf@google.com \
--cc=songliubraving@fb.com \
--cc=yhs@fb.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).