rtla: fix cgroup and trace options parsing
Subsystems (3)
v4 cover rtla: fix cgroup and trace options parsing
This series fixes 2 issue in rtla timerlat and osnoise parsing.
1. Make -C/--cgroup option more user-friendly. Currently rtla timerlat and osnoise parses does not allow to specify tracer's threads cgroup name as -C [cgroup] or --cgroup [cgroup]. Second patch fixes this by allowing users to specify cgroup in the aforementioned manner.
2. When specifying -t/--trace before -a/--auto, trace filename is override to default <osnoise|timerlat>_trace.txt. For example, when running rtla as
rtla timerlat top -t custom_file.txt -a 100
when the threshold is reached, timerlat_trace.txt file is created instead of specified custom_file.txt. Third patch addresses this issue.
changes v3 -> v4:
- Removed patch [1] as it has been mainlined
- Resolved merge conflicts with [2]
changes v2 -> v3:
- Combined common logic into a utility function to parse optional argument value
- Removed change that removed clear_terminal
changes v1 -> v2:
- Moved removing clear_terminal from fix -C/--cgroup interface patch to fix -a overriding -t argument patch
- Added clarification why to remove clear_terminal
- Added Fixes: tag to the fix -C/--cgroup interface patch
v3: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
[1] https://lore.kernel.org/all/164ffc2ec8edacaf1295789dad82a07817b6263d.1757034919.git.ipravdin.official@gmail.com/
[2] https://patchwork.kernel.org/project/linux-trace-kernel/list/?series=999703
Ivan Pravdin (2):
rtla: fix -C/--cgroup interface
rtla: fix -a overriding -t argument
Documentation/tools/rtla/common_options.rst | 2 +- tools/tracing/rtla/src/osnoise_hist.c | 29 +++++++-------------- tools/tracing/rtla/src/osnoise_top.c | 28 +++++++------------- tools/tracing/rtla/src/timerlat_hist.c | 29 +++++++-------------- tools/tracing/rtla/src/timerlat_top.c | 29 +++++++-------------- tools/tracing/rtla/src/utils.c | 26 ++++++++++++++++++ tools/tracing/rtla/src/utils.h | 1 + 7 files changed, 64 insertions(+), 80 deletions(-)
Signature · 1 line
2.48.1
PATCH v4 1/2 rtla: fix -C/--cgroup interface
Currently, user can only specify cgroup to the tracer's thread the following ways:
`-C[cgroup]`
`-C[=cgroup]`
`--cgroup[=cgroup]`If user tries to specify cgroup as -C [cgroup] or --cgroup [cgroup], the parser silently fails and rtla's cgroup is used for the tracer threads.
To make interface more user-friendly, allow user to specify cgroup in the aforementioned way, i.e. -C [cgroup] and --cgroup [cgroup].
Refactor identical logic between -t/--trace and -C/--cgroup into a common function.
Change documentation to reflect this user interface change.
--- Documentation/tools/rtla/common_options.rst | 2 +- tools/tracing/rtla/src/osnoise_hist.c | 26 ++++++--------------- tools/tracing/rtla/src/osnoise_top.c | 26 ++++++--------------- tools/tracing/rtla/src/timerlat_hist.c | 26 ++++++--------------- tools/tracing/rtla/src/timerlat_top.c | 26 ++++++--------------- tools/tracing/rtla/src/utils.c | 26 +++++++++++++++++++++ tools/tracing/rtla/src/utils.h | 1 + 7 files changed, 56 insertions(+), 77 deletions(-)
Documentation/tools/rtla/common_options.rst +1 -1
@@ -42,7 +42,7 @@ - *f:prio* - use SCHED_FIFO with *prio*; - *d:runtime[us|ms|s]:period[us|ms|s]* - use SCHED_DEADLINE with *runtime* and *period* in nanoseconds. -**-C**, **--cgroup**\[*=cgroup*] +**-C**, **--cgroup** \[*cgroup*] Set a *cgroup* to the tracer's threads. If the **-C** option is passed without arguments, the tracer's thread will inherit **rtla**'s *cgroup*. Otherwise, the threads will be placed on the *cgroup* passed to the option.
tools/tracing/rtla/src/osnoise_hist.c +7 -19
@@ -428,9 +428,9 @@ static void osnoise_hist_usage(char *usage)static const char *const msg[] = {"" ," usage: rtla osnoise hist [-h] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\" , -" [-T us] [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\" , +" [-T us] [-t [file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\" ," [-c cpu-list] [-H cpu-list] [-P priority] [-b N] [-E N] [--no-header] [--no-summary] \\" , -" [--no-index] [--with-zeros] [-C[=cgroup_name]] [--warm-up]" , +" [--no-index] [--with-zeros] [-C [cgroup_name]] [--warm-up]" ,"" ," -h/--help: print this menu" ," -a/--auto: set automatic trace mode, stopping the session if argument in us sample is hit" ,
@@ -441,10 +441,10 @@ static void osnoise_hist_usage(char *usage)" -T/--threshold us: the minimum delta to be considered a noise" ," -c/--cpus cpu-list: list of cpus to run osnoise threads" ," -H/--house-keeping cpus: run rtla control threads only on the given cpus" , -" -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" , +" -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" ," -d/--duration time[s|m|h|d]: duration of the session" ," -D/--debug: print debug info" , -" -t/--trace[file]: save the stopped trace to [file|osnoise_trace.txt]" , +" -t/--trace [file]: save the stopped trace to [file|osnoise_trace.txt]" ," -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed" ," --filter <filter>: enable a trace event filter to the previous -e event" , " --trigger <trigger>: enable a trace event trigger to the previous -e event",
@@ -574,13 +574,7 @@ static struct common_paramsbreak ;case 'C' : params->common.cgroup =1 ; -if (!optarg) { -/* will inherit this cgroup */ - params->common.cgroup_name =NULL ; - }else if (*optarg =='=' ) { -/* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name =parse_optional_arg (argc, argv);break ;case 'D' : config_debug =1 ;
@@ -646,14 +640,8 @@ static struct common_params params->threshold =get_llong_from_str (optarg);break ;case 't' : -if (optarg) { -if (optarg[0 ] =='=' ) - trace_output = &optarg[1 ]; -else - trace_output = &optarg[0 ]; - }else if (optind < argc && argv[optind][0 ] !='0' ) - trace_output = argv[optind]; -else + trace_output =parse_optional_arg (argc, argv); +if (!trace_output) trace_output ="osnoise_trace.txt" ;break ;case '0' :/* no header */
tools/tracing/rtla/src/osnoise_top.c +7 -19
@@ -263,8 +263,8 @@ static void osnoise_top_usage(struct osnoise_params *params, char *usage)static const char *const msg[] = {" [-h] [-q] [-D] [-d s] [-a us] [-p us] [-r us] [-s us] [-S us] \\" , -" [-T us] [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\" , -" [-c cpu-list] [-H cpu-list] [-P priority] [-C[=cgroup_name]] [--warm-up s]" , +" [-T us] [-t [file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] \\" , +" [-c cpu-list] [-H cpu-list] [-P priority] [-C [cgroup_name]] [--warm-up s]" ,"" ," -h/--help: print this menu" ," -a/--auto: set automatic trace mode, stopping the session if argument in us sample is hit" ,
@@ -275,10 +275,10 @@ static void osnoise_top_usage(struct osnoise_params *params, char *usage)" -T/--threshold us: the minimum delta to be considered a noise" ," -c/--cpus cpu-list: list of cpus to run osnoise threads" ," -H/--house-keeping cpus: run rtla control threads only on the given cpus" , -" -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" , +" -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" ," -d/--duration time[s|m|h|d]: duration of the session" ," -D/--debug: print debug info" , -" -t/--trace[file]: save the stopped trace to [file|osnoise_trace.txt]" , +" -t/--trace [file]: save the stopped trace to [file|osnoise_trace.txt]" ," -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed" ," --filter <filter>: enable a trace event filter to the previous -e event" , " --trigger <trigger>: enable a trace event trigger to the previous -e event",
@@ -408,13 +408,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)break ;case 'C' : params->common.cgroup =1 ; -if (!optarg) { -/* will inherit this cgroup */ - params->common.cgroup_name =NULL ; - }else if (*optarg =='=' ) { -/* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name =parse_optional_arg (argc, argv);break ;case 'D' : config_debug =1 ;
@@ -474,14 +468,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv) params->common.stop_total_us =get_llong_from_str (optarg);break ;case 't' : -if (optarg) { -if (optarg[0 ] =='=' ) - trace_output = &optarg[1 ]; -else - trace_output = &optarg[0 ]; - }else if (optind < argc && argv[optind][0 ] !='-' ) - trace_output = argv[optind]; -else + trace_output =parse_optional_arg (argc, argv); +if (!trace_output) trace_output ="osnoise_trace.txt" ;break ;case 'T' :
tools/tracing/rtla/src/timerlat_hist.c +7 -19
@@ -717,9 +717,9 @@ static void timerlat_hist_usage(char *usage)char *msg[] = {"" ," usage: [rtla] timerlat hist [-h] [-q] [-d s] [-D] [-n] [-a us] [-p us] [-i us] [-T us] [-s us] \\" , -" [-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\" , +" [-t [file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\" ," [-P priority] [-E N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\" , -" [--no-index] [--with-zeros] [--dma-latency us] [-C[=cgroup_name]] [--no-aa] [--dump-task] [-u|-k]" , +" [--no-index] [--with-zeros] [--dma-latency us] [-C [cgroup_name]] [--no-aa] [--dump-task] [-u|-k]" ," [--warm-up s] [--deepest-idle-state n]" ,"" ," -h/--help: print this menu" ,
@@ -730,11 +730,11 @@ static void timerlat_hist_usage(char *usage)" -s/--stack us: save the stack trace at the IRQ if a thread latency is higher than the argument in us" ," -c/--cpus cpus: run the tracer only on the given cpus" ," -H/--house-keeping cpus: run rtla control threads only on the given cpus" , -" -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" , +" -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" ," -d/--duration time[m|h|d]: duration of the session in seconds" ," --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)" ," -D/--debug: print debug info" , -" -t/--trace[file]: save the stopped trace to [file|timerlat_trace.txt]" , +" -t/--trace [file]: save the stopped trace to [file|timerlat_trace.txt]" ," -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed" ," --filter <filter>: enable a trace event filter to the previous -e event" , " --trigger <trigger>: enable a trace event trigger to the previous -e event",
@@ -889,13 +889,7 @@ static struct common_paramsbreak ;case 'C' : params->common.cgroup =1 ; -if (!optarg) { -/* will inherit this cgroup */ - params->common.cgroup_name =NULL ; - }else if (*optarg =='=' ) { -/* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name =parse_optional_arg (argc, argv);break ;case 'b' : params->common.hist.bucket_size =get_llong_from_str (optarg);
@@ -968,14 +962,8 @@ static struct common_params params->common.stop_total_us =get_llong_from_str (optarg);break ;case 't' : -if (optarg) { -if (optarg[0 ] =='=' ) - trace_output = &optarg[1 ]; -else - trace_output = &optarg[0 ]; - }else if (optind < argc && argv[optind][0 ] !='-' ) - trace_output = argv[optind]; -else + trace_output =parse_optional_arg (argc, argv); +if (!trace_output) trace_output ="timerlat_trace.txt" ;break ;case 'u' :
tools/tracing/rtla/src/timerlat_top.c +7 -19
@@ -483,8 +483,8 @@ static void timerlat_top_usage(char *usage)static const char *const msg[] = {"" ," usage: rtla timerlat [top] [-h] [-q] [-a us] [-d s] [-D] [-n] [-p us] [-i us] [-T us] [-s us] \\" , -" [[-t[file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\" , -" [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s] [--deepest-idle-state n]" , +" [[-t [file]] [-e sys[:event]] [--filter <filter>] [--trigger <trigger>] [-c cpu-list] [-H cpu-list]\\" , +" [-P priority] [--dma-latency us] [--aa-only us] [-C [cgroup_name]] [-u|-k] [--warm-up s] [--deepest-idle-state n]" ,"" ," -h/--help: print this menu" ," -a/--auto: set automatic trace mode, stopping the session if argument in us latency is hit" ,
@@ -495,11 +495,11 @@ static void timerlat_top_usage(char *usage)" -s/--stack us: save the stack trace at the IRQ if a thread latency is higher than the argument in us" ," -c/--cpus cpus: run the tracer only on the given cpus" ," -H/--house-keeping cpus: run rtla control threads only on the given cpus" , -" -C/--cgroup[=cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" , +" -C/--cgroup [cgroup_name]: set cgroup, if no cgroup_name is passed, the rtla's cgroup will be inherited" ," -d/--duration time[s|m|h|d]: duration of the session" ," -D/--debug: print debug info" ," --dump-tasks: prints the task running on all CPUs if stop conditions are met (depends on !--no-aa)" , -" -t/--trace[file]: save the stopped trace to [file|timerlat_trace.txt]" , +" -t/--trace [file]: save the stopped trace to [file|timerlat_trace.txt]" ," -e/--event <sys:event>: enable the <sys:event> in the trace instance, multiple -e are allowed" ," --filter <command>: enable a trace event filter to the previous -e event" , " --trigger <command>: enable a trace event trigger to the previous -e event",
@@ -653,13 +653,7 @@ static struct common_paramsbreak ;case 'C' : params->common.cgroup =1 ; -if (!optarg) { -/* will inherit this cgroup */ - params->common.cgroup_name =NULL ; - }else if (*optarg =='=' ) { -/* skip the = */ - params->common.cgroup_name = ++optarg; - } + params->common.cgroup_name = optarg;break ;case 'D' : config_debug =1 ;
@@ -722,14 +716,8 @@ static struct common_params params->common.stop_total_us =get_llong_from_str (optarg);break ;case 't' : -if (optarg) { -if (optarg[0 ] =='=' ) - trace_output = &optarg[1 ]; -else - trace_output = &optarg[0 ]; - }else if (optind < argc && argv[optind][0 ] !='-' ) - trace_output = argv[optind]; -else + trace_output =parse_optional_arg (argc, argv); +if (!trace_output) trace_output ="timerlat_trace.txt" ;break ;case 'u' :
tools/tracing/rtla/src/utils.c +26 -0
@@ -959,3 +959,29 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)return 1 ; } + +/** +* parse_optional_arg - Parse optional argument value +* +* Parse optional argument value, which can be in the form of: +* -sarg, -s/--long=arg, -s/--long arg +* +* Returns arg value if found, NULL otherwise. +*/ +char *parse_optional_arg (int argc,char **argv) +{ +if (optarg) { +if (optarg[0 ] =='=' ) { +/* skip the = */ +return &optarg[1 ]; + }else { +return optarg; + } +/* parse argument of form -s [arg] and --long [arg]*/ + }else if (optind < argc && argv[optind][0 ] !='-' ) { +/* consume optind */ +return argv[optind++]; + }else { +return NULL ; + } +}
tools/tracing/rtla/src/utils.h +1 -0
@@ -24,6 +24,7 @@ long parse_seconds_duration(char *val);void get_duration (time_t start_time,char *output,int output_size);int parse_cpu_list (char *cpu_list,char **monitored_cpus); +char *parse_optional_arg (int argc,char **argv);long long get_llong_from_str (char *start); static inline void
Signature · 1 line
2.48.1
PATCH v4 2/2 rtla: fix -a overriding -t argument
In reply to Ivan PravdinWhen running rtla as
rtla <timerlat|osnoise> <top|hist> -t custom_file.txt -a 100
-a options override trace output filename specified by -t option. Running the command above will create <timerlat|osnoise>_trace.txt file instead of custom_file.txt. Fix this by making sure that -a option does not override trace output filename even if it's passed after trace output filename is specified.
--- tools/tracing/rtla/src/osnoise_hist.c | 3 ++- tools/tracing/rtla/src/osnoise_top.c | 2 ++ tools/tracing/rtla/src/timerlat_hist.c | 3 ++- tools/tracing/rtla/src/timerlat_top.c | 3 ++- 4 files changed, 8 insertions(+), 3 deletions(-)
tools/tracing/rtla/src/osnoise_hist.c +2 -1
@@ -557,7 +557,8 @@ static struct common_params params->threshold =1 ;/* set trace */ - trace_output ="osnoise_trace.txt" ; +if (!trace_output) + trace_output ="osnoise_trace.txt" ;break ;case 'b' :
tools/tracing/rtla/src/osnoise_top.c +2 -0
@@ -398,6 +398,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)/* set trace */ trace_output ="osnoise_trace.txt" ; +if (!trace_output) + trace_output ="osnoise_trace.txt" ;break ;case 'c' :
tools/tracing/rtla/src/timerlat_hist.c +2 -1
@@ -878,7 +878,8 @@ static struct common_params params->print_stack = auto_thresh;/* set trace */ - trace_output ="timerlat_trace.txt" ; +if (!trace_output) + trace_output ="timerlat_trace.txt" ;break ;case 'c' :
tools/tracing/rtla/src/timerlat_top.c +2 -1
@@ -628,7 +628,8 @@ static struct common_params params->print_stack = auto_thresh;/* set trace */ - trace_output ="timerlat_trace.txt" ; +if (!trace_output) + trace_output ="timerlat_trace.txt" ;break ;case '5' :
Signature · 1 line
2.48.1