On Thu, 09 Apr 2026 20:05:13 +0800 Menglong Dong <[email protected]> wrote:
> On 2026/4/9 18:35 Masami Hiramatsu (Google) <[email protected]> write: > > From: Masami Hiramatsu (Google) <[email protected]> > > > > fprobe_remove_node_in_module() is called under RCU read locked, but > > this invokes kcalloc() if there are more than 8 fprobes installed > > on the module. Sashiko warns it because kcalloc() can sleep [1]. > > > > [1] > > https://sashiko.dev/#/patchset/177552432201.853249.5125045538812833325.stgit%40mhiramat.tok.corp.google.com > > > > To fix this issue, expand the batch size to 128 and do not expand > > the fprobe_addr_list, but just cancel walking on fprobe_ip_table, > > update fgraph/ftrace_ops and retry the loop again. > > > > Fixes: 0de4c70d04a4 ("tracing: fprobe: use rhltable for fprobe_ip_table") > > Cc: [email protected] > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > > --- > > kernel/trace/fprobe.c | 53 > > ++++++++++++++++++------------------------------- > > 1 file changed, 19 insertions(+), 34 deletions(-) > > > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > > index 56d145017902..058cf6ef7ebb 100644 > > --- a/kernel/trace/fprobe.c > > +++ b/kernel/trace/fprobe.c > > @@ -536,7 +536,7 @@ static void fprobe_graph_remove_ips(unsigned long > > *addrs, int num) > > Hi, Masami. Thanks for the fixes. Overall, the whole series > LGTM. > > Some nits below. > > > > > #ifdef CONFIG_MODULES > > > [...] > > unsigned long val, void *data) > > @@ -591,6 +567,7 @@ static int fprobe_module_callback(struct notifier_block > > *nb, > > struct fprobe_hlist_node *node; > > struct rhashtable_iter iter; > > struct module *mod = data; > > + bool retry; > > > > if (val != MODULE_STATE_GOING) > > return NOTIFY_DONE; > > @@ -600,13 +577,19 @@ static int fprobe_module_callback(struct > > notifier_block *nb, > > if (!alist.addrs) > > return NOTIFY_DONE; > > The "retry" confuse me a little. How about we use "again" and "more" > here: > > +again: > + more = false; OK. And Sashiko pointed out that we can retry right after calling rhltable_walk_enter(), and it seems true according to https://lwn.net/Articles/751374/ We can seep inside rhltable_walk_enter()/exit() but not inside rhashtable_walk_start()/end(). So let me update it. > > > > > +retry: > > + retry = false; > > + alist.index = 0; > > mutex_lock(&fprobe_mutex); > > rhltable_walk_enter(&fprobe_ip_table, &iter); > > do { > > rhashtable_walk_start(&iter); > > > > while ((node = rhashtable_walk_next(&iter)) && !IS_ERR(node)) > > - fprobe_remove_node_in_module(mod, node, &alist); > > + if (fprobe_remove_node_in_module(mod, node, &alist) < > > 0) { > > + retry = true; > > + break; > > + } > > Wrap the code within the "while" with {}? OK. Thank you! > > Thanks! > Menglong Dong > > > > > rhashtable_walk_stop(&iter); > > } while (node == ERR_PTR(-EAGAIN)); > > @@ -615,6 +598,8 @@ static int fprobe_module_callback(struct notifier_block > > *nb, > > if (alist.index > 0) > > fprobe_set_ips(alist.addrs, alist.index, 1, 0); > > mutex_unlock(&fprobe_mutex); > > + if (retry) > > + goto retry; > > > > kfree(alist.addrs); > > > > > > > > > > > > -- Masami Hiramatsu (Google) <[email protected]>
