All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/4] tracing/probes: allow no event name input when create group
@ 2022-06-27  2:19 Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 1/4] tracing: eprobe: add missing log index Linyu Yuan
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Linyu Yuan @ 2022-06-27  2:19 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi; +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]

similar change apply to eprobe and uprobe.

V2: (v1: https://lore.kernel.org/lkml/1651397651-30454-1-git-send-email-quic_linyyuan@quicinc.com/)
    fix remove comment in V1 patch1,
    remove v1 patch2 as it is NACK.

v3: (v2 : https://lore.kernel.org/lkml/1653795294-19764-1-git-send-email-quic_linyyuan@quicinc.com/)
    add selftest cases for kprobe and eprobe event,
    remove macro used in v1,v2,
    change location to generate eprobe event name.

v4: (v3 : https://lore.kernel.org/lkml/1653978552-18637-1-git-send-email-quic_linyyuan@quicinc.com/)
    fix comment of kprobe/eprobe test case.

v5: (v4: https://lore.kernel.org/lkml/1654171861-24014-1-git-send-email-quic_linyyuan@quicinc.com/)
    for eprobe, when only input a "SYSTEM.", it is invalid.
    add Acked-by from Masami Hiramatsu (Google) <mhiramat@kernel.org>

v6: (v5: https://lore.kernel.org/lkml/1655168698-19898-1-git-send-email-quic_linyyuan@quicinc.com/)
    change some code order according review comment from Tom Zanussi,
    some minor changes.

v7: (v6: https://lore.kernel.org/lkml/1655776787-18133-1-git-send-email-quic_linyyuan@quicinc.com/)
    move log index to a separate patch according review comment

Linyu Yuan (4):
  tracing: eprobe: add missing log index
  tracing: eprobe: remove duplicate is_good_name() operation
  tracing: auto generate event name when create a group of events
  selftests/ftrace: add test case for GRP/ only input

 Documentation/trace/kprobetrace.rst                |  8 +++----
 Documentation/trace/uprobetracer.rst               |  8 +++----
 kernel/trace/trace.c                               |  8 +++----
 kernel/trace/trace_dynevent.c                      |  2 +-
 kernel/trace/trace_eprobe.c                        | 28 +++++++++++-----------
 kernel/trace/trace_kprobe.c                        | 16 ++++++++-----
 kernel/trace/trace_probe.c                         |  4 ++++
 kernel/trace/trace_uprobe.c                        | 12 ++++++----
 .../ftrace/test.d/dynevent/add_remove_eprobe.tc    |  9 ++++++-
 .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |  7 ++++++
 10 files changed, 64 insertions(+), 38 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v7 1/4] tracing: eprobe: add missing log index
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
@ 2022-06-27  2:19 ` Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 2/4] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Linyu Yuan @ 2022-06-27  2:19 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi; +Cc: linux-kernel, Linyu Yuan

Add trace_probe_log_set_index(1) to allow report correct error
if user input wrong SYSTEM.EVENT format.

Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v7: move one line change from v6 to a separate patch

 kernel/trace/trace_eprobe.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index 7d44785..b805b57 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -881,6 +881,7 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	if (!is_good_name(event) || !is_good_name(group))
 		goto parse_error;
 
+	trace_probe_log_set_index(1);
 	sys_event = argv[1];
 	ret = traceprobe_parse_event_name(&sys_event, &sys_name, buf2,
 					  sys_event - argv[1]);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 2/4] tracing: eprobe: remove duplicate is_good_name() operation
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 1/4] tracing: eprobe: add missing log index Linyu Yuan
@ 2022-06-27  2:19 ` Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 3/4] tracing: auto generate event name when create a group of events Linyu Yuan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Linyu Yuan @ 2022-06-27  2:19 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi; +Cc: linux-kernel, Linyu Yuan

traceprobe_parse_event_name() already validate SYSTEM and EVENT name,
there is no need to call is_good_name() after it.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
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().
v3: move it as first patch.
v4: no change
v5: add Acked-by tag
v6: keep is_good_name() check for group and event name
    add trace_probe_log_set_index(1) to report correct error message.
v7: move trace_probe_log_set_index() to seprate patch#1

 kernel/trace/trace_eprobe.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
index b805b57..8979cb9e 100644
--- a/kernel/trace/trace_eprobe.c
+++ b/kernel/trace/trace_eprobe.c
@@ -887,8 +887,6 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 					  sys_event - argv[1]);
 	if (ret || !sys_name)
 		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] 7+ messages in thread

* [PATCH v7 3/4] tracing: auto generate event name when create a group of events
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 1/4] tracing: eprobe: add missing log index Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 2/4] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan
@ 2022-06-27  2:19 ` Linyu Yuan
  2022-06-27  2:19 ` [PATCH v7 4/4] selftests/ftrace: add test case for GRP/ only input Linyu Yuan
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Linyu Yuan @ 2022-06-27  2:19 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi; +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.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
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.
v3: remove TP_ENAME_* macro which suggest by Masami Hiramatsu,
    allow generate event name from SYSTEM/EVENT for eprobe.
