On Wed, 15 Apr 2026 17:47:11 +0800 Menglong Dong <[email protected]> wrote:
> On 2026/4/14 17:14 Masami Hiramatsu (Google) <[email protected]> write: > > From: Masami Hiramatsu (Google) <[email protected]> > > > > When register_fprobe_ips() fails, it tries to remove a list of > > fprobe_hash_node from fprobe_ip_table, but it missed to remove > > fprobe itself from fprobe_table. Moreover, when removing > > the fprobe_hash_node which is added to rhltable once, it must > > use kfree_rcu() after removing from rhltable. > > > > To fix these issues, this reuses unregister_fprobe() internal > > code to rollback the half-way registered fprobe. > > > > Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer") > > Cc: [email protected] > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > > --- > [...] > > > > +static int unregister_fprobe_nolock(struct fprobe *fp, bool force); > > + > > /** > > * register_fprobe_ips() - Register fprobe to ftrace by address. > > * @fp: A fprobe data structure to be registered. > > @@ -847,29 +855,26 @@ int register_fprobe_ips(struct fprobe *fp, unsigned > > long *addrs, int num) > > if (ret) > > return ret; > > Hi, Masami. The logic of unregister_fprobe_nolock() looks a little > messy. How about we make the logic here like this: > > for (i = 0; i < hlist_array->size; i++) { > // The node->fp is NULL, so it's safe to add the node before > // fprobe_ftrace_add_ips(), right? > ret = insert_fprobe_node(&hlist_array->array[i], fp); > if (ret) > goto fallback_err; > } > > if (fprobe_is_ftrace(fp)) > ret = fprobe_ftrace_add_ips(addrs, num); > else > ret = fprobe_graph_add_ips(addrs, num); > if (ret) > goto fallback_err; > > add_fprobe_hash(fp); > for (i = 0; i < hlist_array->size; i++) > WRITE_ONCE(hlist_array->array[i].fp, fp); > > return 0; > > fallback_err: > for (i--; i >= 0; i--) > delete_fprobe_node(&hlist_array->array[i]); > fprobe_fail_cleanup(fp); > return ret; > > Then, we don't need to change unregister_fprobe_nolock and > insert_fprobe_node. Thanks for the idea, but I don't like repeat it. It is better to do the same thing(unregister) in the same code. Above seems a bit optimized for fixing a problem. (Maybe revisit it later for optimization) Thank you, -- Masami Hiramatsu (Google) <[email protected]>
