* [PATCH v2 0/2] tracing/probes: allow no event name input when create group
@ 2022-05-29 3:34 Linyu Yuan
2022-05-29 3:34 ` [PATCH v2 1/2] tracing: auto generate event name when create a group of events Linyu Yuan
2022-05-29 3:34 ` [PATCH v2 2/2] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan
0 siblings, 2 replies; 5+ messages in thread
From: Linyu Yuan @ 2022-05-29 3:34 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi, Ingo Molnar
Cc: linux-kernel, Linyu Yuan
take kprobe event as example, when create a group of events,
p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS],
according to this format, we must input EVENT name,
this change allow only GRP/ input, EVENT name auto generate from KSYM,
p[:[GRP/][EVENT]] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS]
siliar change apply to eprobe and uprobe.
V1: https://lore.kernel.org/lkml/1651397651-30454-1-git-send-email-quic_linyyuan@quicinc.com/T/#m33b0665941d9caeeb118afa84db4016321cea9a6
V2: fix remove comment in V1 patch1,
remove v1 patch2 as it is NACK.
Linyu Yuan (2):
tracing: auto generate event name when create a group of events
tracing: eprobe: remove duplicate is_good_name() operation
Documentation/trace/kprobetrace.rst | 8 ++++----
Documentation/trace/uprobetracer.rst | 8 ++++----
kernel/trace/trace_dynevent.c | 2 +-
kernel/trace/trace_eprobe.c | 24 ++++++++++++------------
kernel/trace/trace_kprobe.c | 19 ++++++++++++-------
kernel/trace/trace_probe.c | 9 ++++++++-
kernel/trace/trace_probe.h | 4 ++++
kernel/trace/trace_uprobe.c | 15 ++++++++++-----
8 files changed, 55 insertions(+), 34 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2 1/2] tracing: auto generate event name when create a group of events 2022-05-29 3:34 [PATCH v2 0/2] tracing/probes: allow no event name input when create group Linyu Yuan @ 2022-05-29 3:34 ` Linyu Yuan 2022-05-30 0:47 ` Masami Hiramatsu 2022-05-29 3:34 ` [PATCH v2 2/2] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan 1 sibling, 1 reply; 5+ messages in thread From: Linyu Yuan @ 2022-05-29 3:34 UTC (permalink / raw) To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi, Ingo Molnar Cc: linux-kernel, Linyu Yuan Currently when create a specific group of trace events, take kprobe event as example, user must use the following format: p:GRP/EVENT [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], which means user must enter EVENT name, one example is: echo 'p:usb_gadget/config_usb_cfg_link config_usb_cfg_link $arg1' >> kprobe_events, it is not simple if there are too many entries because the event name is same as symbol name. This change allows user to specify no EVENT name, format changed as: p:GRP/ [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], it will generate event name automatically and one example is: echo 'p:usb_gadget/ config_usb_cfg_link $arg1' >> kprobe_events. Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- v2: fix review comments in V1: change TP_ENAME_EMPTTY to TP_ENAME_EMPTY, add some space, document the macros return by traceprobe_parse_event_name(), updatea commit description. Documentation/trace/kprobetrace.rst | 8 ++++---- Documentation/trace/uprobetracer.rst | 8 ++++---- kernel/trace/trace_dynevent.c | 2 +- kernel/trace/trace_eprobe.c | 20 ++++++++++++-------- kernel/trace/trace_kprobe.c | 19 ++++++++++++------- kernel/trace/trace_probe.c | 9 ++++++++- kernel/trace/trace_probe.h | 4 ++++ kernel/trace/trace_uprobe.c | 15 ++++++++++----- 8 files changed, 55 insertions(+), 30 deletions(-) diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst index b175d88..4274cc6 100644 --- a/Documentation/trace/kprobetrace.rst +++ b/Documentation/trace/kprobetrace.rst @@ -28,10 +28,10 @@ Synopsis of kprobe_events ------------------------- :: - p[:[GRP/]EVENT] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe - r[MAXACTIVE][:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe - p:[GRP/]EVENT] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe - -:[GRP/]EVENT : Clear a probe + p[:[GRP/][EVENT]] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe + r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe + p[:[GRP/][EVENT]] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe + -:[GRP/][EVENT] : Clear a probe GRP : Group name. If omitted, use "kprobes" for it. EVENT : Event name. If omitted, the event name is generated diff --git a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst index a8e5938..3a1797d7 100644 --- a/Documentation/trace/uprobetracer.rst +++ b/Documentation/trace/uprobetracer.rst @@ -26,10 +26,10 @@ Synopsis of uprobe_tracer ------------------------- :: - p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe - r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) - p[:[GRP/]EVENT] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) - -:[GRP/]EVENT : Clear uprobe or uretprobe event + p[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a uprobe + r[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) + p[:[GRP/][EVENT]] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) + -:[GRP/][EVENT] : Clear uprobe or uretprobe event GRP : Group name. If omitted, "uprobes" is the default value. EVENT : Event name. If omitted, the event name is generated based diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c index e34e8182..033248d 100644 --- a/kernel/trace/trace_dynevent.c +++ b/kernel/trace/trace_dynevent.c @@ -101,7 +101,7 @@ int dyn_event_release(const char *raw_command, struct dyn_event_operations *type event = p + 1; *p = '\0'; } - if (event[0] == '\0') { + if (!system && event[0] == '\0') { ret = -EINVAL; goto out; } diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c index 7d44785..13cd7fc 100644 --- a/kernel/trace/trace_eprobe.c +++ b/kernel/trace/trace_eprobe.c @@ -125,6 +125,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, * We match the following: * event only - match all eprobes with event name * system and event only - match all system/event probes + * system only - match all system probes * * The below has the above satisfied with more arguments: * @@ -143,7 +144,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, return false; /* Must match the event name */ - if (strcmp(trace_probe_name(&ep->tp), event) != 0) + if (event[0] != '\0' && strcmp(trace_probe_name(&ep->tp), event) != 0) return false; /* No arguments match all */ @@ -848,7 +849,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) { /* * Argument syntax: - * e[:[GRP/]ENAME] SYSTEM.EVENT [FETCHARGS] + * e[:[GRP/][ENAME]] SYSTEM.EVENT [FETCHARGS] * Fetch args: * <name>=$<field>[:TYPE] */ @@ -858,6 +859,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) struct trace_eprobe *ep = NULL; char buf1[MAX_EVENT_NAME_LEN]; char buf2[MAX_EVENT_NAME_LEN]; + char grp_buf[MAX_EVENT_NAME_LEN]; int ret = 0; int i; @@ -866,14 +868,17 @@ static int __trace_eprobe_create(int argc, const char *argv[]) trace_probe_log_init("event_probe", argc, argv); + ret = TP_ENAME_EMPTY; event = strchr(&argv[0][1], ':'); if (event) { event++; - ret = traceprobe_parse_event_name(&event, &group, buf1, + ret = traceprobe_parse_event_name(&event, &group, grp_buf, event - argv[0]); - if (ret) + if (ret < 0) goto parse_error; - } else { + } + + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN); sanitize_event_name(buf1); event = buf1; @@ -882,9 +887,8 @@ static int __trace_eprobe_create(int argc, const char *argv[]) goto parse_error; sys_event = argv[1]; - ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, - sys_event - argv[1]); - if (ret || !sys_name) + ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); + if (ret != TP_ENAME_GROUP_EVENT) goto parse_error; if (!is_good_name(sys_event) || !is_good_name(sys_name)) goto parse_error; diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 47cebef..55822b6 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -163,7 +163,8 @@ static bool trace_kprobe_match(const char *system, const char *event, { struct trace_kprobe *tk = to_trace_kprobe(ev); - return strcmp(trace_probe_name(&tk->tp), event) == 0 && + return (event[0] == '\0' || + strcmp(trace_probe_name(&tk->tp), event) == 0) && (!system || strcmp(trace_probe_group_name(&tk->tp), system) == 0) && trace_kprobe_match_command_head(tk, argc, argv); } @@ -708,11 +709,11 @@ static int __trace_kprobe_create(int argc, const char *argv[]) /* * Argument syntax: * - Add kprobe: - * p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] + * p[:[GRP/][EVENT]] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] * - Add kretprobe: - * r[MAXACTIVE][:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS] + * r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]KSYM[+0] [FETCHARGS] * Or - * p:[GRP/]EVENT] [MOD:]KSYM[+0]%return [FETCHARGS] + * p[:[GRP/][EVENT]] [MOD:]KSYM[+0]%return [FETCHARGS] * * Fetch args: * $retval : fetch return value @@ -739,6 +740,7 @@ static int __trace_kprobe_create(int argc, const char *argv[]) long offset = 0; void *addr = NULL; char buf[MAX_EVENT_NAME_LEN]; + char grp_buf[MAX_EVENT_NAME_LEN]; unsigned int flags = TPARG_FL_KERNEL; switch (argv[0][0]) { @@ -832,12 +834,15 @@ static int __trace_kprobe_create(int argc, const char *argv[]) } trace_probe_log_set_index(0); + ret = TP_ENAME_EMPTY; if (event) { - ret = traceprobe_parse_event_name(&event, &group, buf, + ret = traceprobe_parse_event_name(&event, &group, grp_buf, event - argv[0]); - if (ret) + if (ret < 0) goto parse_error; - } else { + } + + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { /* Make a new event name */ if (symbol) snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_%ld", diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c index 80863c6..7fd50ab 100644 --- a/kernel/trace/trace_probe.c +++ b/kernel/trace/trace_probe.c @@ -257,6 +257,9 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, } len = strlen(event); if (len == 0) { + if (slash) + return TP_ENAME_GROUP; + trace_probe_log_err(offset, NO_EVENT_NAME); return -EINVAL; } else if (len > MAX_EVENT_NAME_LEN) { @@ -267,7 +270,11 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, trace_probe_log_err(offset, BAD_EVENT_NAME); return -EINVAL; } - return 0; + + if (slash) + return TP_ENAME_GROUP_EVENT; + + return TP_ENAME_EVENT; } #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long)) diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index 92cc149..7390669 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -364,6 +364,10 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg); extern int traceprobe_split_symbol_offset(char *symbol, long *offset); int traceprobe_parse_event_name(const char **pevent, const char **pgroup, char *buf, int offset); +#define TP_ENAME_GROUP_EVENT 0 /* GRP/EVENT format, user defined group and event name */ +#define TP_ENAME_EVENT 1 /* EVENT format, user defined event name, default group */ +#define TP_ENAME_GROUP 2 /* GRP/ format, user defined group name, auto event name */ +#define TP_ENAME_EMPTY 3 /* default group and auto event name */ enum probe_print_type { PROBE_PRINT_NORMAL, diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 9711589..bc32361 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -312,7 +312,8 @@ static bool trace_uprobe_match(const char *system, const char *event, { struct trace_uprobe *tu = to_trace_uprobe(ev); - return strcmp(trace_probe_name(&tu->tp), event) == 0 && + return (event[0] == '\0' || + strcmp(trace_probe_name(&tu->tp), event) == 0) && (!system || strcmp(trace_probe_group_name(&tu->tp), system) == 0) && trace_uprobe_match_command_head(tu, argc, argv); } @@ -532,7 +533,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu) /* * Argument syntax: - * - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET[%return][(REF)] [FETCHARGS] + * - Add uprobe: p|r[:[GRP/][EVENT]] PATH:OFFSET[%return][(REF)] [FETCHARGS] */ static int __trace_uprobe_create(int argc, const char **argv) { @@ -540,6 +541,7 @@ static int __trace_uprobe_create(int argc, const char **argv) const char *event = NULL, *group = UPROBE_EVENT_SYSTEM; char *arg, *filename, *rctr, *rctr_end, *tmp; char buf[MAX_EVENT_NAME_LEN]; + char grp_buf[MAX_EVENT_NAME_LEN]; enum probe_print_type ptype; struct path path; unsigned long offset, ref_ctr_offset; @@ -644,12 +646,15 @@ static int __trace_uprobe_create(int argc, const char **argv) /* setup a probe */ trace_probe_log_set_index(0); + ret = TP_ENAME_EMPTY; if (event) { - ret = traceprobe_parse_event_name(&event, &group, buf, + ret = traceprobe_parse_event_name(&event, &group, grp_buf, event - argv[0]); - if (ret) + if (ret < 0) goto fail_address_parse; - } else { + } + + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { char *tail; char *ptr; -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] tracing: auto generate event name when create a group of events 2022-05-29 3:34 ` [PATCH v2 1/2] tracing: auto generate event name when create a group of events Linyu Yuan @ 2022-05-30 0:47 ` Masami Hiramatsu 2022-05-30 1:28 ` Linyu Yuan 0 siblings, 1 reply; 5+ messages in thread From: Masami Hiramatsu @ 2022-05-30 0:47 UTC (permalink / raw) To: Linyu Yuan Cc: Steven Rostedt, Tom Zanussi, Ingo Molnar, linux-kernel, Tzvetomir Stoyanov Hi Linyu, On Sun, 29 May 2022 11:34:53 +0800 Linyu Yuan <quic_linyyuan@quicinc.com> wrote: > Currently when create a specific group of trace events, > take kprobe event as example, user must use the following format: > p:GRP/EVENT [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], > which means user must enter EVENT name, one example is: echo > 'p:usb_gadget/config_usb_cfg_link config_usb_cfg_link $arg1' >> > kprobe_events, it is not simple if there are too many entries > because the event name is same as symbol name. > > This change allows user to specify no EVENT name, format changed as: > p:GRP/ [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], > it will generate event name automatically and one example is: > echo 'p:usb_gadget/ config_usb_cfg_link $arg1' >> kprobe_events. > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > v2: fix review comments in V1: > change TP_ENAME_EMPTTY to TP_ENAME_EMPTY, Thanks for update! And I think this return code is a bit redundant because those cases can be rephrased as follows; TP_ENAME_GROUP_EVENT : event != NULL && group != (default) TP_ENAME_GROUP : event == NULL && group != (default) TP_ENAME_EVENT : event != NULL && group == (default) TP_ENAME_EMPTY : event == NULL && group == (default) What about this (e.g. trace_kprobe.c)? if (event) { /* event and group will be updated in this function */ ret = traceprobe_parse_event_name(&event, &group, gbuf, event - argv[0]); if (ret) goto parse_error; } if (!event) { /* Make a new event name */ if (symbol) snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_%ld", is_return ? 'r' : 'p', symbol, offset); else snprintf(buf, MAX_EVENT_NAME_LEN, "%c_0x%p", is_return ? 'r' : 'p', addr); sanitize_event_name(buf); event = buf; } This makes the code clearer. > add some space, > document the macros return by traceprobe_parse_event_name(), > updatea commit description. > > Documentation/trace/kprobetrace.rst | 8 ++++---- > Documentation/trace/uprobetracer.rst | 8 ++++---- > kernel/trace/trace_dynevent.c | 2 +- > kernel/trace/trace_eprobe.c | 20 ++++++++++++-------- > kernel/trace/trace_kprobe.c | 19 ++++++++++++------- > kernel/trace/trace_probe.c | 9 ++++++++- > kernel/trace/trace_probe.h | 4 ++++ > kernel/trace/trace_uprobe.c | 15 ++++++++++----- > 8 files changed, 55 insertions(+), 30 deletions(-) > > diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst > index b175d88..4274cc6 100644 > --- a/Documentation/trace/kprobetrace.rst > +++ b/Documentation/trace/kprobetrace.rst > @@ -28,10 +28,10 @@ Synopsis of kprobe_events > ------------------------- > :: > > - p[:[GRP/]EVENT] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe > - r[MAXACTIVE][:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe > - p:[GRP/]EVENT] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe > - -:[GRP/]EVENT : Clear a probe > + p[:[GRP/][EVENT]] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe > + r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe > + p[:[GRP/][EVENT]] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe > + -:[GRP/][EVENT] : Clear a probe Could you also update 'readme_msg' in kernel/trace/trace.c in this patch? e.g. #if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS) "\t accepts: event-definitions (one definition per line)\n" "\t Format: p[:[<group>/][<event>]] <place> [<args>]\n" "\t r[maxactive][:[<group>/][<event>]] <place> [<args>]\n" #ifdef CONFIG_HIST_TRIGGERS "\t s:[synthetic/]<event> <field> [<field>]\n" #endif "\t e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]\n" "\t -:[<group>/]<event>\n" And also, could you add *another patch* to update below testcases about dynevent to ensure this feature is working? tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc and tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc e.g. if grep -q '\[<event>\]' README then (add your test...) fi Thank you, > > GRP : Group name. If omitted, use "kprobes" for it. > EVENT : Event name. If omitted, the event name is generated > diff --git a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst > index a8e5938..3a1797d7 100644 > --- a/Documentation/trace/uprobetracer.rst > +++ b/Documentation/trace/uprobetracer.rst > @@ -26,10 +26,10 @@ Synopsis of uprobe_tracer > ------------------------- > :: > > - p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe > - r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) > - p[:[GRP/]EVENT] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) > - -:[GRP/]EVENT : Clear uprobe or uretprobe event > + p[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a uprobe > + r[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) > + p[:[GRP/][EVENT]] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) > + -:[GRP/][EVENT] : Clear uprobe or uretprobe event > > GRP : Group name. If omitted, "uprobes" is the default value. > EVENT : Event name. If omitted, the event name is generated based > diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c > index e34e8182..033248d 100644 > --- a/kernel/trace/trace_dynevent.c > +++ b/kernel/trace/trace_dynevent.c > @@ -101,7 +101,7 @@ int dyn_event_release(const char *raw_command, struct dyn_event_operations *type > event = p + 1; > *p = '\0'; > } > - if (event[0] == '\0') { > + if (!system && event[0] == '\0') { > ret = -EINVAL; > goto out; > } > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c > index 7d44785..13cd7fc 100644 > --- a/kernel/trace/trace_eprobe.c > +++ b/kernel/trace/trace_eprobe.c > @@ -125,6 +125,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, > * We match the following: > * event only - match all eprobes with event name > * system and event only - match all system/event probes > + * system only - match all system probes > * > * The below has the above satisfied with more arguments: > * > @@ -143,7 +144,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, > return false; > > /* Must match the event name */ > - if (strcmp(trace_probe_name(&ep->tp), event) != 0) > + if (event[0] != '\0' && strcmp(trace_probe_name(&ep->tp), event) != 0) > return false; > > /* No arguments match all */ > @@ -848,7 +849,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > { > /* > * Argument syntax: > - * e[:[GRP/]ENAME] SYSTEM.EVENT [FETCHARGS] > + * e[:[GRP/][ENAME]] SYSTEM.EVENT [FETCHARGS] > * Fetch args: > * <name>=$<field>[:TYPE] > */ > @@ -858,6 +859,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > struct trace_eprobe *ep = NULL; > char buf1[MAX_EVENT_NAME_LEN]; > char buf2[MAX_EVENT_NAME_LEN]; > + char grp_buf[MAX_EVENT_NAME_LEN]; > int ret = 0; > int i; > > @@ -866,14 +868,17 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > > trace_probe_log_init("event_probe", argc, argv); > > + ret = TP_ENAME_EMPTY; > event = strchr(&argv[0][1], ':'); > if (event) { > event++; > - ret = traceprobe_parse_event_name(&event, &group, buf1, > + ret = traceprobe_parse_event_name(&event, &group, grp_buf, > event - argv[0]); > - if (ret) > + if (ret < 0) > goto parse_error; > - } else { > + } > + > + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { > strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN); > sanitize_event_name(buf1); > event = buf1; > @@ -882,9 +887,8 @@ static int __trace_eprobe_create(int argc, const char *argv[]) > goto parse_error; > > sys_event = argv[1]; > - ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, > - sys_event - argv[1]); > - if (ret || !sys_name) > + ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); > + if (ret != TP_ENAME_GROUP_EVENT) > goto parse_error; > if (!is_good_name(sys_event) || !is_good_name(sys_name)) > goto parse_error; > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index 47cebef..55822b6 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -163,7 +163,8 @@ static bool trace_kprobe_match(const char *system, const char *event, > { > struct trace_kprobe *tk = to_trace_kprobe(ev); > > - return strcmp(trace_probe_name(&tk->tp), event) == 0 && > + return (event[0] == '\0' || > + strcmp(trace_probe_name(&tk->tp), event) == 0) && > (!system || strcmp(trace_probe_group_name(&tk->tp), system) == 0) && > trace_kprobe_match_command_head(tk, argc, argv); > } > @@ -708,11 +709,11 @@ static int __trace_kprobe_create(int argc, const char *argv[]) > /* > * Argument syntax: > * - Add kprobe: > - * p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] > + * p[:[GRP/][EVENT]] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] > * - Add kretprobe: > - * r[MAXACTIVE][:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS] > + * r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]KSYM[+0] [FETCHARGS] > * Or > - * p:[GRP/]EVENT] [MOD:]KSYM[+0]%return [FETCHARGS] > + * p[:[GRP/][EVENT]] [MOD:]KSYM[+0]%return [FETCHARGS] > * > * Fetch args: > * $retval : fetch return value > @@ -739,6 +740,7 @@ static int __trace_kprobe_create(int argc, const char *argv[]) > long offset = 0; > void *addr = NULL; > char buf[MAX_EVENT_NAME_LEN]; > + char grp_buf[MAX_EVENT_NAME_LEN]; > unsigned int flags = TPARG_FL_KERNEL; > > switch (argv[0][0]) { > @@ -832,12 +834,15 @@ static int __trace_kprobe_create(int argc, const char *argv[]) > } > > trace_probe_log_set_index(0); > + ret = TP_ENAME_EMPTY; > if (event) { > - ret = traceprobe_parse_event_name(&event, &group, buf, > + ret = traceprobe_parse_event_name(&event, &group, grp_buf, > event - argv[0]); > - if (ret) > + if (ret < 0) > goto parse_error; > - } else { > + } > + > + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { > /* Make a new event name */ > if (symbol) > snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_%ld", > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > index 80863c6..7fd50ab 100644 > --- a/kernel/trace/trace_probe.c > +++ b/kernel/trace/trace_probe.c > @@ -257,6 +257,9 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, > } > len = strlen(event); > if (len == 0) { > + if (slash) > + return TP_ENAME_GROUP; > + > trace_probe_log_err(offset, NO_EVENT_NAME); > return -EINVAL; > } else if (len > MAX_EVENT_NAME_LEN) { > @@ -267,7 +270,11 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, > trace_probe_log_err(offset, BAD_EVENT_NAME); > return -EINVAL; > } > - return 0; > + > + if (slash) > + return TP_ENAME_GROUP_EVENT; > + > + return TP_ENAME_EVENT; > } > > #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long)) > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h > index 92cc149..7390669 100644 > --- a/kernel/trace/trace_probe.h > +++ b/kernel/trace/trace_probe.h > @@ -364,6 +364,10 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg); > extern int traceprobe_split_symbol_offset(char *symbol, long *offset); > int traceprobe_parse_event_name(const char **pevent, const char **pgroup, > char *buf, int offset); > +#define TP_ENAME_GROUP_EVENT 0 /* GRP/EVENT format, user defined group and event name */ > +#define TP_ENAME_EVENT 1 /* EVENT format, user defined event name, default group */ > +#define TP_ENAME_GROUP 2 /* GRP/ format, user defined group name, auto event name */ > +#define TP_ENAME_EMPTY 3 /* default group and auto event name */ > > enum probe_print_type { > PROBE_PRINT_NORMAL, > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 9711589..bc32361 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -312,7 +312,8 @@ static bool trace_uprobe_match(const char *system, const char *event, > { > struct trace_uprobe *tu = to_trace_uprobe(ev); > > - return strcmp(trace_probe_name(&tu->tp), event) == 0 && > + return (event[0] == '\0' || > + strcmp(trace_probe_name(&tu->tp), event) == 0) && > (!system || strcmp(trace_probe_group_name(&tu->tp), system) == 0) && > trace_uprobe_match_command_head(tu, argc, argv); > } > @@ -532,7 +533,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu) > > /* > * Argument syntax: > - * - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET[%return][(REF)] [FETCHARGS] > + * - Add uprobe: p|r[:[GRP/][EVENT]] PATH:OFFSET[%return][(REF)] [FETCHARGS] > */ > static int __trace_uprobe_create(int argc, const char **argv) > { > @@ -540,6 +541,7 @@ static int __trace_uprobe_create(int argc, const char **argv) > const char *event = NULL, *group = UPROBE_EVENT_SYSTEM; > char *arg, *filename, *rctr, *rctr_end, *tmp; > char buf[MAX_EVENT_NAME_LEN]; > + char grp_buf[MAX_EVENT_NAME_LEN]; > enum probe_print_type ptype; > struct path path; > unsigned long offset, ref_ctr_offset; > @@ -644,12 +646,15 @@ static int __trace_uprobe_create(int argc, const char **argv) > > /* setup a probe */ > trace_probe_log_set_index(0); > + ret = TP_ENAME_EMPTY; > if (event) { > - ret = traceprobe_parse_event_name(&event, &group, buf, > + ret = traceprobe_parse_event_name(&event, &group, grp_buf, > event - argv[0]); > - if (ret) > + if (ret < 0) > goto fail_address_parse; > - } else { > + } > + > + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { > char *tail; > char *ptr; > > -- > 2.7.4 > -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] tracing: auto generate event name when create a group of events 2022-05-30 0:47 ` Masami Hiramatsu @ 2022-05-30 1:28 ` Linyu Yuan 0 siblings, 0 replies; 5+ messages in thread From: Linyu Yuan @ 2022-05-30 1:28 UTC (permalink / raw) To: Masami Hiramatsu (Google) Cc: Steven Rostedt, Tom Zanussi, Ingo Molnar, linux-kernel, Tzvetomir Stoyanov (VMware) On 5/30/2022 8:47 AM, Masami Hiramatsu (Google) wrote: > Hi Linyu, > > On Sun, 29 May 2022 11:34:53 +0800 > Linyu Yuan <quic_linyyuan@quicinc.com> wrote: > >> Currently when create a specific group of trace events, >> take kprobe event as example, user must use the following format: >> p:GRP/EVENT [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], >> which means user must enter EVENT name, one example is: echo >> 'p:usb_gadget/config_usb_cfg_link config_usb_cfg_link $arg1' >> >> kprobe_events, it is not simple if there are too many entries >> because the event name is same as symbol name. >> >> This change allows user to specify no EVENT name, format changed as: >> p:GRP/ [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS], >> it will generate event name automatically and one example is: >> echo 'p:usb_gadget/ config_usb_cfg_link $arg1' >> kprobe_events. >> >> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> >> --- >> v2: fix review comments in V1: >> change TP_ENAME_EMPTTY to TP_ENAME_EMPTY, > Thanks for update! And I think this return code is a bit redundant > because those cases can be rephrased as follows; > > TP_ENAME_GROUP_EVENT : event != NULL && group != (default) > TP_ENAME_GROUP : event == NULL && group != (default) > TP_ENAME_EVENT : event != NULL && group == (default) > TP_ENAME_EMPTY : event == NULL && group == (default) > > What about this (e.g. trace_kprobe.c)? > > if (event) { > /* event and group will be updated in this function */ > ret = traceprobe_parse_event_name(&event, &group, gbuf, > event - argv[0]); > if (ret) > goto parse_error; > } > > if (!event) { > /* Make a new event name */ > if (symbol) > snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_%ld", > is_return ? 'r' : 'p', symbol, offset); > else > snprintf(buf, MAX_EVENT_NAME_LEN, "%c_0x%p", > is_return ? 'r' : 'p', addr); > sanitize_event_name(buf); > event = buf; > } > > This makes the code clearer. good idea, will follow it. > >> add some space, >> document the macros return by traceprobe_parse_event_name(), >> updatea commit description. >> >> Documentation/trace/kprobetrace.rst | 8 ++++---- >> Documentation/trace/uprobetracer.rst | 8 ++++---- >> kernel/trace/trace_dynevent.c | 2 +- >> kernel/trace/trace_eprobe.c | 20 ++++++++++++-------- >> kernel/trace/trace_kprobe.c | 19 ++++++++++++------- >> kernel/trace/trace_probe.c | 9 ++++++++- >> kernel/trace/trace_probe.h | 4 ++++ >> kernel/trace/trace_uprobe.c | 15 ++++++++++----- >> 8 files changed, 55 insertions(+), 30 deletions(-) >> >> diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst >> index b175d88..4274cc6 100644 >> --- a/Documentation/trace/kprobetrace.rst >> +++ b/Documentation/trace/kprobetrace.rst >> @@ -28,10 +28,10 @@ Synopsis of kprobe_events >> ------------------------- >> :: >> >> - p[:[GRP/]EVENT] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe >> - r[MAXACTIVE][:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe >> - p:[GRP/]EVENT] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe >> - -:[GRP/]EVENT : Clear a probe >> + p[:[GRP/][EVENT]] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe >> + r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]SYM[+0] [FETCHARGS] : Set a return probe >> + p[:[GRP/][EVENT]] [MOD:]SYM[+0]%return [FETCHARGS] : Set a return probe >> + -:[GRP/][EVENT] : Clear a probe > > Could you also update 'readme_msg' in kernel/trace/trace.c in this patch? > e.g. > > #if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS) > "\t accepts: event-definitions (one definition per line)\n" > "\t Format: p[:[<group>/][<event>]] <place> [<args>]\n" > "\t r[maxactive][:[<group>/][<event>]] <place> [<args>]\n" > #ifdef CONFIG_HIST_TRIGGERS > "\t s:[synthetic/]<event> <field> [<field>]\n" > #endif > "\t e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]\n" > "\t -:[<group>/]<event>\n" sure. > > And also, could you add *another patch* to update below testcases about > dynevent to ensure this feature is working? > > tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc > and tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc > > e.g. > > if grep -q '\[<event>\]' README then > (add your test...) > fi thanks, will do it. > > > Thank you, > > >> >> GRP : Group name. If omitted, use "kprobes" for it. >> EVENT : Event name. If omitted, the event name is generated >> diff --git a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst >> index a8e5938..3a1797d7 100644 >> --- a/Documentation/trace/uprobetracer.rst >> +++ b/Documentation/trace/uprobetracer.rst >> @@ -26,10 +26,10 @@ Synopsis of uprobe_tracer >> ------------------------- >> :: >> >> - p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe >> - r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) >> - p[:[GRP/]EVENT] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) >> - -:[GRP/]EVENT : Clear uprobe or uretprobe event >> + p[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a uprobe >> + r[:[GRP/][EVENT]] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe) >> + p[:[GRP/][EVENT]] PATH:OFFSET%return [FETCHARGS] : Set a return uprobe (uretprobe) >> + -:[GRP/][EVENT] : Clear uprobe or uretprobe event >> >> GRP : Group name. If omitted, "uprobes" is the default value. >> EVENT : Event name. If omitted, the event name is generated based >> diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c >> index e34e8182..033248d 100644 >> --- a/kernel/trace/trace_dynevent.c >> +++ b/kernel/trace/trace_dynevent.c >> @@ -101,7 +101,7 @@ int dyn_event_release(const char *raw_command, struct dyn_event_operations *type >> event = p + 1; >> *p = '\0'; >> } >> - if (event[0] == '\0') { >> + if (!system && event[0] == '\0') { >> ret = -EINVAL; >> goto out; >> } >> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c >> index 7d44785..13cd7fc 100644 >> --- a/kernel/trace/trace_eprobe.c >> +++ b/kernel/trace/trace_eprobe.c >> @@ -125,6 +125,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, >> * We match the following: >> * event only - match all eprobes with event name >> * system and event only - match all system/event probes >> + * system only - match all system probes >> * >> * The below has the above satisfied with more arguments: >> * >> @@ -143,7 +144,7 @@ static bool eprobe_dyn_event_match(const char *system, const char *event, >> return false; >> >> /* Must match the event name */ >> - if (strcmp(trace_probe_name(&ep->tp), event) != 0) >> + if (event[0] != '\0' && strcmp(trace_probe_name(&ep->tp), event) != 0) >> return false; >> >> /* No arguments match all */ >> @@ -848,7 +849,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) >> { >> /* >> * Argument syntax: >> - * e[:[GRP/]ENAME] SYSTEM.EVENT [FETCHARGS] >> + * e[:[GRP/][ENAME]] SYSTEM.EVENT [FETCHARGS] >> * Fetch args: >> * <name>=$<field>[:TYPE] >> */ >> @@ -858,6 +859,7 @@ static int __trace_eprobe_create(int argc, const char *argv[]) >> struct trace_eprobe *ep = NULL; >> char buf1[MAX_EVENT_NAME_LEN]; >> char buf2[MAX_EVENT_NAME_LEN]; >> + char grp_buf[MAX_EVENT_NAME_LEN]; >> int ret = 0; >> int i; >> >> @@ -866,14 +868,17 @@ static int __trace_eprobe_create(int argc, const char *argv[]) >> >> trace_probe_log_init("event_probe", argc, argv); >> >> + ret = TP_ENAME_EMPTY; >> event = strchr(&argv[0][1], ':'); >> if (event) { >> event++; >> - ret = traceprobe_parse_event_name(&event, &group, buf1, >> + ret = traceprobe_parse_event_name(&event, &group, grp_buf, >> event - argv[0]); >> - if (ret) >> + if (ret < 0) >> goto parse_error; >> - } else { >> + } >> + >> + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { >> strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN); >> sanitize_event_name(buf1); >> event = buf1; >> @@ -882,9 +887,8 @@ static int __trace_eprobe_create(int argc, const char *argv[]) >> goto parse_error; >> >> sys_event = argv[1]; >> - ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, >> - sys_event - argv[1]); >> - if (ret || !sys_name) >> + ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); >> + if (ret != TP_ENAME_GROUP_EVENT) >> goto parse_error; >> if (!is_good_name(sys_event) || !is_good_name(sys_name)) >> goto parse_error; >> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c >> index 47cebef..55822b6 100644 >> --- a/kernel/trace/trace_kprobe.c >> +++ b/kernel/trace/trace_kprobe.c >> @@ -163,7 +163,8 @@ static bool trace_kprobe_match(const char *system, const char *event, >> { >> struct trace_kprobe *tk = to_trace_kprobe(ev); >> >> - return strcmp(trace_probe_name(&tk->tp), event) == 0 && >> + return (event[0] == '\0' || >> + strcmp(trace_probe_name(&tk->tp), event) == 0) && >> (!system || strcmp(trace_probe_group_name(&tk->tp), system) == 0) && >> trace_kprobe_match_command_head(tk, argc, argv); >> } >> @@ -708,11 +709,11 @@ static int __trace_kprobe_create(int argc, const char *argv[]) >> /* >> * Argument syntax: >> * - Add kprobe: >> - * p[:[GRP/]EVENT] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] >> + * p[:[GRP/][EVENT]] [MOD:]KSYM[+OFFS]|KADDR [FETCHARGS] >> * - Add kretprobe: >> - * r[MAXACTIVE][:[GRP/]EVENT] [MOD:]KSYM[+0] [FETCHARGS] >> + * r[MAXACTIVE][:[GRP/][EVENT]] [MOD:]KSYM[+0] [FETCHARGS] >> * Or >> - * p:[GRP/]EVENT] [MOD:]KSYM[+0]%return [FETCHARGS] >> + * p[:[GRP/][EVENT]] [MOD:]KSYM[+0]%return [FETCHARGS] >> * >> * Fetch args: >> * $retval : fetch return value >> @@ -739,6 +740,7 @@ static int __trace_kprobe_create(int argc, const char *argv[]) >> long offset = 0; >> void *addr = NULL; >> char buf[MAX_EVENT_NAME_LEN]; >> + char grp_buf[MAX_EVENT_NAME_LEN]; >> unsigned int flags = TPARG_FL_KERNEL; >> >> switch (argv[0][0]) { >> @@ -832,12 +834,15 @@ static int __trace_kprobe_create(int argc, const char *argv[]) >> } >> >> trace_probe_log_set_index(0); >> + ret = TP_ENAME_EMPTY; >> if (event) { >> - ret = traceprobe_parse_event_name(&event, &group, buf, >> + ret = traceprobe_parse_event_name(&event, &group, grp_buf, >> event - argv[0]); >> - if (ret) >> + if (ret < 0) >> goto parse_error; >> - } else { >> + } >> + >> + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { >> /* Make a new event name */ >> if (symbol) >> snprintf(buf, MAX_EVENT_NAME_LEN, "%c_%s_%ld", >> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c >> index 80863c6..7fd50ab 100644 >> --- a/kernel/trace/trace_probe.c >> +++ b/kernel/trace/trace_probe.c >> @@ -257,6 +257,9 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, >> } >> len = strlen(event); >> if (len == 0) { >> + if (slash) >> + return TP_ENAME_GROUP; >> + >> trace_probe_log_err(offset, NO_EVENT_NAME); >> return -EINVAL; >> } else if (len > MAX_EVENT_NAME_LEN) { >> @@ -267,7 +270,11 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, >> trace_probe_log_err(offset, BAD_EVENT_NAME); >> return -EINVAL; >> } >> - return 0; >> + >> + if (slash) >> + return TP_ENAME_GROUP_EVENT; >> + >> + return TP_ENAME_EVENT; >> } >> >> #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long)) >> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h >> index 92cc149..7390669 100644 >> --- a/kernel/trace/trace_probe.h >> +++ b/kernel/trace/trace_probe.h >> @@ -364,6 +364,10 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg); >> extern int traceprobe_split_symbol_offset(char *symbol, long *offset); >> int traceprobe_parse_event_name(const char **pevent, const char **pgroup, >> char *buf, int offset); >> +#define TP_ENAME_GROUP_EVENT 0 /* GRP/EVENT format, user defined group and event name */ >> +#define TP_ENAME_EVENT 1 /* EVENT format, user defined event name, default group */ >> +#define TP_ENAME_GROUP 2 /* GRP/ format, user defined group name, auto event name */ >> +#define TP_ENAME_EMPTY 3 /* default group and auto event name */ >> >> enum probe_print_type { >> PROBE_PRINT_NORMAL, >> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c >> index 9711589..bc32361 100644 >> --- a/kernel/trace/trace_uprobe.c >> +++ b/kernel/trace/trace_uprobe.c >> @@ -312,7 +312,8 @@ static bool trace_uprobe_match(const char *system, const char *event, >> { >> struct trace_uprobe *tu = to_trace_uprobe(ev); >> >> - return strcmp(trace_probe_name(&tu->tp), event) == 0 && >> + return (event[0] == '\0' || >> + strcmp(trace_probe_name(&tu->tp), event) == 0) && >> (!system || strcmp(trace_probe_group_name(&tu->tp), system) == 0) && >> trace_uprobe_match_command_head(tu, argc, argv); >> } >> @@ -532,7 +533,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu) >> >> /* >> * Argument syntax: >> - * - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET[%return][(REF)] [FETCHARGS] >> + * - Add uprobe: p|r[:[GRP/][EVENT]] PATH:OFFSET[%return][(REF)] [FETCHARGS] >> */ >> static int __trace_uprobe_create(int argc, const char **argv) >> { >> @@ -540,6 +541,7 @@ static int __trace_uprobe_create(int argc, const char **argv) >> const char *event = NULL, *group = UPROBE_EVENT_SYSTEM; >> char *arg, *filename, *rctr, *rctr_end, *tmp; >> char buf[MAX_EVENT_NAME_LEN]; >> + char grp_buf[MAX_EVENT_NAME_LEN]; >> enum probe_print_type ptype; >> struct path path; >> unsigned long offset, ref_ctr_offset; >> @@ -644,12 +646,15 @@ static int __trace_uprobe_create(int argc, const char **argv) >> >> /* setup a probe */ >> trace_probe_log_set_index(0); >> + ret = TP_ENAME_EMPTY; >> if (event) { >> - ret = traceprobe_parse_event_name(&event, &group, buf, >> + ret = traceprobe_parse_event_name(&event, &group, grp_buf, >> event - argv[0]); >> - if (ret) >> + if (ret < 0) >> goto fail_address_parse; >> - } else { >> + } >> + >> + if (ret == TP_ENAME_EMPTY || ret == TP_ENAME_GROUP) { >> char *tail; >> char *ptr; >> >> -- >> 2.7.4 >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] tracing: eprobe: remove duplicate is_good_name() operation 2022-05-29 3:34 [PATCH v2 0/2] tracing/probes: allow no event name input when create group Linyu Yuan 2022-05-29 3:34 ` [PATCH v2 1/2] tracing: auto generate event name when create a group of events Linyu Yuan @ 2022-05-29 3:34 ` Linyu Yuan 1 sibling, 0 replies; 5+ messages in thread From: Linyu Yuan @ 2022-05-29 3:34 UTC (permalink / raw) To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi, Ingo Molnar Cc: linux-kernel, Linyu Yuan traceprobe_parse_event_name() already validate group and event name, there is no need to call is_good_name() after it. Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- v2: drop v1 change as it is NACK. add it to remove duplicate is_good_name(). kernel/trace/trace_eprobe.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c index 13cd7fc..2ee041d 100644 --- a/kernel/trace/trace_eprobe.c +++ b/kernel/trace/trace_eprobe.c @@ -883,15 +883,11 @@ static int __trace_eprobe_create(int argc, const char *argv[]) sanitize_event_name(buf1); event = buf1; } - if (!is_good_name(event) || !is_good_name(group)) - goto parse_error; sys_event = argv[1]; ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2, 0); if (ret != TP_ENAME_GROUP_EVENT) goto parse_error; - if (!is_good_name(sys_event) || !is_good_name(sys_name)) - goto parse_error; mutex_lock(&event_mutex); event_call = find_and_get_event(sys_name, sys_event); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-30 1:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-29 3:34 [PATCH v2 0/2] tracing/probes: allow no event name input when create group Linyu Yuan 2022-05-29 3:34 ` [PATCH v2 1/2] tracing: auto generate event name when create a group of events Linyu Yuan 2022-05-30 0:47 ` Masami Hiramatsu 2022-05-30 1:28 ` Linyu Yuan 2022-05-29 3:34 ` [PATCH v2 2/2] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).