linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf ftrace: Use process/session specific trace settings
@ 2025-05-19 14:52 Thomas Richter
  2025-05-19 21:22 ` Arnaldo Carvalho de Melo
  2025-05-20  0:00 ` Namhyung Kim
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Richter @ 2025-05-19 14:52 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linux-perf-users, acme, namhyung
  Cc: agordeev, gor, sumanthk, hca, Thomas Richter, Alexander Egorenkov

V1 --> V2: Add Suggestion from Arnaldo Cavalho de Melo confirmed by
           Steven Rostedt. Use rmdir(../tracing/instances/dir) to stop
	   process/session specific tracing and delete all 
	   process/session specific setings.

Executing perf ftrace commands ftrace, profile and latency
leave tracing disabled as can seen in this output:

 # echo 1 > /sys/kernel/debug/tracing/tracing_on
 # cat /sys/kernel/debug/tracing/tracing_on
 1
 # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
 # cat /sys/kernel/debug/tracing/tracing_on
 0
 #

The tracing_on file is not restored to its value before the command.
Fix this behavior and restore the trace setting to what
is was before the invocation of the command.
On Fedora 41 and 42 tracing is turned on by default.

This patch use the .../tracing/instances/XXX subdirectory feature.
Each perf ftrace invocation creates its own session/process
specific subdirectory and does not change the global state
in the .../tracing directory itself.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 tools/perf/builtin-ftrace.c | 105 +++++++++++++++++++++++++++++++-----
 1 file changed, 91 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 7caa18d5ffc3..3faf96e7185e 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -38,6 +38,8 @@
 #include "util/units.h"
 #include "util/parse-sublevel-options.h"
 
+#include <sys/stat.h>
+
 #define DEFAULT_TRACER  "function_graph"
 
 static volatile sig_atomic_t workload_exec_errno;
@@ -45,6 +47,8 @@ static volatile sig_atomic_t done;
 
 static struct stats latency_stats;  /* for tracepoints */
 
+static char tracing_instance[PATH_MAX];	/* Trace instance directory */
+
 static void sig_handler(int sig __maybe_unused)
 {
 	done = true;
@@ -100,6 +104,34 @@ static bool is_ftrace_supported(void)
 	return supported;
 }
 
