[PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[Metin Kaya]

We may want to preserve some of the dynamic events (e.g., kprobes)
during a "trace-cmd reset" in some cases as discussed at [1]. Thus,
introduce a new command line option to "trace-cmd reset" to permit
sustaining specified events across reset.

To implement the necessary support, address a corner case in command
line option parsing handler in "trace-cmd reset" first. Because all
dynamic events gets destroyed even if the user passes an invalid
parameter to "trace-cmd reset".

Here is an example usage with the new command line option (-k):

    # trace-cmd reset -h

    trace-cmd version 3.3.1 (841c96c66b529d52bfce9e84a533fa904ba954ed)

    usage:
     trace-cmd reset [-b size][-B buf][-k event][-a][-d][-t]
              Disables the tracer (may reset trace file)
              Used in conjunction with start
              -b change the kernel buffer size (in kilobytes per CPU)
              -d delete the previous specified instance
              -B reset the given buffer instance (may specify multiple -B)
              -a reset all instances (except top one)
              -t reset the top level instance (useful with -B or -a)
              -k keep dynamic event during reset (can be specified multiple times).
                  Valid values are:
                  'kprobe', 'kretprobe', 'uprobe', 'uretprobe',
                  'eprobe', 'synth' and 'all'.

    # Add kprobe and kretprobe for sys_open():
    $ echo 'p do_sys_open' > /sys/kernel/tracing/kprobe_events
    $ echo 1 > /sys/kernel/tracing/events/kprobes/p_do_sys_open_0/enable
    $ echo 'r do_sys_open' >> /sys/kernel/tracing/kprobe_events
    $ echo 1 > /sys/kernel/tracing/events/kprobes/r_do_sys_open_0/enable
    $ cat /sys/kernel/tracing/kprobe_events
    p:kprobes/p_do_sys_open_0 do_sys_open
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Issue reset, but keep kprobes and kretprobes ('-k all' would keep
    # all existing dynamic events):
    $ trace-cmd reset -k kprobe -k kretprobe
    $ cat /sys/kernel/tracing/kprobe_events
    p:kprobes/p_do_sys_open_0 do_sys_open
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Issue reset, but this time only keep kretprobes:
    $ trace-cmd reset -k kretprobe
    $ cat /sys/kernel/tracing/kprobe_events
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Don't preserve any dynamic event:
    $ trace-cmd reset
    $ cat /sys/kernel/tracing/kprobe_events
    $

Changes since v1:
- Update man page for -k command line parameter of "trace-cmd reset".
- Add tab completion support for "trace-cmd reset".

[1] https://bugzilla.kernel.org/show_bug.cgi?id=219302

Metin Kaya (4):
  trace-cmd reset: Bail out immediately if user provides an invalid
    option
  trace-cmd reset: Add option to preserve specific dynamic events
  trace-cmd reset: Update man page for -k option
  trace-cmd reset: Add bash tab completion for -B and -k

 Documentation/trace-cmd/trace-cmd-reset.1.txt | 12 +++-
 tracecmd/trace-cmd.bash                       | 31 ++++++++++
 tracecmd/trace-record.c                       | 62 +++++++++++++++++--
 tracecmd/trace-usage.c                        |  6 +-
 4 files changed, 103 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] trace-cmd reset: Bail out immediately if user provides an invalid option

[Metin Kaya]

trace_reset() does not care invalid command line options and carries on
execution. For instance, "trace-cmd reset -x" command successfully
completes the reset request (despite of "reset: invalid option -- 'x'"
warning). Thus, show the usage message and terminate execution instead
of running the reset routine.

Signed-off-by: Metin Kaya <metin.kaya@arm.com>
---
 tracecmd/trace-record.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 0063d528..8c0039d4 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -6102,6 +6102,9 @@ void trace_reset(int argc, char **argv)
 				instance->flags &= ~BUFFER_FL_KEEP;
 			}
 			break;
+		default:
+			usage(argv);
+			break;
 		}
 	}
 	update_first_instance(instance, topt);
-- 
2.34.1

[PATCH v2 2/4] trace-cmd reset: Add option to preserve specific dynamic events

[Metin Kaya]

One may want to preserve some of the dynamic events (e.g., kprobes)
during a trace-cmd reset. Thus, provide -k command line option for
trace-cmd reset to allow keeping specified events untouched during the
reset operation.