v4: no change
v5: when eprobe input "SYSTEM." is invalid,
    add Acked-by tag
v6: remove is_good_name() check of group and event
    remove snprintf() and keep origial strscpy()
    remove two empty lines.

 Documentation/trace/kprobetrace.rst  |  8 ++++----
 Documentation/trace/uprobetracer.rst |  8 ++++----
 kernel/trace/trace.c                 |  8 ++++----
 kernel/trace/trace_dynevent.c        |  2 +-
 kernel/trace/trace_eprobe.c          | 25 +++++++++++++------------
 kernel/trace/trace_kprobe.c          | 16 ++++++++++------
 kernel/trace/trace_probe.c           |  4 ++++
 kernel/trace/trace_uprobe.c          | 12 ++++++++----
 8 files changed, 48 insertions(+), 35 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.c b/kernel/trace/trace.c
index f400800b..76274c6 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5575,13 +5575,13 @@ static const char readme_msg[] =
 #endif
 #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"
+	"\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"
+	"\t           e[:[<group>/][<event>]] <attached-group>.<attached-event> [<args>]\n"
+	"\t           -:[<group>/][<event>]\n"
 #ifdef CONFIG_KPROBE_EVENTS
 	"\t    place: [<module>:]<symbol>[+<offset>]|<memaddr>\n"
   "place (kretprobe): [<module>:]<symbol>[+<offset>]%return|<memaddr>\n"
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 076b447..1549966 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 8979cb9e..a30f214 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 gbuf[MAX_EVENT_NAME_LEN];
 	int ret = 0;
 	int i;
 
@@ -869,25 +871,24 @@ static int __trace_eprobe_create(int argc, const char *argv[])
 	event = strchr(&argv[0][1], ':');
 	if (event) {
 		event++;
-		ret = traceprobe_parse_event_name(&event, &group, buf1,
+		ret = traceprobe_parse_event_name(&event, &group, gbuf,
 						  event - argv[0]);
 		if (ret)
 			goto parse_error;
-	} else {
-		strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN);
-		sanitize_event_name(buf1);
-		event = buf1;
 	}
-	if (!is_good_name(event) || !is_good_name(group))
-		goto parse_error;
 
 	trace_probe_log_set_index(1);
 	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 (!sys_event || !sys_name)
 		goto parse_error;
 
