Re: [PATCH 3/4] tracing: fprobe-events: Register fprobe-events only when it is enabled
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-03-25 18:40:28
Also in:
lkml
On Sun, 16 Mar 2025 21:21:42 +0900 "Masami Hiramatsu (Google)" [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Masami Hiramatsu (Google) <mhiramat@kernel.org> Currently fprobe events are registered when it is defined. Thus it will give some overhead even if it is disabled. This changes it to register the fprobe only when it is enabled. Suggested-by: Steven Rostedt <rostedt@goodmis.org> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> --- include/linux/fprobe.h | 8 + kernel/trace/fprobe.c | 29 +++-- kernel/trace/trace_fprobe.c | 234 +++++++++++++++++++++---------------------- 3 files changed, 140 insertions(+), 131 deletions(-)diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h index 702099f08929..9635a24d5a25 100644 --- a/include/linux/fprobe.h +++ b/include/linux/fprobe.h@@ -94,6 +94,8 @@ int register_fprobe_ips(struct fprobe *fp, unsigned long *addrs, int num); int register_fprobe_syms(struct fprobe *fp, const char **syms, int num); int unregister_fprobe(struct fprobe *fp); bool fprobe_is_registered(struct fprobe *fp); +int fprobe_alloc_ip_list_from_filter(const char *filter, const char *notfilter, + unsigned long **addrs); #else static inline int register_fprobe(struct fprobe *fp, const char *filter, const char *notfilter) {@@ -115,6 +117,12 @@ static inline bool fprobe_is_registered(struct fprobe *fp) { return false; } +static inline int fprobe_alloc_ip_list_from_filter(const char *filter, + const char *notfilter, + unsigned long **addrs) +{ + return -EOPNOTSUPP; +} #endif /**diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 33082c4e8154..05050f1c2239 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c@@ -486,6 +486,24 @@ static int ip_list_from_filter(const char *filter, const char *notfilter, return match.index ?: -ENOENT; } +#define FPROBE_IPS_MAX INT_MAX + +int fprobe_alloc_ip_list_from_filter(const char *filter, const char *notfilter, + unsigned long **addrs) +{ + int ret; + + /* Count the number of ips from filter. */ + ret = ip_list_from_filter(filter, notfilter, NULL, FPROBE_IPS_MAX); + if (ret < 0) + return ret; + + *addrs = kcalloc(ret, sizeof(unsigned long), GFP_KERNEL); + if (!*addrs) + return -ENOMEM; + return ip_list_from_filter(filter, notfilter, *addrs, ret);
This was in the old code, but I'm wondering. Does this code prevent modules from being loaded and unloaded too? I'm asking because if we call the first instance of ip_list_from_filter() and it finds a list of functions from a module, and then that module is unloaded, the ip_list_from_filter() will return a failure, and *addrs would be a memory leak. -- Steve
quoted hunk ↗ jump to hunk
+} + static void fprobe_fail_cleanup(struct fprobe *fp) { kfree(fp->hlist_array);@@ -528,8 +546,6 @@ static int fprobe_init(struct fprobe *fp, unsigned long *addrs, int num) return 0; } -#define FPROBE_IPS_MAX INT_MAX - /** * register_fprobe() - Register fprobe to ftrace by pattern. * @fp: A fprobe data structure to be registered.@@ -549,14 +565,7 @@ int register_fprobe(struct fprobe *fp, const char *filter, const char *notfilter if (!fp || !filter) return -EINVAL; - ret = ip_list_from_filter(filter, notfilter, NULL, FPROBE_IPS_MAX); - if (ret < 0) - return ret; - - addrs = kcalloc(ret, sizeof(unsigned long), GFP_KERNEL); - if (!addrs) - return -ENOMEM; - ret = ip_list_from_filter(filter, notfilter, addrs, ret); + ret = fprobe_alloc_ip_list_from_filter(filter, notfilter, &addrs); if (ret > 0) ret = register_fprobe_ips(fp, addrs, ret);