For example, it's possible to prevent kprobes & kretprobes from being
destroyed in trace-cmd reset with this patch:

    # Add kprobe and kretprobe for sys_open():
    $ echo 'p do_sys_open' > /sys/kernel/tracing/kprobe_events
    $ echo 1 > /sys/kernel/tracing/events/kprobes/p_do_sys_open_0/enable
    $ echo 'r do_sys_open' >> /sys/kernel/tracing/kprobe_events
    $ echo 1 > /sys/kernel/tracing/events/kprobes/r_do_sys_open_0/enable
    $ cat /sys/kernel/tracing/kprobe_events
    p:kprobes/p_do_sys_open_0 do_sys_open
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Issue reset, but keep kprobes and kretprobes ('-k all' would keep
    # all existing dynamic events):
    $ trace-cmd reset -k kprobe -k kretprobe
    $ cat /sys/kernel/tracing/kprobe_events
    p:kprobes/p_do_sys_open_0 do_sys_open
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Issue reset, but this time only keep kretprobes:
    $ trace-cmd reset -k kretprobe
    $ cat /sys/kernel/tracing/kprobe_events
    r128:kprobes/r_do_sys_open_0 do_sys_open

    # Don't preserve any dynamic event:
    $ trace-cmd reset
    $ cat /sys/kernel/tracing/kprobe_events
    $

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=219302
Signed-off-by: Metin Kaya <metin.kaya@arm.com>
---
 tracecmd/trace-record.c | 59 ++++++++++++++++++++++++++++++++++++-----
 tracecmd/trace-usage.c  |  6 ++++-
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 8c0039d4..a008cdfd 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -5399,11 +5399,15 @@ static void clear_error_log(void)
 		clear_instance_error_log(instance);
 }
 
-static void clear_all_dynamic_events(void)
+static void clear_all_dynamic_events(unsigned int excluded_types)
 {
-	/* Clear event probes first, as they may be attached to other dynamic event */
-	tracefs_dynevent_destroy_all(TRACEFS_DYNEVENT_EPROBE, true);
-	tracefs_dynevent_destroy_all(TRACEFS_DYNEVENT_ALL, true);
+	/*
+	 * Clear event probes first (if they are not excluded), as they may be
+	 * attached to other dynamic event.
+	 */
+	if (!(excluded_types & TRACEFS_DYNEVENT_EPROBE))
+		tracefs_dynevent_destroy_all(TRACEFS_DYNEVENT_EPROBE, true);
+	tracefs_dynevent_destroy_all(~excluded_types & TRACEFS_DYNEVENT_ALL, true);
 }
 
 static void clear_func_filters(void)
@@ -6036,11 +6040,51 @@ void trace_restart(int argc, char **argv)
 	exit(0);
 }
 
