Re: [PATCH v2 1/2] tracing/fprobe: Avoid kcalloc() in rcu_read_lock section

[Menglong Dong <menglong.dong@linux.dev>]

On 2026/4/9 18:35 Masami Hiramatsu (Google) <mhiramat@kernel.org> write:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 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: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  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);
>  
> 
> 
>