On Wed, Jan 8, 2025 at 1:05 AM Sebastian Andrzej Siewior <[email protected]> wrote: > > __module_address() can be invoked within a RCU section, there is no > requirement to have preemption disabled. > > Replace the preempt_disable() section around __module_address() with > RCU. > > Cc: Alexei Starovoitov <[email protected]> > Cc: Andrii Nakryiko <[email protected]> > Cc: Daniel Borkmann <[email protected]> > Cc: Eduard Zingerman <[email protected]> > Cc: Hao Luo <[email protected]> > Cc: Jiri Olsa <[email protected]> > Cc: John Fastabend <[email protected]> > Cc: KP Singh <[email protected]> > Cc: Martin KaFai Lau <[email protected]> > Cc: Masami Hiramatsu <[email protected]> > Cc: Mathieu Desnoyers <[email protected]> > Cc: Matt Bobrowski <[email protected]> > Cc: Song Liu <[email protected]> > Cc: Stanislav Fomichev <[email protected]> > Cc: Steven Rostedt <[email protected]> > Cc: Yonghong Song <[email protected]> > Cc: [email protected] > Cc: [email protected] > Acked-by: Peter Zijlstra (Intel) <[email protected]> > Signed-off-by: Sebastian Andrzej Siewior <[email protected]> > --- > kernel/trace/bpf_trace.c | 19 ++++++++----------- > 1 file changed, 8 insertions(+), 11 deletions(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 1b8db5aee9d38..020df7b6ff90c 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -2336,10 +2336,9 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map > *btp) > { > struct module *mod; > > - preempt_disable(); > + guard(rcu)(); > mod = __module_address((unsigned long)btp); > module_put(mod); > - preempt_enable(); > } > > static __always_inline > @@ -2907,16 +2906,14 @@ static int get_modules_for_addrs(struct module > ***mods, unsigned long *addrs, u3 > for (i = 0; i < addrs_cnt; i++) { > struct module *mod; > > - preempt_disable(); > - mod = __module_address(addrs[i]); > - /* Either no module or we it's already stored */ > - if (!mod || has_module(&arr, mod)) { > - preempt_enable(); > - continue; > + scoped_guard(rcu) { > + mod = __module_address(addrs[i]); > + /* Either no module or we it's already stored */ > + if (!mod || has_module(&arr, mod)) > + continue; > + if (!try_module_get(mod)) > + err = -EINVAL;
lgtm. Should we take into bpf-next or the whole set is handled together somewhere?
