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