linux-kernel - Re: [PATCH v2] perf ftrace: Use process/session specific trace settings

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1660a81e-e057-44d3-be99-ec91fbf512e7@linux.ibm.com>
Date: Tue, 20 May 2025 11:21:39 +0200
From: Thomas Richter <tmricht@...ux.ibm.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-perf-users@...r.kernel.org, acme@...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 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@...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>
> 
> 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