+/*
+ * Wrapper to test if a file in directory .../tracing/instances/XXX
+ * exists. If so return the .../tracing/instances/XXX file for use.
+ * Otherwise the file exists only in directory .../tracing and
+ * is applicable to all instances, for example file available_filter_functions.
+ * Return that file name in this case.
+ *
+ * This functions works similar to get_tracing_file() and expects its caller
+ * to free the returned file name.
+ *
+ * The global variable tracing_instance is set in init_tracing_instance()
+ * called a the  beginning to a process specific tracing subdirectory.
+ */
+static char *get_tracing_instance_file(const char *name)
+{
+	char *file;
+
+	if (asprintf(&file, "%s/%s", tracing_instance, name) < 0)
+		return NULL;
+
+	if (!access(file, F_OK))
+		return file;
+
+	put_tracing_file(file);
+	file = get_tracing_file(name);
+	return file;
+}
+
 static int __write_tracing_file(const char *name, const char *val, bool append)
 {
 	char *file;
@@ -109,7 +141,7 @@ static int __write_tracing_file(const char *name, const char *val, bool append)
 	char errbuf[512];
 	char *val_copy;
 
-	file = get_tracing_file(name);
+	file = get_tracing_instance_file(name);
 	if (!file) {
 		pr_debug("cannot get tracing file: %s\n", name);
 		return -1;
@@ -167,7 +199,7 @@ static int read_tracing_file_to_stdout(const char *name)
 	int fd;
 	int ret = -1;
 
-	file = get_tracing_file(name);
+	file = get_tracing_instance_file(name);
 	if (!file) {
 		pr_debug("cannot get tracing file: %s\n", name);
 		return -1;
@@ -209,7 +241,7 @@ static int read_tracing_file_by_line(const char *name,
 	char *file;
 	FILE *fp;
 
-	file = get_tracing_file(name);
+	file = get_tracing_instance_file(name);
 	if (!file) {
 		pr_debug("cannot get tracing file: %s\n", name);
 		return -1;
@@ -299,6 +331,36 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	return 0;
 }
 
+/* Remove .../tracing/instances/XXX subdirectory created with
+ * init_tracing_instance().
+ */
+static void exit_tracing_instance(void)
+{
+	rmdir(tracing_instance);
+}
+
+/* Create subdirectory within .../tracing/instances/XXX to have session
+ * or process specific setup. To delete this setup, simply remove the
+ * subdirectory.
+ */
+static int init_tracing_instance(void)
+{
+	char dirname[] = "instances/perf-ftrace-XXXXXX";
+	char *path;
+
+	path = get_tracing_file(dirname);
+	if (!path)
+		return -1;
+	strcpy(tracing_instance, path);
+	put_tracing_file(path);
+	path = mkdtemp(tracing_instance);
+	if (!path) {
+		pr_err("failed to create tracing/instances directory\n");
+		return -1;
+	}
+	return 0;
+}
+
 static int set_tracing_pid(struct perf_ftrace *ftrace)
 {
 	int i;
@@ -629,14 +691,19 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
 
 	select_tracer(ftrace);
 
+	if (init_tracing_instance() < 0) {
+		pr_err("failed to create tracing/instances\n");
+		goto out;
+	}
+
 	if (reset_tracing_files(ftrace) < 0) {
 		pr_err("failed to reset ftrace\n");
-		goto out;
+		goto out_reset;
 	}
 
 	/* reset ftrace buffer */
 	if (write_tracing_file("trace", "0") < 0)
-		goto out;
+		goto out_reset;
 
 	if (set_tracing_options(ftrace) < 0)
 		goto out_reset;
@@ -648,7 +715,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
 
 	setup_pager();
 
-	trace_file = get_tracing_file("trace_pipe");
+	trace_file = get_tracing_instance_file("trace_pipe");
 	if (!trace_file) {
 		pr_err("failed to open trace_pipe\n");
 		goto out_reset;
@@ -723,7 +790,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
 out_close_fd:
 	close(trace_fd);
 out_reset:
-	reset_tracing_files(ftrace);
+	exit_tracing_instance();
 out:
 	return (done && !workload_exec_errno) ? 0 : -1;
 }
@@ -924,6 +991,11 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
 	if (ftrace->target.use_bpf)
 		return perf_ftrace__latency_prepare_bpf(ftrace);
 
+	if (init_tracing_instance() < 0) {
+		pr_err("failed to create tracing/instances\n");
+		return -1;
+	}
+
 	if (reset_tracing_files(ftrace) < 0) {
 		pr_err("failed to reset ftrace\n");
 		return -1;
@@ -942,7 +1014,7 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
 		return -1;
 	}
 
-	trace_file = get_tracing_file("trace_pipe");
+	trace_file = get_tracing_instance_file("trace_pipe");
 	if (!trace_file) {
 		pr_err("failed to open trace_pipe\n");
 		return -1;
@@ -993,7 +1065,7 @@ static int cleanup_func_latency(struct perf_ftrace *ftrace)
 	if (ftrace->target.use_bpf)
 		return perf_ftrace__latency_cleanup_bpf(ftrace);
 
-	reset_tracing_files(ftrace);
+	exit_tracing_instance();
 	return 0;
 }
 
@@ -1304,17 +1376,22 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
 		goto out;
 	}
 
+	if (init_tracing_instance() < 0) {
+		pr_err("failed to create tracing/instances\n");
+		goto out;
+	}
+
 	if (reset_tracing_files(ftrace) < 0) {
 		pr_err("failed to reset ftrace\n");
-		goto out;
+		goto out_reset;
 	}
 
 	/* reset ftrace buffer */
 	if (write_tracing_file("trace", "0") < 0)
-		goto out;
+		goto out_reset;
 
 	if (set_tracing_options(ftrace) < 0)
-		return -1;
+		goto out_reset;
 
 	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
 		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
@@ -1323,7 +1400,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
 
 	setup_pager();
 
-	trace_file = get_tracing_file("trace_pipe");
+	trace_file = get_tracing_instance_file("trace_pipe");
 	if (!trace_file) {
 		pr_err("failed to open trace_pipe\n");
 		goto out_reset;
@@ -1385,7 +1462,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
 out_close_fd:
 	close(trace_fd);
 out_reset:
-	reset_tracing_files(ftrace);
+	exit_tracing_instance();
 out:
 	return (done && !workload_exec_errno) ? 0 : -1;
 }
-- 
2.49.0


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

* Re: [PATCH v2] perf ftrace: Use process/session specific trace settings
  2025-05-19 14:52 [PATCH v2] perf ftrace: Use process/session specific trace settings Thomas Richter
@ 2025-05-19 21:22 ` Arnaldo Carvalho de Melo
  2025-05-19 21:27   ` Arnaldo Carvalho de Melo
  2025-05-20  0:00 ` Namhyung Kim
  1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-19 21:22 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-s390, linux-perf-users, namhyung, agordeev,
	gor, sumanthk, hca, Alexander Egorenkov

On Mon, May 19, 2025 at 04:52:35PM +0200, Thomas Richter wrote:
> V1 --> V2: Add Suggestion from Arnaldo Cavalho de Melo confirmed by
>            Steven Rostedt. Use rmdir(../tracing/instances/dir) to stop
> 	   process/session specific tracing and delete all 
> 	   process/session specific setings.
> 
> Executing perf ftrace commands ftrace, profile and latency
> leave tracing disabled as can seen in this output:
> 
>  # echo 1 > /sys/kernel/debug/tracing/tracing_on
>  # cat /sys/kernel/debug/tracing/tracing_on
>  1
>  # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
>  # cat /sys/kernel/debug/tracing/tracing_on
>  0
>  #
> 
> The tracing_on file is not restored to its value before the command.
> Fix this behavior and restore the trace setting to what
> is was before the invocation of the command.

The above paragraph doesn't apply after you moved to instances, right?

I changed the commit log message to:

----------------------
perf ftrace: Use process/session specific trace settings

Executing 'perf ftrace' commands 'ftrace', 'profile' and 'latency' leave
tracing disabled as can seen in this output:

 # echo 1 > /sys/kernel/debug/tracing/tracing_on
 # cat /sys/kernel/debug/tracing/tracing_on
 1
 # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
 # cat /sys/kernel/debug/tracing/tracing_on
 0
 #

The 'tracing_on' file is not restored to its value before the command.

To fix that this patch uses the .../tracing/instances/XXX subdirectory
feature.

Each 'perf ftrace' invocation creates its own session/process
specific subdirectory and does not change the global state
in the .../tracing directory itself.

Use rmdir(../tracing/instances/dir) to stop process/session specific
tracing and delete all process/session specific setings.

-----------

followed by the tags you provided, ok?

- Arnaldo

> On Fedora 41 and 42 tracing is turned on by default.
> 
> This patch use the .../tracing/instances/XXX subdirectory feature.
> Each perf ftrace invocation creates its own session/process
> specific subdirectory and does not change the global state
> in the .../tracing directory itself.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> ---
>  tools/perf/builtin-ftrace.c | 105 +++++++++++++++++++++++++++++++-----
>  1 file changed, 91 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 7caa18d5ffc3..3faf96e7185e 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -38,6 +38,8 @@
>  #include "util/units.h"
>  #include "util/parse-sublevel-options.h"
>  
> +#include <sys/stat.h>
> +
>  #define DEFAULT_TRACER  "function_graph"
>  
>  static volatile sig_atomic_t workload_exec_errno;
> @@ -45,6 +47,8 @@ static volatile sig_atomic_t done;
>  
>  static struct stats latency_stats;  /* for tracepoints */
>  
> +static char tracing_instance[PATH_MAX];	/* Trace instance directory */
> +
>  static void sig_handler(int sig __maybe_unused)
>  {
>  	done = true;
> @@ -100,6 +104,34 @@ static bool is_ftrace_supported(void)
>  	return supported;
>  }
>  
> +/*
> + * Wrapper to test if a file in directory .../tracing/instances/XXX
> + * exists. If so return the .../tracing/instances/XXX file for use.
> + * Otherwise the file exists only in directory .../tracing and
> + * is applicable to all instances, for example file available_filter_functions.
> + * Return that file name in this case.
> + *
> + * This functions works similar to get_tracing_file() and expects its caller
> + * to free the returned file name.
> + *
> + * The global variable tracing_instance is set in init_tracing_instance()
> + * called a the  beginning to a process specific tracing subdirectory.
> + */
> +static char *get_tracing_instance_file(const char *name)
> +{
> +	char *file;
> +
> +	if (asprintf(&file, "%s/%s", tracing_instance, name) < 0)
> +		return NULL;
> +
> +	if (!access(file, F_OK))
> +		return file;
> +
> +	put_tracing_file(file);
> +	file = get_tracing_file(name);
> +	return file;
> +}
> +
>  static int __write_tracing_file(const char *name, const char *val, bool append)
>  {
>  	char *file;
> @@ -109,7 +141,7 @@ static int __write_tracing_file(const char *name, const char *val, bool append)
>  	char errbuf[512];
>  	char *val_copy;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -167,7 +199,7 @@ static int read_tracing_file_to_stdout(const char *name)
>  	int fd;
>  	int ret = -1;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -209,7 +241,7 @@ static int read_tracing_file_by_line(const char *name,
>  	char *file;
>  	FILE *fp;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -299,6 +331,36 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>  	return 0;
>  }
>  
> +/* Remove .../tracing/instances/XXX subdirectory created with
> + * init_tracing_instance().
> + */
> +static void exit_tracing_instance(void)
> +{
> +	rmdir(tracing_instance);
> +}
> +
> +/* Create subdirectory within .../tracing/instances/XXX to have session
> + * or process specific setup. To delete this setup, simply remove the
> + * subdirectory.
> + */
> +static int init_tracing_instance(void)
> +{
> +	char dirname[] = "instances/perf-ftrace-XXXXXX";
> +	char *path;
> +
> +	path = get_tracing_file(dirname);
> +	if (!path)
> +		return -1;
> +	strcpy(tracing_instance, path);
> +	put_tracing_file(path);
> +	path = mkdtemp(tracing_instance);
> +	if (!path) {
> +		pr_err("failed to create tracing/instances directory\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  static int set_tracing_pid(struct perf_ftrace *ftrace)
>  {
>  	int i;
> @@ -629,14 +691,19 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  
>  	select_tracer(ftrace);
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");
> +		goto out;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
> -		goto out;
> +		goto out_reset;
>  	}
>  
>  	/* reset ftrace buffer */
>  	if (write_tracing_file("trace", "0") < 0)
> -		goto out;
> +		goto out_reset;
>  
>  	if (set_tracing_options(ftrace) < 0)
>  		goto out_reset;
> @@ -648,7 +715,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  
>  	setup_pager();
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		goto out_reset;
> @@ -723,7 +790,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  out_close_fd:
>  	close(trace_fd);
>  out_reset:
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  out:
>  	return (done && !workload_exec_errno) ? 0 : -1;
>  }
> @@ -924,6 +991,11 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
>  	if (ftrace->target.use_bpf)
>  		return perf_ftrace__latency_prepare_bpf(ftrace);
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");
> +		return -1;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
>  		return -1;
> @@ -942,7 +1014,7 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
>  		return -1;
>  	}
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		return -1;
> @@ -993,7 +1065,7 @@ static int cleanup_func_latency(struct perf_ftrace *ftrace)
>  	if (ftrace->target.use_bpf)
>  		return perf_ftrace__latency_cleanup_bpf(ftrace);
>  
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  	return 0;
>  }
>  
> @@ -1304,17 +1376,22 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  		goto out;
>  	}
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");
> +		goto out;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
> -		goto out;
> +		goto out_reset;
>  	}
>  
>  	/* reset ftrace buffer */
>  	if (write_tracing_file("trace", "0") < 0)
> -		goto out;
> +		goto out_reset;
>  
>  	if (set_tracing_options(ftrace) < 0)
> -		return -1;
> +		goto out_reset;
>  
>  	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
>  		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
> @@ -1323,7 +1400,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  
>  	setup_pager();
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		goto out_reset;
> @@ -1385,7 +1462,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  out_close_fd:
>  	close(trace_fd);
>  out_reset:
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  out:
>  	return (done && !workload_exec_errno) ? 0 : -1;
>  }
> -- 
> 2.49.0

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

* Re: [PATCH v2] perf ftrace: Use process/session specific trace settings
  2025-05-19 21:22 ` Arnaldo Carvalho de Melo
@ 2025-05-19 21:27   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-05-19 21:27 UTC (permalink / raw)
  To: Namhyung Kim, Thomas Richter
  Cc: linux-kernel, linux-s390, linux-perf-users, namhyung, agordeev,
	gor, sumanthk, hca, Alexander Egorenkov

On Mon, May 19, 2025 at 06:22:30PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, May 19, 2025 at 04:52:35PM +0200, Thomas Richter wrote:
> > V1 --> V2: Add Suggestion from Arnaldo Cavalho de Melo confirmed by
> >            Steven Rostedt. Use rmdir(../tracing/instances/dir) to stop
> > 	   process/session specific tracing and delete all 
> > 	   process/session specific setings.

Namhyung,

	I tested this and think it is working as advertised now and
followed the suggestions provided during review, are you ok with it?

Now it does:

root@number:~# perf trace -e open*,write,*dir perf ftrace latency -T schedule sleep 1
<SNIP>
   9.168 ( 3.540 ms): perf/76257 mkdir(pathname: "/sys/kernel/tracing/instances/perf-ftrace-nCYgxe", mode: IRWXU) = 0
    12.717 ( 0.007 ms): perf/76257 openat(dfd: CWD, filename: "/sys/kernel/tracing/instances/perf-ftrace-nCYgxe/tracing_on", flags: TRUNC|WRONLY) = 3
    12.725 ( 0.019 ms): perf/76257 write(fd: 3, buf: 0\10, count: 2)                                     = 2
    12.747 ( 0.002 ms): perf/76257 openat(dfd: CWD, filename: "/sys/kernel/tracing/instances/perf-ftrace-nCYgxe/current_tracer", flags: TRUNC|WRONLY) = 3
    12.749 ( 0.002 ms): perf/76257 write(fd: 3, buf: nop\10, count: 4)                                   = 4
    12.752 ( 0.003 ms): perf/76257 openat(dfd: CWD, filename: "/sys/kernel/tracing/instances/perf-ftrace-nCYgxe/set_ftrace_pid", flags: TRUNC|WRONLY) = 3
    12.756 ( 0.019 ms): perf/76257 write(fd: 3, buf:  \10, count: 2)                                     = 2
    12.784 ( 0.005 ms): perf/76257 openat(dfd: CWD, filename: "/sys/devices/system/cpu/online")          = 3
<SNIP more ftrace setup using an instance>
  1943.702 ( 0.002 ms): perf/76257 write(fd: 1, buf: \10# statistics  (in usec)\10, count: 25)           = 25
  1943.707 ( 0.002 ms): perf/76257 write(fd: 1, buf:   total time:              10000, count: 35)        = 35
  1943.710 ( 0.003 ms): perf/76257 write(fd: 1, buf:     avg time:              10000, count: 35)        = 35
  1943.714 ( 0.003 ms): perf/76257 write(fd: 1, buf:     max time:              10000, count: 35)        = 35
  1943.718 ( 0.002 ms): perf/76257 write(fd: 1, buf:     min time:              10000, count: 35)        = 35
  1943.721 ( 0.001 ms): perf/76257 write(fd: 1, buf:        count:                   , count: 35)        = 35
  1943.729 (48.859 ms): perf/76257 rmdir(pathname: "/sys/kernel/tracing/instances/perf-ftrace-nCYgxe")   = 0
root@number:~# 

- Arnaldo

> > Executing perf ftrace commands ftrace, profile and latency
> > leave tracing disabled as can seen in this output:
> > 
> >  # echo 1 > /sys/kernel/debug/tracing/tracing_on
> >  # cat /sys/kernel/debug/tracing/tracing_on
> >  1
> >  # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
> >  # cat /sys/kernel/debug/tracing/tracing_on
> >  0
> >  #
> > 
> > The tracing_on file is not restored to its value before the command.
> > Fix this behavior and restore the trace setting to what
> > is was before the invocation of the command.
> 
> The above paragraph doesn't apply after you moved to instances, right?
> 
> I changed the commit log message to:
> 
> ----------------------
> perf ftrace: Use process/session specific trace settings
> 
> Executing 'perf ftrace' commands 'ftrace', 'profile' and 'latency' leave
> tracing disabled as can seen in this output:
> 
>  # echo 1 > /sys/kernel/debug/tracing/tracing_on
>  # cat /sys/kernel/debug/tracing/tracing_on
>  1
>  # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
>  # cat /sys/kernel/debug/tracing/tracing_on
>  0
>  #
> 
> The 'tracing_on' file is not restored to its value before the command.
> 
> To fix that this patch uses the .../tracing/instances/XXX subdirectory
> feature.
> 
> Each 'perf ftrace' invocation creates its own session/process
> specific subdirectory and does not change the global state
> in the .../tracing directory itself.
> 
> Use rmdir(../tracing/instances/dir) to stop process/session specific
> tracing and delete all process/session specific setings.
> 
> -----------
> 
> followed by the tags you provided, ok?
> 
> - Arnaldo
> 
> > On Fedora 41 and 42 tracing is turned on by default.
> > 
> > This patch use the .../tracing/instances/XXX subdirectory feature.
> > Each perf ftrace invocation creates its own session/process
> > specific subdirectory and does not change the global state
> > in the .../tracing directory itself.
> > 
> > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> > Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> > ---
> >  tools/perf/builtin-ftrace.c | 105 +++++++++++++++++++++++++++++++-----
> >  1 file changed, 91 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 7caa18d5ffc3..3faf96e7185e 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -38,6 +38,8 @@
> >  #include "util/units.h"
> >  #include "util/parse-sublevel-options.h"
> >  
> > +#include <sys/stat.h>
> > +
> >  #define DEFAULT_TRACER  "function_graph"
> >  
> >  static volatile sig_atomic_t workload_exec_errno;
> > @@ -45,6 +47,8 @@ static volatile sig_atomic_t done;
> >  
> >  static struct stats latency_stats;  /* for tracepoints */
> >  
> > +static char tracing_instance[PATH_MAX];	/* Trace instance directory */
> > +
> >  static void sig_handler(int sig __maybe_unused)
> >  {
> >  	done = true;
> > @@ -100,6 +104,34 @@ static bool is_ftrace_supported(void)
> >  	return supported;
> >  }
> >  
> > +/*
> > + * Wrapper to test if a file in directory .../tracing/instances/XXX
> > + * exists. If so return the .../tracing/instances/XXX file for use.
> > + * Otherwise the file exists only in directory .../tracing and
> > + * is applicable to all instances, for example file available_filter_functions.
> > + * Return that file name in this case.
> > + *
> > + * This functions works similar to get_tracing_file() and expects its caller
> > + * to free the returned file name.
> > + *
> > + * The global variable tracing_instance is set in init_tracing_instance()
> > + * called a the  beginning to a process specific tracing subdirectory.
> > + */
> > +static char *get_tracing_instance_file(const char *name)
> > +{
> > +	char *file;
> > +
> > +	if (asprintf(&file, "%s/%s", tracing_instance, name) < 0)
> > +		return NULL;
> > +
> > +	if (!access(file, F_OK))
> > +		return file;
> > +
> > +	put_tracing_file(file);
> > +	file = get_tracing_file(name);
> > +	return file;
> > +}
> > +
> >  static int __write_tracing_file(const char *name, const char *val, bool append)
> >  {
> >  	char *file;
> > @@ -109,7 +141,7 @@ static int __write_tracing_file(const char *name, const char *val, bool append)
> >  	char errbuf[512];
> >  	char *val_copy;
> >  
> > -	file = get_tracing_file(name);
> > +	file = get_tracing_instance_file(name);
> >  	if (!file) {
> >  		pr_debug("cannot get tracing file: %s\n", name);
> >  		return -1;
> > @@ -167,7 +199,7 @@ static int read_tracing_file_to_stdout(const char *name)
> >  	int fd;
> >  	int ret = -1;
> >  
> > -	file = get_tracing_file(name);
> > +	file = get_tracing_instance_file(name);
> >  	if (!file) {
> >  		pr_debug("cannot get tracing file: %s\n", name);
> >  		return -1;
> > @@ -209,7 +241,7 @@ static int read_tracing_file_by_line(const char *name,
> >  	char *file;
> >  	FILE *fp;
> >  
> > -	file = get_tracing_file(name);
> > +	file = get_tracing_instance_file(name);
> >  	if (!file) {
> >  		pr_debug("cannot get tracing file: %s\n", name);
> >  		return -1;
> > @@ -299,6 +331,36 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
> >  	return 0;
> >  }
> >  
> > +/* Remove .../tracing/instances/XXX subdirectory created with
> > + * init_tracing_instance().
> > + */
> > +static void exit_tracing_instance(void)
> > +{
> > +	rmdir(tracing_instance);
> > +}
> > +
> > +/* Create subdirectory within .../tracing/instances/XXX to have session
> > + * or process specific setup. To delete this setup, simply remove the
> > + * subdirectory.
> > + */
> > +static int init_tracing_instance(void)
> > +{
> > +	char dirname[] = "instances/perf-ftrace-XXXXXX";
> > +	char *path;
> > +
> > +	path = get_tracing_file(dirname);
> > +	if (!path)
> > +		return -1;
> > +	strcpy(tracing_instance, path);
> > +	put_tracing_file(path);
> > +	path = mkdtemp(tracing_instance);
> > +	if (!path) {
> > +		pr_err("failed to create tracing/instances directory\n");
> > +		return -1;
> > +	}
> > +	return 0;
> > +}
> > +
> >  static int set_tracing_pid(struct perf_ftrace *ftrace)
> >  {
> >  	int i;
> > @@ -629,14 +691,19 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
> >  
> >  	select_tracer(ftrace);
> >  
> > +	if (init_tracing_instance() < 0) {
> > +		pr_err("failed to create tracing/instances\n");
> > +		goto out;
> > +	}
> > +
> >  	if (reset_tracing_files(ftrace) < 0) {
> >  		pr_err("failed to reset ftrace\n");
> > -		goto out;
> > +		goto out_reset;
> >  	}
> >  
> >  	/* reset ftrace buffer */
> >  	if (write_tracing_file("trace", "0") < 0)
> > -		goto out;
> > +		goto out_reset;
> >  
> >  	if (set_tracing_options(ftrace) < 0)
> >  		goto out_reset;
> > @@ -648,7 +715,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
> >  
> >  	setup_pager();
> >  
> > -	trace_file = get_tracing_file("trace_pipe");
> > +	trace_file = get_tracing_instance_file("trace_pipe");
> >  	if (!trace_file) {
> >  		pr_err("failed to open trace_pipe\n");
> >  		goto out_reset;
> > @@ -723,7 +790,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
> >  out_close_fd:
> >  	close(trace_fd);
> >  out_reset:
> > -	reset_tracing_files(ftrace);
> > +	exit_tracing_instance();
> >  out:
> >  	return (done && !workload_exec_errno) ? 0 : -1;
> >  }
> > @@ -924,6 +991,11 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
> >  	if (ftrace->target.use_bpf)
> >  		return perf_ftrace__latency_prepare_bpf(ftrace);
> >  
> > +	if (init_tracing_instance() < 0) {
> > +		pr_err("failed to create tracing/instances\n");
> > +		return -1;
> > +	}
> > +
> >  	if (reset_tracing_files(ftrace) < 0) {
> >  		pr_err("failed to reset ftrace\n");
> >  		return -1;
> > @@ -942,7 +1014,7 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
> >  		return -1;
> >  	}
> >  
> > -	trace_file = get_tracing_file("trace_pipe");
> > +	trace_file = get_tracing_instance_file("trace_pipe");
> >  	if (!trace_file) {
> >  		pr_err("failed to open trace_pipe\n");
> >  		return -1;
> > @@ -993,7 +1065,7 @@ static int cleanup_func_latency(struct perf_ftrace *ftrace)
> >  	if (ftrace->target.use_bpf)
> >  		return perf_ftrace__latency_cleanup_bpf(ftrace);
> >  
> > -	reset_tracing_files(ftrace);
> > +	exit_tracing_instance();
> >  	return 0;
> >  }
> >  
> > @@ -1304,17 +1376,22 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
> >  		goto out;
> >  	}
> >  
> > +	if (init_tracing_instance() < 0) {
> > +		pr_err("failed to create tracing/instances\n");
> > +		goto out;
> > +	}
> > +
> >  	if (reset_tracing_files(ftrace) < 0) {
> >  		pr_err("failed to reset ftrace\n");
> > -		goto out;
> > +		goto out_reset;
> >  	}
> >  
> >  	/* reset ftrace buffer */
> >  	if (write_tracing_file("trace", "0") < 0)
> > -		goto out;
> > +		goto out_reset;
> >  
> >  	if (set_tracing_options(ftrace) < 0)
> > -		return -1;
> > +		goto out_reset;
> >  
> >  	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
> >  		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
> > @@ -1323,7 +1400,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
> >  
> >  	setup_pager();
> >  
> > -	trace_file = get_tracing_file("trace_pipe");
> > +	trace_file = get_tracing_instance_file("trace_pipe");
> >  	if (!trace_file) {
> >  		pr_err("failed to open trace_pipe\n");
> >  		goto out_reset;
> > @@ -1385,7 +1462,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
> >  out_close_fd:
> >  	close(trace_fd);
> >  out_reset:
> > -	reset_tracing_files(ftrace);
> > +	exit_tracing_instance();
> >  out:
> >  	return (done && !workload_exec_errno) ? 0 : -1;
> >  }
> > -- 
> > 2.49.0

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

* Re: [PATCH v2] perf ftrace: Use process/session specific trace settings
  2025-05-19 14:52 [PATCH v2] perf ftrace: Use process/session specific trace settings Thomas Richter
  2025-05-19 21:22 ` Arnaldo Carvalho de Melo
@ 2025-05-20  0:00 ` Namhyung Kim
  2025-05-20  9:21   ` Thomas Richter
  1 sibling, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2025-05-20  0:00 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, agordeev, gor,
	sumanthk, hca, Alexander Egorenkov

Hello,

On Mon, May 19, 2025 at 04:52:35PM +0200, Thomas Richter wrote:
> V1 --> V2: Add Suggestion from Arnaldo Cavalho de Melo confirmed by
>            Steven Rostedt. Use rmdir(../tracing/instances/dir) to stop
> 	   process/session specific tracing and delete all 
> 	   process/session specific setings.
> 
> Executing perf ftrace commands ftrace, profile and latency
> leave tracing disabled as can seen in this output:
> 
>  # echo 1 > /sys/kernel/debug/tracing/tracing_on
>  # cat /sys/kernel/debug/tracing/tracing_on
>  1
>  # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
>  # cat /sys/kernel/debug/tracing/tracing_on
>  0
>  #
> 
> The tracing_on file is not restored to its value before the command.
> Fix this behavior and restore the trace setting to what
> is was before the invocation of the command.
> On Fedora 41 and 42 tracing is turned on by default.
> 
> This patch use the .../tracing/instances/XXX subdirectory feature.
> Each perf ftrace invocation creates its own session/process
> specific subdirectory and does not change the global state
> in the .../tracing directory itself.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
> ---
>  tools/perf/builtin-ftrace.c | 105 +++++++++++++++++++++++++++++++-----
>  1 file changed, 91 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 7caa18d5ffc3..3faf96e7185e 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -38,6 +38,8 @@
>  #include "util/units.h"
>  #include "util/parse-sublevel-options.h"
>  
> +#include <sys/stat.h>

The standard header files are placed in the above.  Let's put it there.

> +
>  #define DEFAULT_TRACER  "function_graph"
>  
>  static volatile sig_atomic_t workload_exec_errno;
> @@ -45,6 +47,8 @@ static volatile sig_atomic_t done;
>  
>  static struct stats latency_stats;  /* for tracepoints */
>  
> +static char tracing_instance[PATH_MAX];	/* Trace instance directory */
> +
>  static void sig_handler(int sig __maybe_unused)
>  {
>  	done = true;
> @@ -100,6 +104,34 @@ static bool is_ftrace_supported(void)
>  	return supported;
>  }
>  
> +/*
> + * Wrapper to test if a file in directory .../tracing/instances/XXX
> + * exists. If so return the .../tracing/instances/XXX file for use.
> + * Otherwise the file exists only in directory .../tracing and
> + * is applicable to all instances, for example file available_filter_functions.
> + * Return that file name in this case.

Not sure if it's needed.  Can we call get_tracing_file() directly for
the global files?

> + *
> + * This functions works similar to get_tracing_file() and expects its caller
> + * to free the returned file name.
> + *
> + * The global variable tracing_instance is set in init_tracing_instance()
> + * called a the  beginning to a process specific tracing subdirectory.

s/called a /called at / ?

> + */
> +static char *get_tracing_instance_file(const char *name)
> +{
> +	char *file;
> +
> +	if (asprintf(&file, "%s/%s", tracing_instance, name) < 0)
> +		return NULL;
> +
> +	if (!access(file, F_OK))
> +		return file;
> +
> +	put_tracing_file(file);

This didn't use the get_tracing_file() API.  Just call free().


> +	file = get_tracing_file(name);
> +	return file;
> +}
> +
>  static int __write_tracing_file(const char *name, const char *val, bool append)
>  {
>  	char *file;
> @@ -109,7 +141,7 @@ static int __write_tracing_file(const char *name, const char *val, bool append)
>  	char errbuf[512];
>  	char *val_copy;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -167,7 +199,7 @@ static int read_tracing_file_to_stdout(const char *name)
>  	int fd;
>  	int ret = -1;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -209,7 +241,7 @@ static int read_tracing_file_by_line(const char *name,
>  	char *file;
>  	FILE *fp;
>  
> -	file = get_tracing_file(name);
> +	file = get_tracing_instance_file(name);
>  	if (!file) {
>  		pr_debug("cannot get tracing file: %s\n", name);
>  		return -1;
> @@ -299,6 +331,36 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>  	return 0;
>  }
>  
> +/* Remove .../tracing/instances/XXX subdirectory created with
> + * init_tracing_instance().
> + */
> +static void exit_tracing_instance(void)
> +{
> +	rmdir(tracing_instance);

Can we check the return value and print a message if failed?  I think
it'd succeed mostly but let's prepare for errors.

> +}
> +
> +/* Create subdirectory within .../tracing/instances/XXX to have session
> + * or process specific setup. To delete this setup, simply remove the
> + * subdirectory.
> + */
> +static int init_tracing_instance(void)
> +{
> +	char dirname[] = "instances/perf-ftrace-XXXXXX";
> +	char *path;
> +
> +	path = get_tracing_file(dirname);
> +	if (!path)
> +		return -1;
> +	strcpy(tracing_instance, path);

strncpy() instead?


> +	put_tracing_file(path);
> +	path = mkdtemp(tracing_instance);
> +	if (!path) {
> +		pr_err("failed to create tracing/instances directory\n");
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  static int set_tracing_pid(struct perf_ftrace *ftrace)
>  {
>  	int i;
> @@ -629,14 +691,19 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  
>  	select_tracer(ftrace);
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");

Duplicated error message.


> +		goto out;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
> -		goto out;
> +		goto out_reset;
>  	}
>  
>  	/* reset ftrace buffer */
>  	if (write_tracing_file("trace", "0") < 0)
> -		goto out;
> +		goto out_reset;
>  
>  	if (set_tracing_options(ftrace) < 0)
>  		goto out_reset;
> @@ -648,7 +715,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  
>  	setup_pager();
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		goto out_reset;
> @@ -723,7 +790,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  out_close_fd:
>  	close(trace_fd);
>  out_reset:
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  out:
>  	return (done && !workload_exec_errno) ? 0 : -1;
>  }
> @@ -924,6 +991,11 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
>  	if (ftrace->target.use_bpf)
>  		return perf_ftrace__latency_prepare_bpf(ftrace);
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");

Ditto.


> +		return -1;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
>  		return -1;
> @@ -942,7 +1014,7 @@ static int prepare_func_latency(struct perf_ftrace *ftrace)
>  		return -1;
>  	}
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		return -1;
> @@ -993,7 +1065,7 @@ static int cleanup_func_latency(struct perf_ftrace *ftrace)
>  	if (ftrace->target.use_bpf)
>  		return perf_ftrace__latency_cleanup_bpf(ftrace);
>  
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  	return 0;
>  }
>  
> @@ -1304,17 +1376,22 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  		goto out;
>  	}
>  
> +	if (init_tracing_instance() < 0) {
> +		pr_err("failed to create tracing/instances\n");

Ditto.

Thanks,
Namhyung


> +		goto out;
> +	}
> +
>  	if (reset_tracing_files(ftrace) < 0) {
>  		pr_err("failed to reset ftrace\n");
> -		goto out;
> +		goto out_reset;
>  	}
>  
>  	/* reset ftrace buffer */
>  	if (write_tracing_file("trace", "0") < 0)
> -		goto out;
> +		goto out_reset;
>  
>  	if (set_tracing_options(ftrace) < 0)
> -		return -1;
> +		goto out_reset;
>  
>  	if (write_tracing_file("current_tracer", ftrace->tracer) < 0) {
>  		pr_err("failed to set current_tracer to %s\n", ftrace->tracer);
> @@ -1323,7 +1400,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  
>  	setup_pager();
>  
> -	trace_file = get_tracing_file("trace_pipe");
> +	trace_file = get_tracing_instance_file("trace_pipe");
>  	if (!trace_file) {
>  		pr_err("failed to open trace_pipe\n");
>  		goto out_reset;
> @@ -1385,7 +1462,7 @@ static int __cmd_profile(struct perf_ftrace *ftrace)
>  out_close_fd:
>  	close(trace_fd);
>  out_reset:
> -	reset_tracing_files(ftrace);
> +	exit_tracing_instance();
>  out:
>  	return (done && !workload_exec_errno) ? 0 : -1;
>  }
> -- 
> 2.49.0
> 

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

* Re: [PATCH v2] perf ftrace: Use process/session specific trace settings
  2025-05-20  0:00 ` Namhyung Kim
@ 2025-05-20  9:21   ` Thomas Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Richter @ 2025-05-20  9:21 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, agordeev, gor,
	sumanthk, hca, Alexander Egorenkov

On 5/20/25 02:00, Namhyung Kim wrote:
> Hello,
> 
> On Mon, May 19, 2025 at 04:52:35PM +0200, Thomas Richter wrote:
>> V1 --> V2: Add Suggestion from Arnaldo Cavalho de Melo confirmed by
>>            Steven Rostedt. Use rmdir(../tracing/instances/dir) to stop
>> 	   process/session specific tracing and delete all 
>> 	   process/session specific setings.
>>
>> Executing perf ftrace commands ftrace, profile and latency
>> leave tracing disabled as can seen in this output:
>>
>>  # echo 1 > /sys/kernel/debug/tracing/tracing_on
>>  # cat /sys/kernel/debug/tracing/tracing_on
>>  1
>>  # perf ftrace trace --graph-opts depth=5 sleep 0.1 > /dev/null
>>  # cat /sys/kernel/debug/tracing/tracing_on
>>  0
>>  #
>>
>> The tracing_on file is not restored to its value before the command.
>> Fix this behavior and restore the trace setting to what
>> is was before the invocation of the command.
>> On Fedora 41 and 42 tracing is turned on by default.
>>
>> This patch use the .../tracing/instances/XXX subdirectory feature.
>> Each perf ftrace invocation creates its own session/process
>> specific subdirectory and does not change the global state
>> in the .../tracing directory itself.
>>
>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Reported-by: Alexander Egorenkov <egorenar@linux.ibm.com>
>> ---
>>  tools/perf/builtin-ftrace.c | 105 +++++++++++++++++++++++++++++++-----
>>  1 file changed, 91 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
>> index 7caa18d5ffc3..3faf96e7185e 100644
>> --- a/tools/perf/builtin-ftrace.c
>> +++ b/tools/perf/builtin-ftrace.c
>> @@ -38,6 +38,8 @@
>>  #include "util/units.h"
>>  #include "util/parse-sublevel-options.h"
>>  
>> +#include <sys/stat.h>
> 
> The standard header files are placed in the above.  Let's put it there.
> 
>> +
>>  #define DEFAULT_TRACER  "function_graph"
>>  
>>  static volatile sig_atomic_t workload_exec_errno;
>> @@ -45,6 +47,8 @@ static volatile sig_atomic_t done;
>>  
>>  static struct stats latency_stats;  /* for tracepoints */
>>  
>> +static char tracing_instance[PATH_MAX];	/* Trace instance directory */
>> +
>>  static void sig_handler(int sig __maybe_unused)
>>  {
>>  	done = true;
>> @@ -100,6 +104,34 @@ static bool is_ftrace_supported(void)
>>  	return supported;
>>  }
>>  
>> +/*
>> + * Wrapper to test if a file in directory .../tracing/instances/XXX
>> + * exists. If so return the .../tracing/instances/XXX file for use.
>> + * Otherwise the file exists only in directory .../tracing and
>> + * is applicable to all instances, for example file available_filter_functions.
>> + * Return that file name in this case.
> 
> Not sure if it's needed.  Can we call get_tracing_file() directly for
> the global files?
> 
[SNIP]
>> +static char *get_tracing_instance_file(const char *name)

Not really, some files are not available in the tracing/instances/XXX
directory, for example 

   ret = read_tracing_file_by_line("available_filter_functions",         
                                   list_function_cb, filter);  

That file is not created in the instances subdirectory:
 # pwd
/sys/kernel/debug/tracing
[
# find . -name tracing_on | grep -E '(\.|foo)/t'
./instances/foo/tracing_on
./tracing_on
# find . -name available_filter_functions | grep -E '(\.|foo)/a'
./available_filter_functions
#

The easiest approach is to always test for file existance in the
instances/XXX directory first and if it is missing, fall back
to the tracing directory. Otherwise the file location has to
be known by file name.
The other files which are not created in the instances/XXX subdirectory
are
  max_graph_depth, tracing_thres, set_graph_function and set_graph_notrace.

Hope this helps.

I will add all your other rewiev comment and submit V3.

Thanks a lot
    
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

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

end of thread, other threads:[~2025-05-20  9:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-19 14:52 [PATCH v2] perf ftrace: Use process/session specific trace settings Thomas Richter
2025-05-19 21:22 ` Arnaldo Carvalho de Melo
2025-05-19 21:27   ` Arnaldo Carvalho de Melo
2025-05-20  0:00 ` Namhyung Kim
2025-05-20  9:21   ` Thomas Richter

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).