| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aCuiP0qB1WbMnPcb@x1>
Date: Mon, 19 May 2025 18:27:27 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>,
Thomas Richter <tmricht@...ux.ibm.com>
Cc: linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
linux-perf-users@...r.kernel.org, namhyung@...nel.org,
agordeev@...ux.ibm.com, gor@...ux.ibm.com, sumanthk@...ux.ibm.com,
hca@...ux.ibm.com, Alexander Egorenkov <egorenar@...ux.ibm.com>
Subject: Re: [PATCH v2] perf ftrace: Use process/session specific trace
settings
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@...ber:~# 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@...ber:~#
- 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@...ux.ibm.com>
> > Suggested-by: Arnaldo Carvalho de Melo <acme@...nel.org>
> > Reported-by: Alexander Egorenkov <egorenar@...ux.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
Powered by blists - more mailing lists