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;
>
> +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 {}?
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);
>
>
>
>
