Re: [PATCH v2] libtracefs: Have tracefs_dynevent_get_all() find kprobes and uprobes properly
Metin Kaya <[email protected]>
| Newsgroups | org.kernel.vger.linux-trace-devel |
|---|---|
| Message-ID | <[email protected]> |
On 16/10/2024 3:47 pm, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" <[email protected]> > > The function tracefs_dynevent_get_all() will only look at the > dynamic_events file if it exists to find matching probes. But because > uprobes and kprobes both use the same prefix "p" as well as uretprobes and > kretprobes (with prefix "r"), it cannot use the dynamic_events file to > differentiate between the two. > > Have kprobes and uprobes always use their own files (kprobe_events and > uprobe_events) even if dynamic_events file exists. > > Also cleaned up the code to be a bit more efficient. > > Link: https://lore.kernel.org/all/[email protected]/ > > Fixes: b04f18b005c6b ("libtracefs: New APIs for dynamic events") > Reported-by: Metin Kaya <[email protected]> > Signed-off-by: Steven Rostedt (Google) <[email protected]> > --- > Changes since v1: https://lore.kernel.org/all/[email protected]/ > > - Fixed reading an empty kprobe_events or uprobe_events file > > src/tracefs-dynevents.c | 119 ++++++++++++++++++++++------------------ > 1 file changed, 65 insertions(+), 54 deletions(-) > > diff --git a/src/tracefs-dynevents.c b/src/tracefs-dynevents.c > index 85c1fcd9d5d5..6df7212fb38e 100644 > --- a/src/tracefs-dynevents.c > +++ b/src/tracefs-dynevents.c > @@ -34,23 +34,21 @@ static int dyn_generic_del(struct dyn_events_desc *, struct tracefs_dynevent *); > static int dyn_synth_del(struct dyn_events_desc *, struct tracefs_dynevent *); > > struct dyn_events_desc { > - enum tracefs_dynevent_type type; > - const char *file; > - const char *prefix; > + enum tracefs_dynevent_type type; > + const char *file; > + const char *prefix; > int (*del)(struct dyn_events_desc *desc, struct tracefs_dynevent *dyn); > int (*parse)(struct dyn_events_desc *desc, const char *group, > char *line, struct tracefs_dynevent **ret_dyn); > } dynevents[] = { > - {TRACEFS_DYNEVENT_KPROBE, KPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse}, > + {TRACEFS_DYNEVENT_KPROBE, KPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse}, > {TRACEFS_DYNEVENT_KRETPROBE, KPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse}, > - {TRACEFS_DYNEVENT_UPROBE, UPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse}, > + {TRACEFS_DYNEVENT_UPROBE, UPROBE_EVENTS, "p", dyn_generic_del, dyn_generic_parse}, > {TRACEFS_DYNEVENT_URETPROBE, UPROBE_EVENTS, "r", dyn_generic_del, dyn_generic_parse}, > - {TRACEFS_DYNEVENT_EPROBE, "", "e", dyn_generic_del, dyn_generic_parse}, > - {TRACEFS_DYNEVENT_SYNTH, SYNTH_EVENTS, "", dyn_synth_del, dyn_synth_parse}, > + {TRACEFS_DYNEVENT_EPROBE, "", "e", dyn_generic_del, dyn_generic_parse}, > + {TRACEFS_DYNEVENT_SYNTH, SYNTH_EVENTS, "", dyn_synth_del, dyn_synth_parse}, > }; > > - > - > static int dyn_generic_del(struct dyn_events_desc *desc, struct tracefs_dynevent *dyn) > { > char *str; > @@ -280,8 +278,13 @@ static void init_devent_desc(void) > return; > > /* Use ftrace dynamic_events, if available */ > - for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++) > + for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++) { > + /* kprobes and uprobes do not use default file */ > + if (dynevents[i].prefix[0] == 'p' || > + dynevents[i].prefix[0] == 'r') > + continue; > dynevents[i].file = DYNEVENTS_EVENTS; > + } > > dynevents[EVENT_INDEX(TRACEFS_DYNEVENT_SYNTH)].prefix = "s"; > } > @@ -480,24 +483,30 @@ int tracefs_dynevent_destroy(struct tracefs_dynevent *devent, bool force) > return desc->del(desc, devent); > } > > -static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system, > - struct tracefs_dynevent ***ret_all) > +static int get_dynevent(enum tracefs_dynevent_type type, const char *system, > + struct tracefs_dynevent ***ret_all, int count) > { > struct dyn_events_desc *desc; > - struct tracefs_dynevent *devent, **tmp, **all = NULL; > + struct tracefs_dynevent *devent, **tmp, **all; > char *content; > - int count = 0; > char *line; > char *next; > - int ret; > + int ret = -1; > > desc = get_devent_desc(type); > if (!desc) > return -1; > > + if (!tracefs_file_exists(NULL, desc->file)) > + return -1; > + > content = tracefs_instance_file_read(NULL, desc->file, NULL); > + /* File exists, but may be empty */ > if (!content) > - return -1; > + return 0; > + > + if (ret_all) > + all = *ret_all; > > line = content; > do { > @@ -507,11 +516,12 @@ static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system > ret = desc->parse(desc, system, line, ret_all ? &devent : NULL); > if (!ret) { > if (ret_all) { > - tmp = realloc(all, (count + 1) * sizeof(*tmp)); > + tmp = realloc(all, (count + 2) * sizeof(*tmp)); > if (!tmp) > - goto error; > + break; > all = tmp; > all[count] = devent; > + all[count + 1] = NULL; > } > count++; > } > @@ -521,12 +531,38 @@ static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system > free(content); > if (ret_all) > *ret_all = all; > + > return count; > +} > > -error: > - free(content); > - free(all); > - return -1; > +static int get_all_dynevents(enum tracefs_dynevent_type type, const char *system, > + struct tracefs_dynevent ***ret_all) > +{ > + int count = 0; > + int i; > + > + if (ret_all) > + *ret_all = NULL; > + > + for (i = 0; i < EVENT_INDEX(TRACEFS_DYNEVENT_MAX); i++) { > + if (!((1 << i) & type)) > + continue; > + > + count = get_dynevent((1 << i), system, ret_all, count); > + if (count < 0) { > + count = 0; > + break; > + } > + } > + > + if (!count) { > + if (ret_all) { > + free(*ret_all); > + *ret_all = NULL; > + } > + count = -1; > + } > + return count; > } > > /** > @@ -561,41 +597,16 @@ void tracefs_dynevent_list_free(struct tracefs_dynevent **events) > struct tracefs_dynevent ** > tracefs_dynevent_get_all(unsigned int types, const char *system) > { > - struct tracefs_dynevent **events, **tmp, **all_events = NULL; > - int count, all = 0; > - int i; > + struct tracefs_dynevent **events; > + int count; > > - for (i = 1; i < TRACEFS_DYNEVENT_MAX; i <<= 1) { > - if (types) { > - if (i > types) > - break; > - if (!(types & i)) > - continue; > - } > - count = get_all_dynevents(i, system, &events); > - if (count > 0) { > - tmp = realloc(all_events, (all + count + 1) * sizeof(*tmp)); > - if (!tmp) > - goto error; > - all_events = tmp; > - memcpy(all_events + all, events, count * sizeof(*events)); > - all += count; > - /* Add a NULL pointer at the end */ > - all_events[all] = NULL; > - free(events); > - } > + count = get_all_dynevents(types, system, &events); > + if (count <= 0) { > + free(events); > + return NULL; > } > > - return all_events; > - > -error: > - free(events); > - if (all_events) { > - for (i = 0; i < all; i++) > - free(all_events[i]); > - free(all_events); > - } > - return NULL; > + return events; > } > > /** All unit tests (including new ones written in [1]) in trace-cmd passes after this patch. I also confirm this patch addresses the issues mentioned in v1 [2]. Hence, Tested-by: Metin Kaya <[email protected]> [1] https://lore.kernel.org/linux-trace-devel/[email protected] [2] https://lore.kernel.org/linux-trace-devel/[email protected]