lkml.org 
[lkml]   [2022]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/2] tracing/probes: auto generate events name when create a group of events
From

On 5/25/2022 6:31 AM, Steven Rostedt wrote:
> [ Adding Masami (kprobe maintainer) and Tom (dynamic event maintainer) ]
>
> On Sun, 1 May 2022 17:34:10 +0800
> Linyu Yuan <quic_linyyuan@quicinc.com> wrote:
>
>> change traceprobe_parse_event_name() and return GRP/EVENT types,
>> TP_ENAME_GROUP_EVENT means user input GRP/EVENT format,
>> TP_ENAME_EVENT means user only input EVENT format,
>> TP_ENAME_GROUP means user only input GRP/ format,
>>
>> it allow no event name input when create a group of events,
>> and will auto generate event name according second argument.
> Sorry, but I can't understand the above. Can you reword it, and perhaps
> include an example of what this patch does.

thanks, will try to update after Masai and Tom comment

>
> After reading the code, I guess you are trying to make it so that you can
> leave out the event name and only have the group name, if you add the slash.
>
> echo 'p:group/ schedule' > kprobe_events
>
> And that will create a kprobe "schedule" event under the "group" system?

that's right

>
> Also, for some reason, patch 2 of the series never came to me. But it is in
> my LKML folder :-/
>
>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
>> ---
>> Documentation/trace/kprobetrace.rst | 8 ++++----
>> Documentation/trace/uprobetracer.rst | 8 ++++----
>> kernel/trace/trace_dynevent.c | 2 +-
>> kernel/trace/trace_eprobe.c | 19 +++++++++++--------
>> kernel/trace/trace_kprobe.c | 18 +++++++++++-------
>> kernel/trace/trace_probe.c | 9 ++++++++-
>> kernel/trace/trace_probe.h | 4 ++++
>> kernel/trace/trace_uprobe.c | 14 +++++++++-----
>> 8 files changed, 52 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..b16e067 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,16 @@ static int __trace_eprobe_create(int argc, const char *argv[])
>>
>> trace_probe_log_init("event_probe", argc, argv);
>>
>> + ret = TP_ENAME_EMPTTY;
> EMPTY is spelled wrong.


sorry, will wait comment from  maintainer of V1,  will fix it in V2 if
V1 is not reject.


>> 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 {
>> + }
> add space.

sure, thanks.

>
>> + if (ret == TP_ENAME_EMPTTY || ret == TP_ENAME_GROUP) {
>> strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN);
>> sanitize_event_name(buf1);
>> event = buf1;
> I'd like to see Masami's and Tom's comments on this one.
>
> -- Steve
>
>> @@ -882,9 +886,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..2cd8ef9 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,14 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>> }
>>
>> trace_probe_log_set_index(0);
>> + ret = TP_ENAME_EMPTTY;
>> 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_EMPTTY || 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..d726769 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
>> +#define TP_ENAME_EVENT 1
>> +#define TP_ENAME_GROUP 2
>> +#define TP_ENAME_EMPTTY 3
>>
>> enum probe_print_type {
>> PROBE_PRINT_NORMAL,
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index 9711589..a935c08 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,14 @@ static int __trace_uprobe_create(int argc, const char **argv)
>>
>> /* setup a probe */
>> trace_probe_log_set_index(0);
>> + ret = TP_ENAME_EMPTTY;
>> 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_EMPTTY || ret == TP_ENAME_GROUP) {
>> char *tail;
>> char *ptr;
>>

\
 
 \ /
  Last update: 2022-05-25 03:35    [W:0.074 / U:1.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site