+	if (!event) {
+		strscpy(buf1, argv[1], MAX_EVENT_NAME_LEN);
+		sanitize_event_name(buf1);
+		event = buf1;
+	}
+
 	mutex_lock(&event_mutex);
 	event_call = find_and_get_event(sys_name, sys_event);
 	ep = alloc_event_probe(group, event, event_call, argc - 2);
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9350733..2c36a71 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 gbuf[MAX_EVENT_NAME_LEN];
 	unsigned int flags = TPARG_FL_KERNEL;
 
 	switch (argv[0][0]) {
@@ -833,11 +835,13 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 
 	trace_probe_log_set_index(0);
 	if (event) {
-		ret = traceprobe_parse_event_name(&event, &group, buf,
+		ret = traceprobe_parse_event_name(&event, &group, gbuf,
 						  event - argv[0]);
 		if (ret)
 			goto parse_error;
-	} else {
+	}
+
+	if (!event) {
 		/* 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..850a88a 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -257,6 +257,10 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup,
 	}
 	len = strlen(event);
 	if (len == 0) {
+		if (slash) {
+			*pevent = NULL;
+			return 0;
+		}
 		trace_probe_log_err(offset, NO_EVENT_NAME);
 		return -EINVAL;
 	} else if (len > MAX_EVENT_NAME_LEN) {
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 9711589..5af0e85 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 gbuf[MAX_EVENT_NAME_LEN];
 	enum probe_print_type ptype;
 	struct path path;
 	unsigned long offset, ref_ctr_offset;
@@ -645,11 +647,13 @@ static int __trace_uprobe_create(int argc, const char **argv)
 	/* setup a probe */
 	trace_probe_log_set_index(0);
 	if (event) {
-		ret = traceprobe_parse_event_name(&event, &group, buf,
+		ret = traceprobe_parse_event_name(&event, &group, gbuf,
 						  event - argv[0]);
 		if (ret)
 			goto fail_address_parse;
-	} else {
+	}
+
+	if (!event) {
 		char *tail;
 		char *ptr;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v7 4/4] selftests/ftrace: add test case for GRP/ only input
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
                   ` (2 preceding siblings ...)
  2022-06-27  2:19 ` [PATCH v7 3/4] tracing: auto generate event name when create a group of events Linyu Yuan
@ 2022-06-27  2:19 ` Linyu Yuan
  2022-07-05 21:46 ` [PATCH v7 0/4] tracing/probes: allow no event name input when create group Tom Zanussi
  2022-07-12 19:55 ` Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Linyu Yuan @ 2022-06-27  2:19 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Tom Zanussi; +Cc: linux-kernel, Linyu Yuan

add kprobe and eprobe event test for new GRP/ only format.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
---
v3: first add in this version
v4: remove restriction of test case
v5: add Acked-by tag
v6: no change
v7: no change

 .../selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc        | 9 ++++++++-
 .../selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc        | 7 +++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
index 60c02b4..c300eb0 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_eprobe.tc
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
 # description: Generic dynamic event - add/remove eprobe events
-# requires: dynamic_events events/syscalls/sys_enter_openat "e[:[<group>/]<event>] <attached-group>.<attached-event> [<args>]":README
+# requires: dynamic_events events/syscalls/sys_enter_openat "<attached-group>.<attached-event> [<args>]":README
 
 echo 0 > events/enable
 
@@ -87,4 +87,11 @@ echo "-:eprobes/$EPROBE $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
 ! grep -q "$EPROBE" dynamic_events
 ! test -d events/eprobes/$EPROBE
 
+if grep -q "e\[:\[<group>/]\[<event>]]" README; then
+	echo "e:mygroup/ $SYSTEM/$EVENT $OPTIONS" >> dynamic_events
+	test -d events/mygroup
+	echo "-:mygroup/" >> dynamic_events
+	! test -d events/mygroup
+fi
+
 clear_trace
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
index b4da41d..13d43f4 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
@@ -23,4 +23,11 @@ grep -q myevent1 dynamic_events
 
 echo > dynamic_events
 
+if grep -q "p\[:\[<group>/]\[<event>]]" README; then
+	echo "p:mygroup/ $PLACE" >> dynamic_events
+	test -d events/mygroup
+	echo "-:mygroup/" >> dynamic_events
+	! test -d events/mygroup
+fi
+
 clear_trace
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 0/4] tracing/probes: allow no event name input when create group
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
                   ` (3 preceding siblings ...)
  2022-06-27  2:19 ` [PATCH v7 4/4] selftests/ftrace: add test case for GRP/ only input Linyu Yuan
@ 2022-07-05 21:46 ` Tom Zanussi
  2022-07-12 19:55 ` Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Zanussi @ 2022-07-05 21:46 UTC (permalink / raw)
  To: Linyu Yuan, Steven Rostedt, Masami Hiramatsu; +Cc: linux-kernel

Hi Yuan,

On Mon, 2022-06-27 at 10:19 +0800, Linyu Yuan wrote:
> 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]
> 
> similar change apply to eprobe and uprobe.

Thanks for making those changes!

For the series,

Reviewed-by: Tom Zanussi <zanussi@kernel.org>

Tom

> 
> V2: (v1:
> https://lore.kernel.org/lkml/1651397651-30454-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     fix remove comment in V1 patch1,
>     remove v1 patch2 as it is NACK.
> 
> v3: (v2 :
> https://lore.kernel.org/lkml/1653795294-19764-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     add selftest cases for kprobe and eprobe event,
>     remove macro used in v1,v2,
>     change location to generate eprobe event name.
> 
> v4: (v3 :
> https://lore.kernel.org/lkml/1653978552-18637-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     fix comment of kprobe/eprobe test case.
> 
> v5: (v4:
> https://lore.kernel.org/lkml/1654171861-24014-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     for eprobe, when only input a "SYSTEM.", it is invalid.
>     add Acked-by from Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> v6: (v5:
> https://lore.kernel.org/lkml/1655168698-19898-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     change some code order according review comment from Tom Zanussi,
>     some minor changes.
> 
> v7: (v6:
> https://lore.kernel.org/lkml/1655776787-18133-1-git-send-email-quic_linyyuan@quicinc.com/
> )
>     move log index to a separate patch according review comment
> 
> Linyu Yuan (4):
>   tracing: eprobe: add missing log index
>   tracing: eprobe: remove duplicate is_good_name() operation
>   tracing: auto generate event name when create a group of events
>   selftests/ftrace: add test case for GRP/ only input
> 
>  Documentation/trace/kprobetrace.rst                |  8 +++----
>  Documentation/trace/uprobetracer.rst               |  8 +++----
>  kernel/trace/trace.c                               |  8 +++----
>  kernel/trace/trace_dynevent.c                      |  2 +-
>  kernel/trace/trace_eprobe.c                        | 28 +++++++++++-
> ----------
>  kernel/trace/trace_kprobe.c                        | 16 ++++++++----
> -
>  kernel/trace/trace_probe.c                         |  4 ++++
>  kernel/trace/trace_uprobe.c                        | 12 ++++++----
>  .../ftrace/test.d/dynevent/add_remove_eprobe.tc    |  9 ++++++-
>  .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |  7 ++++++
>  10 files changed, 64 insertions(+), 38 deletions(-)
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v7 0/4] tracing/probes: allow no event name input when create group
  2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
                   ` (4 preceding siblings ...)
  2022-07-05 21:46 ` [PATCH v7 0/4] tracing/probes: allow no event name input when create group Tom Zanussi
@ 2022-07-12 19:55 ` Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2022-07-12 19:55 UTC (permalink / raw)
  To: Linyu Yuan; +Cc: Masami Hiramatsu, Tom Zanussi, linux-kernel

On Mon, 27 Jun 2022 10:19:04 +0800
Linyu Yuan <quic_linyyuan@quicinc.com> wrote:

> 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]
> 
> similar change apply to eprobe and uprobe.
> 


With this patch set applied, this selftest fails:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc

This exactly:

  echo 'p:foo/ vfs_read' > kprobe_events

no longer fails as the test expects it to.

I guess we need to remove that check as it now works and no longer fails as
"expected".

This change fixes it.

If it's OK with everyone, I'll apply this change.

-- Steve

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
index fa928b431555..7c02509c71d0 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_syntax_errors.tc
@@ -21,7 +21,6 @@ check_error 'p:^/bar vfs_read'		# NO_GROUP_NAME
 check_error 'p:^12345678901234567890123456789012345678901234567890123456789012345/bar vfs_read'	# GROUP_TOO_LONG
 
 check_error 'p:^foo.1/bar vfs_read'	# BAD_GROUP_NAME
-check_error 'p:foo/^ vfs_read'		# NO_EVENT_NAME
 check_error 'p:foo/^12345678901234567890123456789012345678901234567890123456789012345 vfs_read'	# EVENT_TOO_LONG
 check_error 'p:foo/^bar.1 vfs_read'	# BAD_EVENT_NAME
 

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-07-12 19:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27  2:19 [PATCH v7 0/4] tracing/probes: allow no event name input when create group Linyu Yuan
2022-06-27  2:19 ` [PATCH v7 1/4] tracing: eprobe: add missing log index Linyu Yuan
2022-06-27  2:19 ` [PATCH v7 2/4] tracing: eprobe: remove duplicate is_good_name() operation Linyu Yuan
2022-06-27  2:19 ` [PATCH v7 3/4] tracing: auto generate event name when create a group of events Linyu Yuan
2022-06-27  2:19 ` [PATCH v7 4/4] selftests/ftrace: add test case for GRP/ only input Linyu Yuan
2022-07-05 21:46 ` [PATCH v7 0/4] tracing/probes: allow no event name input when create group Tom Zanussi
2022-07-12 19:55 ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.