+/**
+ * find_dynevent_type - Find string formatted dynamic event type
+ *			in "enum tracefs_dynevent_type".
+ *
+ * @type: Dynamic event type in string format.
+ *
+ * Returns an unsigned int value for the event specified in @type.
+ */
+static unsigned int find_dynevent_type(const char *type)
+{
+	/* WARN: Should be kept in sync with "enum tracefs_dynevent_type". */
+	if (!strcmp(type, "kprobe"))
+		return TRACEFS_DYNEVENT_KPROBE;
+	else if (!strcmp(type, "kretprobe"))
+		return TRACEFS_DYNEVENT_KRETPROBE;
+	else if (!strcmp(type, "uprobe"))
+		return TRACEFS_DYNEVENT_UPROBE;
+	else if (!strcmp(type, "uretprobe"))
+		return TRACEFS_DYNEVENT_URETPROBE;
+	else if (!strcmp(type, "eprobe"))
+		return TRACEFS_DYNEVENT_EPROBE;
+	else if (!strcmp(type, "synth"))
+		return TRACEFS_DYNEVENT_SYNTH;
+	else if (!strcmp(type, "all"))
+		/*
+		 * Unfortunately TRACEFS_DYNEVENT_ALL does not work here.
+		 * Because tracefs_dynevent_destroy_all() assumes 0 means all
+		 * and destroys all dynamic events.
+		 */
+		return TRACEFS_DYNEVENT_KPROBE |
+		       TRACEFS_DYNEVENT_KRETPROBE |
+		       TRACEFS_DYNEVENT_UPROBE |
+		       TRACEFS_DYNEVENT_URETPROBE |
+		       TRACEFS_DYNEVENT_EPROBE |
+		       TRACEFS_DYNEVENT_SYNTH;
+	else
+		die("Invalid event type '%s'!\n", type);
+}
+
 void trace_reset(int argc, char **argv)
 {
 	int c;
 	int topt = 0;
 	struct buffer_instance *instance = &top_instance;
+	unsigned int excluded_types = 0;
 
 	init_top_instance();
 
@@ -6048,7 +6092,7 @@ void trace_reset(int argc, char **argv)
 	int last_specified_all = 0;
 	struct buffer_instance *inst; /* iterator */
 
-	while ((c = getopt(argc-1, argv+1, "hab:B:td")) >= 0) {
+	while ((c = getopt(argc-1, argv+1, "hab:B:tdk:")) >= 0) {
 
 		switch (c) {
 		case 'h':
@@ -6102,6 +6146,9 @@ void trace_reset(int argc, char **argv)
 				instance->flags &= ~BUFFER_FL_KEEP;
 			}
 			break;
+		case 'k':
+			excluded_types |= find_dynevent_type(optarg);
+			break;
 		default:
 			usage(argv);
 			break;
@@ -6112,7 +6159,7 @@ void trace_reset(int argc, char **argv)
 	set_buffer_size();
 	clear_filters();
 	clear_triggers();
-	clear_all_dynamic_events();
+	clear_all_dynamic_events(excluded_types);
 	clear_error_log();
 	/* set clock to "local" */
 	reset_clock();
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 8bbf2e3e..7d61a371 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -193,7 +193,7 @@ static struct usage_help usage_help[] = {
 	{
 		"reset",
 		"disable all kernel tracing and clear the trace buffers",
-		" %s reset [-b size][-B buf][-a][-d][-t]\n"
+		" %s reset [-b size][-B buf][-k event][-a][-d][-t]\n"
 		"          Disables the tracer (may reset trace file)\n"
 		"          Used in conjunction with start\n"
 		"          -b change the kernel buffer size (in kilobytes per CPU)\n"
@@ -201,6 +201,10 @@ static struct usage_help usage_help[] = {
 		"          -B reset the given buffer instance (may specify multiple -B)\n"
 		"          -a reset all instances (except top one)\n"
 		"          -t reset the top level instance (useful with -B or -a)\n"
+		"          -k keep dynamic event during reset (can be specified multiple times).\n"
+		"              Valid values are:\n"
+		"              'kprobe', 'kretprobe', 'uprobe', 'uretprobe',\n"
+		"              'eprobe', 'synth' and 'all'.\n"
 	},
 	{
 		"clear",
-- 
2.34.1

[PATCH v2 3/4] trace-cmd reset: Update man page for -k option

[Metin Kaya]

Talk about the new -k command line parameter of "trace-cmd reset" in the
trace-cmd man page.

While we are here, fix typo 'immediatly'.

Signed-off-by: Metin Kaya <metin.kaya@arm.com>
---
 Documentation/trace-cmd/trace-cmd-reset.1.txt | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace-cmd/trace-cmd-reset.1.txt b/Documentation/trace-cmd/trace-cmd-reset.1.txt
index eee86751..026232ac 100644
--- a/Documentation/trace-cmd/trace-cmd-reset.1.txt
+++ b/Documentation/trace-cmd/trace-cmd-reset.1.txt
@@ -60,7 +60,7 @@ significant. See EXAMPLES.
 *-d*::
     This option deletes the instance buffer(s) specified by the most recently
     preceding *-B* or *-a* option. Because the top-level instance buffer
-    cannot be deleted, it is invalid to use this immediatly following *-t* or
+    cannot be deleted, it is invalid to use this immediately following *-t* or
     prior to any *-B* or *-a* option on the command line.
 
 *-t*::
@@ -68,6 +68,12 @@ significant. See EXAMPLES.
     this is the same as the default. But if *-B* or *-a* is used, this is
     required if the top level instance buffer should also be reset.
 
+*-k* 'dynevent-name'::
+    This option allows preserving specified dynamic event during reset. Valid
+    parameters are *kprobe*, *kretprobe*, *uprobe*, *uretprobe*, *eprobe*,
+    *synth* and *all* (for keeping all dynamic events). This may be used
+    multiple times to specify different dynamic event types.
+
 EXAMPLES
 --------
 
@@ -94,6 +100,10 @@ instance buffers exist, they will be unaffected:
 
     trace-cmd reset -b 1024
 
+Prevent *kprobes* and *kretprobes* from being destroyed during reset:
+
+    trace-cmd reset -k kprobe -k kretprobe
+
 
 SEE ALSO
 --------
-- 
2.34.1

[PATCH v2 4/4] trace-cmd reset: Add bash tab completion for -B and -k

[Metin Kaya]

Implement the placeholder for tab completion of "trace-cmd reset".

Apparently trace-cmd.bash already has support for instance buffers
lookup (which is show_instances()). Employ it for -B option.

Regarding -k option, show the list of dynamic event types to ease
trace-cmd user's job.

Signed-off-by: Metin Kaya <metin.kaya@arm.com>
---
 tracecmd/trace-cmd.bash | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tracecmd/trace-cmd.bash b/tracecmd/trace-cmd.bash
index 66bd6f4b..01a75578 100644
--- a/tracecmd/trace-cmd.bash
+++ b/tracecmd/trace-cmd.bash
@@ -215,6 +215,33 @@ __trace_cmd_report_complete()
     esac
 }
 
+dynevent_options()
+{
+    local cur="$1"
+    local opts=("kprobe" "kretprobe" "uprobe" "uretprobe" "eprobe" "synth" "all")
+    COMPREPLY=( $(compgen -W "${opts[*]}" -- "${cur}") )
+}
+
+__trace_cmd_reset_complete()
+{
+    local prev=$1
+    local cur=$2
+    shift 2
+    local words=("$@")
+
+    case "$prev" in
+        -B)
+            show_instances "$cur"
+            ;;
+        -k)
+            dynevent_options "$cur"
+            ;;
+        *)
+            cmd_options reset "$cur"
+            ;;
+    esac
+}
+
 __trace_cmd_dump_complete()
 {
     local prev=$1
@@ -329,6 +356,10 @@ _trace_cmd_complete()
 	    __trace_cmd_report_complete "${prev}" "${cur}" ${words[@]}
 	    return 0
 	    ;;
+	reset)
+	    __trace_cmd_reset_complete "${prev}" "${cur}" "${words[@]}"
+	    return 0
+	    ;;
 	dump)
 	    __trace_cmd_dump_complete "${prev}" "${cur}" ${words[@]}
 	    return 0
-- 
2.34.1

Re: [PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[Steven Rostedt]

On Mon, 14 Oct 2024 13:31:32 +0100
Metin Kaya <metin.kaya@arm.com> wrote:

> Changes since v1:
> - Update man page for -k command line parameter of "trace-cmd reset".
> - Add tab completion support for "trace-cmd reset".

Thanks, I'll go ahead and apply this. But can you do me a favor, and add a
test case to utest/tracecmd-utest.c that tests this. It should create a
each of the types, start tracing, do the reset, and make sure that the -k
keeps what is expected. Doesn't need to be too fancy.

I want to add test cases to all new features, as I'm way behind in testing
current features :-p

You don't need to send another version of this series. Just a standalone patch.

-- Steve

Re: [PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[Metin Kaya]

On 14/10/2024 10:59 pm, Steven Rostedt wrote:
> On Mon, 14 Oct 2024 13:31:32 +0100
> Metin Kaya <metin.kaya@arm.com> wrote:
>
>> Changes since v1:
>> - Update man page for -k command line parameter of "trace-cmd reset".
>> - Add tab completion support for "trace-cmd reset".
>
> Thanks, I'll go ahead and apply this. But can you do me a favor, and add a
> test case to utest/tracecmd-utest.c that tests this. It should create a
> each of the types, start tracing, do the reset, and make sure that the -k
> keeps what is expected. Doesn't need to be too fancy.

I wrote the patch below, but noticed I'm unable to preserve uprobes
during reset. I verified trace-cmd tries to prevent
TRACEFS_DYNEVENT_UPROBE from being destroyed, but I cannot see them
after running "trace-cmd reset -k uprobe" command.
The same problem exists for uretprobe, too.
I confirmed utest uses correct trace-cmd executable.

Any ideas?

-->8--

commit 01d0822dc5e22a34155b0a70d56c67fa023ccacc
Author: Metin Kaya <metin.kaya@arm.com>
Date:   Tue Oct 15 09:34:49 2024 +0100

     trace-cmd utest: Add test cases for trace-cmd reset

     Implement test cases for dynamic event types kprobe, kretprobe, uprobe,
     uretprobe and eprobe.

     Verify resetting synthetic events by inserting a hook to
     test_trace_sqlhist_hist().

     While we are in the neighborhood, fix typo "ceated".

     Link:
https://lore.kernel.org/linux-trace-devel/20241014175905.03bec85a@gandalf.local.home

     Signed-off-by: Metin Kaya <metin.kaya@arm.com>

diff --git a/utest/tracecmd-utest.c b/utest/tracecmd-utest.c
index 164f5da1..c1a12965 100644
--- a/utest/tracecmd-utest.c
+++ b/utest/tracecmd-utest.c
@@ -352,6 +352,11 @@ static void test_trace_sqlhist_hist(void)
        sleep(1);
        ret = grep_match(SYNTH_EVENT ":", "show", NULL);
        CU_TEST(ret == 0);
+       /* Ensure synthetic events remain untouched after "trace-cmd reset -k
synth". */
+       ret = run_trace("reset", "-k", "synth", NULL);
+       CU_TEST(ret == 0);
+       ret = grep_match(SYNTH_EVENT, "stat", NULL);
+       CU_TEST(ret == 0);

        tracefs_instance_reset(NULL);
  }
@@ -598,6 +603,159 @@ static void test_trace_library_read_back(void)
        tracecmd_close(handle);
  }

+static void test_trace_reset_kprobe(void)
+{
+       int ret;
+       FILE *fp;
+
+       /* Create a simple kprobe for do_sys_open */
+       fp = fopen("/sys/kernel/tracing/kprobe_events", "w");
+       CU_TEST(fp != NULL);
+       fprintf(fp, "p do_sys_open");
+       fclose(fp);
+
+       /* Ensure the kprobe is listed in "trace-cmd stat" output. */
+       ret = grep_match("p:kprobes/p_do_sys_open_0 do_sys_open", "stat", NULL);
+       CU_TEST(ret == 0);
+
+       /* Issue "trace-cmd reset", but keep kprobes. */
+       ret = run_trace("reset", "-k", "kprobe", NULL);
+       CU_TEST(ret == 0);
+
+       /* Verify the kprobe's existence after reset. */
+       ret = grep_match("p:kprobes/p_do_sys_open_0 do_sys_open", "stat", NULL);
+       CU_TEST(ret == 0);
+}
+
+static void test_trace_reset_kretprobe(void)
+{
+       int ret;
+       FILE *fp;
+
+       /* Create a simple kretprobe for do_sys_open */
+       fp = fopen("/sys/kernel/tracing/kprobe_events", "w");
+       CU_TEST(fp != NULL);
+       fprintf(fp, "r do_sys_open");
+       fclose(fp);
+
+       /* Ensure the kretprobe is listed in "trace-cmd stat" output. */
+       ret = grep_match("r128:kprobes/r_do_sys_open_0 do_sys_open", "stat",
NULL);
+       CU_TEST(ret == 0);
+
+       /* Issue "trace-cmd reset", but keep kretprobes. */
+       ret = run_trace("reset", "-k", "kretprobe", NULL);
+       CU_TEST(ret == 0);
+
+       /* Verify the kretprobe's existence after reset. */
+       ret = grep_match("r128:kprobes/r_do_sys_open_0 do_sys_open", "stat",
NULL);
+       CU_TEST(ret == 0);
+}
+
+static void test_trace_reset_uprobe(void)
+{
+       int ret;
+       FILE *fp;
+
+       /* Create a simple uprobe for do_sys_open */
+       fp = fopen("/sys/kernel/tracing/uprobe_events", "w");
+       CU_TEST(fp != NULL);
+       fprintf(fp, "p /bin/bash:0x4245c0");
+       fclose(fp);
+
+       /* Ensure the uprobe is listed in "trace-cmd stat" output. */
+       ret = grep_match("p:uprobes/p_bash_0x4245c0
/bin/bash:0x00000000004245c0", "stat", NULL);
+       CU_TEST(ret == 0);
+
+       /* Issue "trace-cmd reset", but keep uprobes. */
+       ret = run_trace("reset", "-k", "uprobe", NULL);
+       CU_TEST(ret == 0);
+
+       /* Verify the uprobe's existence after reset. */
+       ret = grep_match("p:uprobes/p_bash_0x4245c0
/bin/bash:0x00000000004245c0", "stat", NULL);
+       CU_TEST(ret == 0); // TODO: FAILS!
+}
+
+static void test_trace_reset_uretprobe(void)
+{
+       int ret;
+       FILE *fp;
+
+       /* Create a simple uretprobe for do_sys_open */
+       fp = fopen("/sys/kernel/tracing/uprobe_events", "w");
+       CU_TEST(fp != NULL);
+       fprintf(fp, "r /bin/bash:0x4245c0");
+       fclose(fp);
+
+       /* Ensure the uretprobe is listed in "trace-cmd stat" output. */
+       ret = grep_match("r:uprobes/p_bash_0x4245c0
/bin/bash:0x00000000004245c0", "stat", NULL);
+       CU_TEST(ret == 0);
+
+       /* Issue "trace-cmd reset", but keep uretprobes. */
+       ret = run_trace("reset", "-k", "uretprobe", NULL);
+       CU_TEST(ret == 0);
+
+       /* Verify the uretprobe's existence after reset. */
+       ret = grep_match("r:uprobes/p_bash_0x4245c0
/bin/bash:0x00000000004245c0", "stat", NULL);
+       CU_TEST(ret == 0); // TODO: FAILS!
+}
+
+static void test_trace_reset_eprobe(void)
+{
+       int ret;
+       bool matched = false;
+       size_t l = 0;
+       ssize_t n;
+       char *buf = NULL;
+       struct tracefs_dynevent *deprobe;
+       FILE *fp;
+
+       deprobe = tracefs_eprobe_alloc(NULL, "sopen_in", "syscalls",
"sys_enter_openat", NULL);
+       CU_TEST(deprobe != NULL);
+
+       ret = tracefs_dynevent_create(deprobe);
+       CU_TEST(ret == 0);
+
+       /* Issue "trace-cmd reset", but keep eprobes. */
+       ret = run_trace("reset", "-k", "eprobe", NULL);
+       CU_TEST(ret == 0);
+
+       /* Verify the eprobe's existence after reset. */
+       fp = fopen("/sys/kernel/tracing/dynamic_events", "r");
+       CU_TEST(fp != NULL);
+
+       while ((n = getline(&buf, &l, fp)) != -1) {
+               if (!strcmp(buf, "e:eprobes/sopen_in syscalls.sys_enter_openat\n")) {
+                       matched = true;
+                       break;
+               }
+       }
+       free(buf);
+
+       fclose(fp);
+
+       CU_TEST(matched == true);
+
+       ret = tracefs_dynevent_destroy(deprobe, false);
+       CU_TEST(ret == 0);
+
+       tracefs_dynevent_free(deprobe);
+}
+
+static void test_trace_reset(void)
+{
+       int ret;
+
+       test_trace_reset_kprobe();
+       test_trace_reset_kretprobe();
+       test_trace_reset_uprobe();
+       test_trace_reset_uretprobe();
+       test_trace_reset_eprobe();
+
+       /* Destroy all dynamic events. */
+       ret = run_trace("reset", NULL);
+       CU_TEST(ret == 0);
+}
+
  static int test_suite_destroy(void)
  {
        unlink(TRACECMD_FILE);
@@ -639,7 +797,7 @@ void test_tracecmd_lib(void)

        suite = CU_add_suite(TRACECMD_SUITE, test_suite_init,
test_suite_destroy);
        if (suite == NULL) {
-               fprintf(stderr, "Suite \"%s\" cannot be ceated\n", TRACECMD_SUITE);
+               fprintf(stderr, "Suite \"%s\" cannot be created\n", TRACECMD_SUITE);
                return;
        }
        CU_add_test(suite, "Simple record and report",
@@ -656,4 +814,5 @@ void test_tracecmd_lib(void)
                    test_trace_library_read_back);
        CU_add_test(suite, "Test max length",
                    test_trace_record_max);
+       CU_add_test(suite, "Simple reset", test_trace_reset);
  }

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[Steven Rostedt]

On Tue, 15 Oct 2024 14:30:58 +0100
Metin Kaya <metin.kaya@arm.com> wrote:

> On 14/10/2024 10:59 pm, Steven Rostedt wrote:
> > On Mon, 14 Oct 2024 13:31:32 +0100
> > Metin Kaya <metin.kaya@arm.com> wrote:
> >  
> >> Changes since v1:
> >> - Update man page for -k command line parameter of "trace-cmd reset".
> >> - Add tab completion support for "trace-cmd reset".  
> >
> > Thanks, I'll go ahead and apply this. But can you do me a favor, and add a
> > test case to utest/tracecmd-utest.c that tests this. It should create a
> > each of the types, start tracing, do the reset, and make sure that the -k
> > keeps what is expected. Doesn't need to be too fancy.  
> 
> I wrote the patch below, but noticed I'm unable to preserve uprobes
> during reset. I verified trace-cmd tries to prevent
> TRACEFS_DYNEVENT_UPROBE from being destroyed, but I cannot see them
> after running "trace-cmd reset -k uprobe" command.
> The same problem exists for uretprobe, too.
> I confirmed utest uses correct trace-cmd executable.
> 
> Any ideas?

I'll check it out, but your patch below is all messed up. Can you send it
properly?

Thanks,

-- Steve


> 
> -->8--  
> 
> commit 01d0822dc5e22a34155b0a70d56c67fa023ccacc
> Author: Metin Kaya <metin.kaya@arm.com>
> Date:   Tue Oct 15 09:34:49 2024 +0100
> 
>      trace-cmd utest: Add test cases for trace-cmd reset
> 
>      Implement test cases for dynamic event types kprobe, kretprobe, uprobe,
>      uretprobe and eprobe.
> 
>      Verify resetting synthetic events by inserting a hook to
>      test_trace_sqlhist_hist().
> 
>      While we are in the neighborhood, fix typo "ceated".
> 
>      Link:
> https://lore.kernel.org/linux-trace-devel/20241014175905.03bec85a@gandalf.local.home
> 
>      Signed-off-by: Metin Kaya <metin.kaya@arm.com>
> 
> diff --git a/utest/tracecmd-utest.c b/utest/tracecmd-utest.c
> index 164f5da1..c1a12965 100644
> --- a/utest/tracecmd-utest.c
> +++ b/utest/tracecmd-utest.c
> @@ -352,6 +352,11 @@ static void test_trace_sqlhist_hist(void)
>         sleep(1);
>         ret = grep_match(SYNTH_EVENT ":", "show", NULL);
>         CU_TEST(ret == 0);
> +       /* Ensure synthetic events remain untouched after "trace-cmd reset -k
> synth". */
> +       ret = run_trace("reset", "-k", "synth", NULL);
> +       CU_TEST(ret == 0);
> +       ret = grep_match(SYNTH_EVENT, "stat", NULL);
> +       CU_TEST(ret == 0);
> 
>         tracefs_instance_reset(NULL);
>   }
> @@ -598,6 +603,159 @@ static void test_trace_library_read_back(void)
>         tracecmd_close(handle);
>   }
> 
> +static void test_trace_reset_kprobe(void)
> +{
> +       int ret;
> +       FILE *fp;
> +
> +       /* Create a simple kprobe for do_sys_open */
> +       fp = fopen("/sys/kernel/tracing/kprobe_events", "w");
> +       CU_TEST(fp != NULL);
> +       fprintf(fp, "p do_sys_open");
> +       fclose(fp);
> +
> +       /* Ensure the kprobe is listed in "trace-cmd stat" output. */
> +       ret = grep_match("p:kprobes/p_do_sys_open_0 do_sys_open", "stat", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Issue "trace-cmd reset", but keep kprobes. */
> +       ret = run_trace("reset", "-k", "kprobe", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Verify the kprobe's existence after reset. */
> +       ret = grep_match("p:kprobes/p_do_sys_open_0 do_sys_open", "stat", NULL);
> +       CU_TEST(ret == 0);
> +}
> +
> +static void test_trace_reset_kretprobe(void)
> +{
> +       int ret;
> +       FILE *fp;
> +
> +       /* Create a simple kretprobe for do_sys_open */
> +       fp = fopen("/sys/kernel/tracing/kprobe_events", "w");
> +       CU_TEST(fp != NULL);
> +       fprintf(fp, "r do_sys_open");
> +       fclose(fp);
> +
> +       /* Ensure the kretprobe is listed in "trace-cmd stat" output. */
> +       ret = grep_match("r128:kprobes/r_do_sys_open_0 do_sys_open", "stat",
> NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Issue "trace-cmd reset", but keep kretprobes. */
> +       ret = run_trace("reset", "-k", "kretprobe", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Verify the kretprobe's existence after reset. */
> +       ret = grep_match("r128:kprobes/r_do_sys_open_0 do_sys_open", "stat",
> NULL);
> +       CU_TEST(ret == 0);
> +}
> +
> +static void test_trace_reset_uprobe(void)
> +{
> +       int ret;
> +       FILE *fp;
> +
> +       /* Create a simple uprobe for do_sys_open */
> +       fp = fopen("/sys/kernel/tracing/uprobe_events", "w");
> +       CU_TEST(fp != NULL);
> +       fprintf(fp, "p /bin/bash:0x4245c0");
> +       fclose(fp);
> +
> +       /* Ensure the uprobe is listed in "trace-cmd stat" output. */
> +       ret = grep_match("p:uprobes/p_bash_0x4245c0
> /bin/bash:0x00000000004245c0", "stat", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Issue "trace-cmd reset", but keep uprobes. */
> +       ret = run_trace("reset", "-k", "uprobe", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Verify the uprobe's existence after reset. */
> +       ret = grep_match("p:uprobes/p_bash_0x4245c0
> /bin/bash:0x00000000004245c0", "stat", NULL);
> +       CU_TEST(ret == 0); // TODO: FAILS!
> +}
> +
> +static void test_trace_reset_uretprobe(void)
> +{
> +       int ret;
> +       FILE *fp;
> +
> +       /* Create a simple uretprobe for do_sys_open */
> +       fp = fopen("/sys/kernel/tracing/uprobe_events", "w");
> +       CU_TEST(fp != NULL);
> +       fprintf(fp, "r /bin/bash:0x4245c0");
> +       fclose(fp);
> +
> +       /* Ensure the uretprobe is listed in "trace-cmd stat" output. */
> +       ret = grep_match("r:uprobes/p_bash_0x4245c0
> /bin/bash:0x00000000004245c0", "stat", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Issue "trace-cmd reset", but keep uretprobes. */
> +       ret = run_trace("reset", "-k", "uretprobe", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Verify the uretprobe's existence after reset. */
> +       ret = grep_match("r:uprobes/p_bash_0x4245c0
> /bin/bash:0x00000000004245c0", "stat", NULL);
> +       CU_TEST(ret == 0); // TODO: FAILS!
> +}
> +
> +static void test_trace_reset_eprobe(void)
> +{
> +       int ret;
> +       bool matched = false;
> +       size_t l = 0;
> +       ssize_t n;
> +       char *buf = NULL;
> +       struct tracefs_dynevent *deprobe;
> +       FILE *fp;
> +
> +       deprobe = tracefs_eprobe_alloc(NULL, "sopen_in", "syscalls",
> "sys_enter_openat", NULL);
> +       CU_TEST(deprobe != NULL);
> +
> +       ret = tracefs_dynevent_create(deprobe);
> +       CU_TEST(ret == 0);
> +
> +       /* Issue "trace-cmd reset", but keep eprobes. */
> +       ret = run_trace("reset", "-k", "eprobe", NULL);
> +       CU_TEST(ret == 0);
> +
> +       /* Verify the eprobe's existence after reset. */
> +       fp = fopen("/sys/kernel/tracing/dynamic_events", "r");
> +       CU_TEST(fp != NULL);
> +
> +       while ((n = getline(&buf, &l, fp)) != -1) {
> +               if (!strcmp(buf, "e:eprobes/sopen_in syscalls.sys_enter_openat\n")) {
> +                       matched = true;
> +                       break;
> +               }
> +       }
> +       free(buf);
> +
> +       fclose(fp);
> +
> +       CU_TEST(matched == true);
> +
> +       ret = tracefs_dynevent_destroy(deprobe, false);
> +       CU_TEST(ret == 0);
> +
> +       tracefs_dynevent_free(deprobe);
> +}
> +
> +static void test_trace_reset(void)
> +{
> +       int ret;
> +
> +       test_trace_reset_kprobe();
> +       test_trace_reset_kretprobe();
> +       test_trace_reset_uprobe();
> +       test_trace_reset_uretprobe();
> +       test_trace_reset_eprobe();
> +
> +       /* Destroy all dynamic events. */
> +       ret = run_trace("reset", NULL);
> +       CU_TEST(ret == 0);
> +}
> +
>   static int test_suite_destroy(void)
>   {
>         unlink(TRACECMD_FILE);
> @@ -639,7 +797,7 @@ void test_tracecmd_lib(void)
> 
>         suite = CU_add_suite(TRACECMD_SUITE, test_suite_init,
> test_suite_destroy);
>         if (suite == NULL) {
> -               fprintf(stderr, "Suite \"%s\" cannot be ceated\n", TRACECMD_SUITE);
> +               fprintf(stderr, "Suite \"%s\" cannot be created\n", TRACECMD_SUITE);
>                 return;
>         }
>         CU_add_test(suite, "Simple record and report",
> @@ -656,4 +814,5 @@ void test_tracecmd_lib(void)
>                     test_trace_library_read_back);
>         CU_add_test(suite, "Test max length",
>                     test_trace_record_max);
> +       CU_add_test(suite, "Simple reset", test_trace_reset);
>   }
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v2 0/4] trace-cmd reset: Add option to preserve specific events

[Steven Rostedt]

On Tue, 15 Oct 2024 14:30:58 +0100
Metin Kaya <metin.kaya@arm.com> wrote:

> On 14/10/2024 10:59 pm, Steven Rostedt wrote:
> > On Mon, 14 Oct 2024 13:31:32 +0100
> > Metin Kaya <metin.kaya@arm.com> wrote:
> >  
> >> Changes since v1:
> >> - Update man page for -k command line parameter of "trace-cmd reset".
> >> - Add tab completion support for "trace-cmd reset".  
> >
> > Thanks, I'll go ahead and apply this. But can you do me a favor, and add a
> > test case to utest/tracecmd-utest.c that tests this. It should create a
> > each of the types, start tracing, do the reset, and make sure that the -k
> > keeps what is expected. Doesn't need to be too fancy.  
> 
> I wrote the patch below, but noticed I'm unable to preserve uprobes
> during reset. I verified trace-cmd tries to prevent
> TRACEFS_DYNEVENT_UPROBE from being destroyed, but I cannot see them
> after running "trace-cmd reset -k uprobe" command.
> The same problem exists for uretprobe, too.
> I confirmed utest uses correct trace-cmd executable.
> 
> Any ideas?

Ug, that's because uprobes are defined in dynamic_events with a "p:", just
like kprobes. :-p  And the tracefs dynamic event parser flags uprobes as
kprobes. Thus, if we are deleting kprobes, we will delete uprobes too.

I need to fix that. We can't change the kernel as it's ABI unfortunately,
but there's other ways libtracefs can differentiate the two.

-- Steve