From deaec85cd8bad3841412ff8cefec463f2688806b Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 29 Jun 2026 10:36:51 +0200 Subject: rtla: Allow unsetting non-list custom-callback CLI options libsubcmd implicitly allows the user to unset already set options using a "no-" prefix for long options. For example, if I set the period like this: $ rtla timerlat -D Loading BPF program reading osnoise/timerlat_period_us returned 1000 setting osnoise/timerlat_period_us to 1000 reading osnoise/print_stack returned 0 setting osnoise/print_stack to 0 ... it can be unset by a subsequent --no-debug: $ rtla timerlat -D --no-debug ... Currently, this works only for boolean options. Extend the feature for all options by implementing handling of the "unset" argument in opt_*() callbacks defined in cli_p.h, except for list options, i.e. options that can be passed multiple times (--event, --filter, --trigger, --on-threshold, --on-end). This allows, for example, unsetting of int/long long options, e.g. "-p": $ rtla timerlat -D -p100 --no-period ... setting osnoise/timerlat_period_us to 1000 ... By default, options in params struct are reset to zero. A constant is added for every parameter with a different default value, which is then used both in _hist_args() while setting the initial value and in opt_*() when unsetting the option. This refactoring ensures there is no duplicate "magic number". The default value for opt_llong_callback() and opt_int_callback() is passed in struct option's defval field; new macros RTLA_OPT_{LLONG,INT}{,_DEFVAL} are added to define the field conveniently. The default value for other callbacks is hardcoded inside each callback's unset logic. Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20260629083654.1548925-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/cli.c | 42 +++----- tools/tracing/rtla/src/cli_p.h | 219 +++++++++++++++++++++++++++++++++-------- 2 files changed, 194 insertions(+), 67 deletions(-) diff --git a/tools/tracing/rtla/src/cli.c b/tools/tracing/rtla/src/cli.c index c5279c987531..fb8c972c0746 100644 --- a/tools/tracing/rtla/src/cli.c +++ b/tools/tracing/rtla/src/cli.c @@ -192,10 +192,10 @@ struct common_params *osnoise_hist_parse_args(int argc, char **argv) actions_init(¶ms->common.threshold_actions); actions_init(¶ms->common.end_actions); - /* display data in microseconds */ - params->common.output_divisor = 1000; - params->common.hist.bucket_size = 1; - params->common.hist.entries = 256; + /* set default values */ + params->common.output_divisor = default_output_divisor; + params->common.hist.bucket_size = default_bucket_size; + params->common.hist.entries = default_entries; argc = parse_options(argc, (const char **)argv, osnoise_hist_options, osnoise_hist_usage, @@ -280,19 +280,15 @@ struct common_params *timerlat_top_parse_args(int argc, char **argv) actions_init(¶ms->common.threshold_actions); actions_init(¶ms->common.end_actions); - /* disabled by default */ - params->dma_latency = -1; - params->deepest_idle_state = -2; - - /* display data in microseconds */ - params->common.output_divisor = 1000; + /* set default values */ + params->dma_latency = default_dma_latency; + params->deepest_idle_state = default_deepest_idle_state; + params->common.output_divisor = default_output_divisor; + params->stack_format = default_stack_format; /* default to BPF mode */ params->mode = TRACING_MODE_BPF; - /* default to truncate stack format */ - params->stack_format = STACK_FORMAT_TRUNCATE; - argc = parse_options(argc, (const char **)argv, timerlat_top_options, timerlat_top_usage, common_parse_options_flags); @@ -403,23 +399,17 @@ struct common_params *timerlat_hist_parse_args(int argc, char **argv) actions_init(¶ms->common.threshold_actions); actions_init(¶ms->common.end_actions); - /* disabled by default */ - params->dma_latency = -1; - - /* disabled by default */ - params->deepest_idle_state = -2; - - /* display data in microseconds */ - params->common.output_divisor = 1000; - params->common.hist.bucket_size = 1; - params->common.hist.entries = 256; + /* set default values */ + params->dma_latency = default_dma_latency; + params->deepest_idle_state = default_deepest_idle_state; + params->common.output_divisor = default_output_divisor; + params->common.hist.bucket_size = default_bucket_size; + params->common.hist.entries = default_entries; + params->stack_format = default_stack_format; /* default to BPF mode */ params->mode = TRACING_MODE_BPF; - /* default to truncate stack format */ - params->stack_format = STACK_FORMAT_TRUNCATE; - argc = parse_options(argc, (const char **)argv, timerlat_hist_options, timerlat_hist_usage, common_parse_options_flags); diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h index 3c939de9abf0..3a93dba60215 100644 --- a/tools/tracing/rtla/src/cli_p.h +++ b/tools/tracing/rtla/src/cli_p.h @@ -22,6 +22,38 @@ struct timerlat_cb_data { char *trace_output; }; +/* + * Non-zero default values for parameters + */ +static const int default_dma_latency = -1; /* -1 = unset */ +static const int default_deepest_idle_state = -2; /* -1 = disable all, -2 = unset */ +static const int default_output_divisor = 1000; +static const int default_bucket_size = 1; +static const int default_entries = 256; +static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE; + +/* + * Shorthand macros for integer/long long command line options using + * opt_int_callback/opt_llong_callback, with variants that set defval. + * + * Note: defval's type is intptr_t. opt_int_callback interprets it directly as + * an int, opt_llong_callback interprets it as a pointer to a long long, as + * long long does not fit into intptr_t on 32-bit architectures. + */ +#define RTLA_OPT_LLONG(s, l, v, a, h) \ + OPT_CALLBACK(s, l, v, a, h, opt_llong_callback) + +#define RTLA_OPT_LLONG_DEFVAL(s, l, v, a, h, d) { .type = OPTION_CALLBACK, \ + .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ + .help = (h), .callback = opt_llong_callback, .defval = (intptr_t)(d) } + +#define RTLA_OPT_INT(s, l, v, a, h) \ + OPT_CALLBACK(s, l, v, a, h, opt_int_callback) + +#define RTLA_OPT_INT_DEFVAL(s, l, v, a, h, d) { .type = OPTION_CALLBACK, \ + .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ + .help = (h), .callback = opt_int_callback, .defval = (intptr_t)(d) } + /* * Macros for command line options common to all tools * @@ -108,14 +140,12 @@ struct timerlat_cb_data { #define RTLA_OPT_QUIET OPT_BOOLEAN('q', "quiet", ¶ms->common.quiet, \ "print only a summary at the end") -#define RTLA_OPT_TRACE_BUFFER_SIZE OPT_CALLBACK(0, "trace-buffer-size", \ +#define RTLA_OPT_TRACE_BUFFER_SIZE RTLA_OPT_INT(0, "trace-buffer-size", \ ¶ms->common.buffer_size, "kB", \ - "set the per-cpu trace buffer size in kB", \ - opt_int_callback) + "set the per-cpu trace buffer size in kB") -#define RTLA_OPT_WARM_UP OPT_CALLBACK(0, "warm-up", ¶ms->common.warmup, "s", \ - "let the workload run for s seconds before collecting data", \ - opt_int_callback) +#define RTLA_OPT_WARM_UP RTLA_OPT_INT(0, "warm-up", ¶ms->common.warmup, "s", \ + "let the workload run for s seconds before collecting data") #define RTLA_OPT_AUTO(cb) OPT_CALLBACK('a', "auto", &cb_data, "us", \ "set automatic trace mode, stopping the session if argument in us sample is hit", \ @@ -143,7 +173,12 @@ static int opt_llong_callback(const struct option *opt, const char *arg, int uns { long long *value = opt->value; - if (unset || !arg) + if (unset) { + *value = opt->defval ? *(long long *)opt->defval : 0; + return 0; + } + + if (!arg) return -1; *value = get_llong_from_str((char *)arg); @@ -154,7 +189,12 @@ static int opt_int_callback(const struct option *opt, const char *arg, int unset { int *value = opt->value; - if (unset || !arg) + if (unset) { + *value = (int)opt->defval; + return 0; + } + + if (!arg) return -1; if (strtoi(arg, value)) @@ -168,7 +208,13 @@ static int opt_cpus_cb(const struct option *opt, const char *arg, int unset) struct common_params *params = opt->value; int retval; - if (unset || !arg) + if (unset) { + CPU_ZERO(¶ms->monitored_cpus); + params->cpus = NULL; + return 0; + } + + if (!arg) return -1; retval = parse_cpu_set((char *)arg, ¶ms->monitored_cpus); @@ -183,8 +229,11 @@ static int opt_cgroup_cb(const struct option *opt, const char *arg, int unset) { struct common_params *params = opt->value; - if (unset) - return -1; + if (unset) { + params->cgroup = 0; + params->cgroup_name = NULL; + return 0; + } params->cgroup = 1; params->cgroup_name = (char *)arg; @@ -199,7 +248,12 @@ static int opt_duration_cb(const struct option *opt, const char *arg, int unset) { struct common_params *params = opt->value; - if (unset || !arg) + if (unset) { + params->duration = 0; + return 0; + } + + if (!arg) return -1; params->duration = parse_seconds_duration((char *)arg); @@ -233,7 +287,13 @@ static int opt_housekeeping_cb(const struct option *opt, const char *arg, int un struct common_params *params = opt->value; int retval; - if (unset || !arg) + if (unset) { + params->hk_cpus = 0; + CPU_ZERO(¶ms->hk_cpu_set); + return 0; + } + + if (!arg) return -1; params->hk_cpus = 1; @@ -249,7 +309,13 @@ static int opt_priority_cb(const struct option *opt, const char *arg, int unset) struct common_params *params = opt->value; int retval; - if (unset || !arg) + if (unset) { + memset(¶ms->sched_param, 0, sizeof(params->sched_param)); + params->set_sched = 0; + return 0; + } + + if (!arg) return -1; retval = parse_prio((char *)arg, ¶ms->sched_param); @@ -301,9 +367,8 @@ static int opt_filter_cb(const struct option *opt, const char *arg, int unset) "osnoise runtime in us", \ opt_osnoise_runtime_cb) -#define OSNOISE_OPT_THRESHOLD OPT_CALLBACK('T', "threshold", ¶ms->threshold, "us", \ - "the minimum delta to be considered a noise", \ - opt_llong_callback) +#define OSNOISE_OPT_THRESHOLD RTLA_OPT_LLONG('T', "threshold", ¶ms->threshold, "us", \ + "the minimum delta to be considered a noise") /* * Callback functions for command line options for osnoise tools @@ -315,7 +380,14 @@ static int opt_osnoise_auto_cb(const struct option *opt, const char *arg, int un struct osnoise_params *params = cb_data->params; long long auto_thresh; - if (unset || !arg) + if (unset) { + params->common.stop_us = 0; + params->threshold = 0; + cb_data->trace_output = NULL; + return 0; + } + + if (!arg) return -1; auto_thresh = get_llong_from_str((char *)arg); @@ -332,7 +404,12 @@ static int opt_osnoise_period_cb(const struct option *opt, const char *arg, int { unsigned long long *period = opt->value; - if (unset || !arg) + if (unset) { + *period = 0; + return 0; + } + + if (!arg) return -1; *period = get_llong_from_str((char *)arg); @@ -346,7 +423,12 @@ static int opt_osnoise_runtime_cb(const struct option *opt, const char *arg, int { unsigned long long *runtime = opt->value; - if (unset || !arg) + if (unset) { + *runtime = 0; + return 0; + } + + if (!arg) return -1; *runtime = get_llong_from_str((char *)arg); @@ -360,8 +442,10 @@ static int opt_osnoise_trace_output_cb(const struct option *opt, const char *arg { const char **trace_output = opt->value; - if (unset) - return -1; + if (unset) { + *trace_output = NULL; + return 0; + } if (!arg) { *trace_output = "osnoise_trace.txt"; @@ -412,9 +496,8 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int "timerlat period in us", \ opt_timerlat_period_cb) -#define TIMERLAT_OPT_STACK OPT_CALLBACK('s', "stack", ¶ms->print_stack, "us", \ - "save the stack trace at the IRQ if a thread latency is higher than the argument in us", \ - opt_llong_callback) +#define TIMERLAT_OPT_STACK RTLA_OPT_LLONG('s', "stack", ¶ms->print_stack, "us", \ + "save the stack trace at the IRQ if a thread latency is higher than the argument in us") #define TIMERLAT_OPT_NANO OPT_CALLBACK_NOOPT('n', "nano", params, NULL, \ "display data in nanoseconds", \ @@ -424,10 +507,10 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int "set /dev/cpu_dma_latency latency to reduce exit from idle latency", \ opt_dma_latency_cb) -#define TIMERLAT_OPT_DEEPEST_IDLE_STATE OPT_CALLBACK(0, "deepest-idle-state", \ +#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DEFVAL(0, "deepest-idle-state", \ ¶ms->deepest_idle_state, "n", \ "only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", \ - opt_int_callback) + default_deepest_idle_state) #define TIMERLAT_OPT_AA_ONLY OPT_CALLBACK(0, "aa-only", params, "us", \ "stop if latency is hit, only printing the auto analysis (reduces CPU usage)", \ @@ -459,7 +542,12 @@ static int opt_timerlat_period_cb(const struct option *opt, const char *arg, int { long long *period = opt->value; - if (unset || !arg) + if (unset) { + *period = 0; + return 0; + } + + if (!arg) return -1; *period = get_llong_from_str((char *)arg); @@ -475,7 +563,15 @@ static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int u struct timerlat_params *params = cb_data->params; long long auto_thresh; - if (unset || !arg) + if (unset) { + params->common.stop_total_us = 0; + params->common.stop_us = 0; + params->print_stack = 0; + cb_data->trace_output = NULL; + return 0; + } + + if (!arg) return -1; auto_thresh = get_llong_from_str((char *)arg); @@ -494,7 +590,12 @@ static int opt_dma_latency_cb(const struct option *opt, const char *arg, int uns int *dma_latency = opt->value; int retval; - if (unset || !arg) + if (unset) { + *dma_latency = default_dma_latency; + return 0; + } + + if (!arg) return -1; retval = strtoi((char *)arg, dma_latency); @@ -511,7 +612,15 @@ static int opt_aa_only_cb(const struct option *opt, const char *arg, int unset) struct timerlat_params *params = opt->value; long long auto_thresh; - if (unset || !arg) + if (unset) { + params->common.stop_total_us = 0; + params->common.stop_us = 0; + params->print_stack = 0; + params->common.aa_only = 0; + return 0; + } + + if (!arg) return -1; auto_thresh = get_llong_from_str((char *)arg); @@ -527,8 +636,10 @@ static int opt_timerlat_trace_output_cb(const struct option *opt, const char *ar { const char **trace_output = opt->value; - if (unset) - return -1; + if (unset) { + *trace_output = NULL; + return 0; + } if (!arg) { *trace_output = "timerlat_trace.txt"; @@ -576,8 +687,11 @@ static int opt_user_threads_cb(const struct option *opt, const char *arg, int un { struct timerlat_params *params = opt->value; - if (unset) - return -1; + if (unset) { + params->common.user_workload = false; + params->common.user_data = false; + return 0; + } params->common.user_workload = true; params->common.user_data = true; @@ -589,8 +703,10 @@ static int opt_nano_cb(const struct option *opt, const char *arg, int unset) { struct timerlat_params *params = opt->value; - if (unset) - return -1; + if (unset) { + params->common.output_divisor = default_output_divisor; + return 0; + } params->common.output_divisor = 1; @@ -601,7 +717,12 @@ static int opt_stack_format_cb(const struct option *opt, const char *arg, int un { int *format = opt->value; - if (unset || !arg) + if (unset) { + *format = default_stack_format; + return 0; + } + + if (!arg) return -1; *format = parse_stack_format((char *)arg); @@ -616,7 +737,13 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int { struct timerlat_params *params = opt->value; - if (unset || !arg) + if (unset) { + params->timerlat_align = false; + params->timerlat_align_us = 0; + return 0; + } + + if (!arg) return -1; params->timerlat_align = true; @@ -662,7 +789,12 @@ static int opt_bucket_size_cb(const struct option *opt, const char *arg, int uns { int *bucket_size = opt->value; - if (unset || !arg) + if (unset) { + *bucket_size = default_bucket_size; + return 0; + } + + if (!arg) return -1; *bucket_size = get_llong_from_str((char *)arg); @@ -676,7 +808,12 @@ static int opt_entries_cb(const struct option *opt, const char *arg, int unset) { int *entries = opt->value; - if (unset || !arg) + if (unset) { + *entries = default_entries; + return 0; + } + + if (!arg) return -1; *entries = get_llong_from_str((char *)arg); -- cgit From 082b1c2c228c6a36e94a5d272dd5ee23c5df93bc Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 29 Jun 2026 10:36:52 +0200 Subject: rtla: Add unit tests for unset in opt callbacks Test for each opt callback that implements the unset option whether the option sets the specified default value back correctly. Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20260629083654.1548925-2-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/unit/cli_opt_callback.c | 295 +++++++++++++++++++++++ 1 file changed, 295 insertions(+) diff --git a/tools/tracing/rtla/tests/unit/cli_opt_callback.c b/tools/tracing/rtla/tests/unit/cli_opt_callback.c index 4a406af42821..413a04f898fb 100644 --- a/tools/tracing/rtla/tests/unit/cli_opt_callback.c +++ b/tools/tracing/rtla/tests/unit/cli_opt_callback.c @@ -40,6 +40,30 @@ START_TEST(test_opt_llong_callback_min) } END_TEST +START_TEST(test_opt_llong_callback_unset) +{ + long long test_value = 0; + const struct option opt = TEST_CALLBACK(&test_value, opt_llong_callback); + + ck_assert_int_eq(opt_llong_callback(&opt, "1234567890", 0), 0); + ck_assert_int_eq(opt_llong_callback(&opt, NULL, 1), 0); + ck_assert_int_eq(test_value, 0); +} +END_TEST + +START_TEST(test_opt_llong_callback_unset_defval) +{ + long long test_value = 0; + const long long default_value = 42; + const struct option opt = RTLA_OPT_LLONG_DEFVAL('t', "test", &test_value, "test value", + "test help", &default_value); + + ck_assert_int_eq(opt_llong_callback(&opt, "1234567890", 0), 0); + ck_assert_int_eq(opt_llong_callback(&opt, NULL, 1), 0); + ck_assert_int_eq(test_value, default_value); +} +END_TEST + START_TEST(test_opt_int_callback_simple) { int test_value = 0; @@ -90,6 +114,29 @@ START_TEST(test_opt_int_callback_non_numeric_suffix) } END_TEST +START_TEST(test_opt_int_callback_unset) +{ + int test_value = 0; + const struct option opt = TEST_CALLBACK(&test_value, opt_int_callback); + + ck_assert_int_eq(opt_int_callback(&opt, "1234567890", 0), 0); + ck_assert_int_eq(opt_int_callback(&opt, NULL, 1), 0); + ck_assert_int_eq(test_value, 0); +} +END_TEST + +START_TEST(test_opt_int_callback_unset_defval) +{ + int test_value = 0; + const struct option opt = RTLA_OPT_INT_DEFVAL('t', "test", &test_value, "test value", + "test help", 42); + + ck_assert_int_eq(opt_int_callback(&opt, "1234567890", 0), 0); + ck_assert_int_eq(opt_int_callback(&opt, NULL, 1), 0); + ck_assert_int_eq(test_value, 42); +} +END_TEST + START_TEST(test_opt_cpus_cb) { struct common_params params = {0}; @@ -134,6 +181,18 @@ START_TEST(test_opt_cgroup_cb_equals) } END_TEST +START_TEST(test_opt_cgroup_cb_unset) +{ + struct common_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_cgroup_cb); + + ck_assert_int_eq(opt_cgroup_cb(&opt, "cgroup", 0), 0); + ck_assert_int_eq(opt_cgroup_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.cgroup, 0); + ck_assert_ptr_null(params.cgroup_name); +} +END_TEST + START_TEST(test_opt_duration_cb) { struct common_params params = {0}; @@ -154,6 +213,17 @@ START_TEST(test_opt_duration_cb_invalid) } END_TEST +START_TEST(test_opt_duration_cb_unset) +{ + struct common_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_duration_cb); + + ck_assert_int_eq(opt_duration_cb(&opt, "1m", 0), 0); + ck_assert_int_eq(opt_duration_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.duration, 0); +} +END_TEST + START_TEST(test_opt_event_cb) { struct trace_events *events = NULL; @@ -205,6 +275,19 @@ START_TEST(test_opt_housekeeping_cb_invalid) } END_TEST +START_TEST(test_opt_housekeeping_cb_unset) +{ + struct common_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_housekeeping_cb); + + nr_cpus = 4; + ck_assert_int_eq(opt_housekeeping_cb(&opt, "0-3", 0), 0); + ck_assert_int_eq(opt_housekeeping_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.hk_cpus, 0); + ck_assert_int_eq(CPU_COUNT(¶ms.hk_cpu_set), 0); +} +END_TEST + START_TEST(test_opt_priority_cb) { struct common_params params = {0}; @@ -226,6 +309,18 @@ START_TEST(test_opt_priority_cb_invalid) } END_TEST +START_TEST(test_opt_priority_cb_unset) +{ + struct common_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_priority_cb); + + ck_assert_int_eq(opt_priority_cb(&opt, "f:95", 0), 0); + ck_assert_int_eq(opt_priority_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.sched_param.sched_policy, 0); + ck_assert_int_eq(params.sched_param.sched_priority, 0); +} +END_TEST + START_TEST(test_opt_trigger_cb) { struct trace_events *events = trace_event_alloc("sched:sched_switch"); @@ -279,6 +374,20 @@ START_TEST(test_opt_osnoise_auto_cb) } END_TEST +START_TEST(test_opt_osnoise_auto_cb_unset) +{ + struct osnoise_params params = {0}; + struct osnoise_cb_data cb_data = {¶ms}; + const struct option opt = TEST_CALLBACK(&cb_data, opt_osnoise_auto_cb); + + ck_assert_int_eq(opt_osnoise_auto_cb(&opt, "10", 0), 0); + ck_assert_int_eq(opt_osnoise_auto_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.common.stop_us, 0); + ck_assert_int_eq(params.threshold, 0); + ck_assert_ptr_null(cb_data.trace_output); +} +END_TEST + START_TEST(test_opt_osnoise_period_cb) { unsigned long long period = 0; @@ -299,6 +408,17 @@ START_TEST(test_opt_osnoise_period_cb_invalid) } END_TEST +START_TEST(test_opt_osnoise_period_cb_unset) +{ + unsigned long long period = 0; + const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb); + + ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0); + ck_assert_int_eq(opt_osnoise_period_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(period, 0); +} +END_TEST + START_TEST(test_opt_osnoise_runtime_cb) { unsigned long long runtime = 0; @@ -319,6 +439,17 @@ START_TEST(test_opt_osnoise_runtime_cb_invalid) } END_TEST +START_TEST(test_opt_osnoise_runtime_cb_unset) +{ + unsigned long long runtime = 0; + const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb); + + ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0); + ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(runtime, 0); +} +END_TEST + START_TEST(test_opt_osnoise_trace_output_cb) { const char *trace_output = NULL; @@ -339,6 +470,17 @@ START_TEST(test_opt_osnoise_trace_output_cb_noarg) } END_TEST +START_TEST(test_opt_osnoise_trace_output_cb_unset) +{ + const char *trace_output = NULL; + const struct option opt = TEST_CALLBACK(&trace_output, opt_osnoise_trace_output_cb); + + ck_assert_int_eq(opt_osnoise_trace_output_cb(&opt, "trace.txt", 0), 0); + ck_assert_int_eq(opt_osnoise_trace_output_cb(&opt, NULL, 1), 0); + ck_assert_ptr_null(trace_output); +} +END_TEST + START_TEST(test_opt_osnoise_on_threshold_cb) { struct actions actions = {0}; @@ -403,6 +545,17 @@ START_TEST(test_opt_timerlat_period_cb_invalid) } END_TEST +START_TEST(test_opt_timerlat_period_cb_unset) +{ + long long period = 0; + const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb); + + ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0); + ck_assert_int_eq(opt_timerlat_period_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(period, 0); +} +END_TEST + START_TEST(test_opt_timerlat_auto_cb) { struct timerlat_params params = {0}; @@ -417,6 +570,21 @@ START_TEST(test_opt_timerlat_auto_cb) } END_TEST +START_TEST(test_opt_timerlat_auto_cb_unset) +{ + struct timerlat_params params = {0}; + struct timerlat_cb_data cb_data = {¶ms}; + const struct option opt = TEST_CALLBACK(&cb_data, opt_timerlat_auto_cb); + + ck_assert_int_eq(opt_timerlat_auto_cb(&opt, "10", 0), 0); + ck_assert_int_eq(opt_timerlat_auto_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.common.stop_us, 0); + ck_assert_int_eq(params.common.stop_total_us, 0); + ck_assert_int_eq(params.print_stack, 0); + ck_assert_ptr_null(cb_data.trace_output); +} +END_TEST + START_TEST(test_opt_dma_latency_cb) { int dma_latency = 0; @@ -447,6 +615,17 @@ START_TEST(test_opt_dma_latency_cb_max) } END_TEST +START_TEST(test_opt_dma_latency_cb_unset) +{ + int dma_latency = 0; + const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb); + + ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0); + ck_assert_int_eq(opt_dma_latency_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(dma_latency, default_dma_latency); +} +END_TEST + START_TEST(test_opt_aa_only_cb) { struct timerlat_params params = {0}; @@ -460,6 +639,20 @@ START_TEST(test_opt_aa_only_cb) } END_TEST +START_TEST(test_opt_aa_only_cb_unset) +{ + struct timerlat_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_aa_only_cb); + + ck_assert_int_eq(opt_aa_only_cb(&opt, "10", 0), 0); + ck_assert_int_eq(opt_aa_only_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.common.stop_us, 0); + ck_assert_int_eq(params.common.stop_total_us, 0); + ck_assert_int_eq(params.print_stack, 0); + ck_assert_int_eq(params.common.aa_only, 0); +} +END_TEST + START_TEST(test_opt_timerlat_trace_output_cb) { const char *trace_output = NULL; @@ -480,6 +673,17 @@ START_TEST(test_opt_timerlat_trace_output_cb_noarg) } END_TEST +START_TEST(test_opt_timerlat_trace_output_cb_unset) +{ + const char *trace_output = NULL; + const struct option opt = TEST_CALLBACK(&trace_output, opt_timerlat_trace_output_cb); + + ck_assert_int_eq(opt_timerlat_trace_output_cb(&opt, "trace.txt", 0), 0); + ck_assert_int_eq(opt_timerlat_trace_output_cb(&opt, NULL, 1), 0); + ck_assert_ptr_null(trace_output); +} +END_TEST + START_TEST(test_opt_timerlat_on_threshold_cb) { struct actions actions = {0}; @@ -535,6 +739,18 @@ START_TEST(test_opt_user_threads_cb) } END_TEST +START_TEST(test_opt_user_threads_cb_unset) +{ + struct timerlat_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_user_threads_cb); + + ck_assert_int_eq(opt_user_threads_cb(&opt, NULL, 0), 0); + ck_assert_int_eq(opt_user_threads_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.common.user_workload, 0); + ck_assert_int_eq(params.common.user_data, 0); +} +END_TEST + START_TEST(test_opt_nano_cb) { struct timerlat_params params = {0}; @@ -545,6 +761,17 @@ START_TEST(test_opt_nano_cb) } END_TEST +START_TEST(test_opt_nano_cb_unset) +{ + struct timerlat_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_nano_cb); + + ck_assert_int_eq(opt_nano_cb(&opt, NULL, 0), 0); + ck_assert_int_eq(opt_nano_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.common.output_divisor, default_output_divisor); +} +END_TEST + START_TEST(test_opt_timerlat_align_cb) { struct timerlat_params params = {0}; @@ -556,6 +783,18 @@ START_TEST(test_opt_timerlat_align_cb) } END_TEST +START_TEST(test_opt_timerlat_align_cb_unset) +{ + struct timerlat_params params = {0}; + const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb); + + ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0); + ck_assert_int_eq(opt_timerlat_align_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(params.timerlat_align, 0); + ck_assert_int_eq(params.timerlat_align_us, 0); +} +END_TEST + START_TEST(test_opt_stack_format_cb) { int stack_format = 0; @@ -576,6 +815,17 @@ START_TEST(test_opt_stack_format_cb_invalid) } END_TEST +START_TEST(test_opt_stack_format_cb_unset) +{ + int stack_format = 0; + const struct option opt = TEST_CALLBACK(&stack_format, opt_stack_format_cb); + + ck_assert_int_eq(opt_stack_format_cb(&opt, "full", 0), 0); + ck_assert_int_eq(opt_stack_format_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(stack_format, default_stack_format); +} +END_TEST + START_TEST(test_opt_bucket_size_cb) { int bucket_size = 0; @@ -606,6 +856,17 @@ START_TEST(test_opt_bucket_size_max) } END_TEST +START_TEST(test_opt_bucket_size_cb_unset) +{ + int bucket_size = 0; + const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb); + + ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0); + ck_assert_int_eq(opt_bucket_size_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(bucket_size, default_bucket_size); +} +END_TEST + START_TEST(test_opt_entries_cb) { int entries = 0; @@ -636,6 +897,17 @@ START_TEST(test_opt_entries_max) } END_TEST +START_TEST(test_opt_entries_cb_unset) +{ + int entries = 0; + const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb); + + ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0); + ck_assert_int_eq(opt_entries_cb(&opt, NULL, 1), 0); + ck_assert_int_eq(entries, default_entries); +} +END_TEST + Suite *cli_opt_callback_suite(void) { Suite *s = suite_create("cli_opt_callback"); @@ -645,23 +917,31 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_llong_callback_simple); tcase_add_test(tc, test_opt_llong_callback_max); tcase_add_test(tc, test_opt_llong_callback_min); + tcase_add_test(tc, test_opt_llong_callback_unset); + tcase_add_test(tc, test_opt_llong_callback_unset_defval); tcase_add_test(tc, test_opt_int_callback_simple); tcase_add_test(tc, test_opt_int_callback_max); tcase_add_test(tc, test_opt_int_callback_min); tcase_add_test(tc, test_opt_int_callback_non_numeric); tcase_add_test(tc, test_opt_int_callback_non_numeric_suffix); + tcase_add_test(tc, test_opt_int_callback_unset); + tcase_add_test(tc, test_opt_int_callback_unset_defval); tcase_add_test(tc, test_opt_cpus_cb); tcase_add_exit_test(tc, test_opt_cpus_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_cgroup_cb); tcase_add_test(tc, test_opt_cgroup_cb_equals); + tcase_add_test(tc, test_opt_cgroup_cb_unset); tcase_add_test(tc, test_opt_duration_cb); + tcase_add_test(tc, test_opt_duration_cb_unset); tcase_add_exit_test(tc, test_opt_duration_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_event_cb); tcase_add_test(tc, test_opt_event_cb_multiple); tcase_add_test(tc, test_opt_housekeeping_cb); tcase_add_exit_test(tc, test_opt_housekeeping_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_housekeeping_cb_unset); tcase_add_test(tc, test_opt_priority_cb); tcase_add_exit_test(tc, test_opt_priority_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_priority_cb_unset); tcase_add_test(tc, test_opt_trigger_cb); tcase_add_exit_test(tc, test_opt_trigger_cb_no_event, EXIT_FAILURE); tcase_add_test(tc, test_opt_filter_cb); @@ -670,12 +950,16 @@ Suite *cli_opt_callback_suite(void) tc = tcase_create("osnoise"); tcase_add_test(tc, test_opt_osnoise_auto_cb); + tcase_add_test(tc, test_opt_osnoise_auto_cb_unset); tcase_add_test(tc, test_opt_osnoise_period_cb); + tcase_add_test(tc, test_opt_osnoise_period_cb_unset); tcase_add_exit_test(tc, test_opt_osnoise_period_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_osnoise_runtime_cb); tcase_add_exit_test(tc, test_opt_osnoise_runtime_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_osnoise_runtime_cb_unset); tcase_add_test(tc, test_opt_osnoise_trace_output_cb); tcase_add_test(tc, test_opt_osnoise_trace_output_cb_noarg); + tcase_add_test(tc, test_opt_osnoise_trace_output_cb_unset); tcase_add_test(tc, test_opt_osnoise_on_threshold_cb); tcase_add_exit_test(tc, test_opt_osnoise_on_threshold_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_osnoise_on_end_cb); @@ -685,31 +969,42 @@ Suite *cli_opt_callback_suite(void) tc = tcase_create("timerlat"); tcase_add_test(tc, test_opt_timerlat_period_cb); tcase_add_exit_test(tc, test_opt_timerlat_period_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_timerlat_period_cb_unset); tcase_add_test(tc, test_opt_timerlat_auto_cb); + tcase_add_test(tc, test_opt_timerlat_auto_cb_unset); tcase_add_test(tc, test_opt_dma_latency_cb); tcase_add_exit_test(tc, test_opt_dma_latency_cb_min, EXIT_FAILURE); tcase_add_exit_test(tc, test_opt_dma_latency_cb_max, EXIT_FAILURE); + tcase_add_test(tc, test_opt_dma_latency_cb_unset); tcase_add_test(tc, test_opt_aa_only_cb); + tcase_add_test(tc, test_opt_aa_only_cb_unset); tcase_add_test(tc, test_opt_timerlat_trace_output_cb); tcase_add_test(tc, test_opt_timerlat_trace_output_cb_noarg); + tcase_add_test(tc, test_opt_timerlat_trace_output_cb_unset); tcase_add_test(tc, test_opt_timerlat_on_threshold_cb); tcase_add_exit_test(tc, test_opt_timerlat_on_threshold_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_timerlat_on_end_cb); tcase_add_exit_test(tc, test_opt_timerlat_on_end_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_user_threads_cb); + tcase_add_test(tc, test_opt_user_threads_cb_unset); tcase_add_test(tc, test_opt_nano_cb); + tcase_add_test(tc, test_opt_nano_cb_unset); tcase_add_test(tc, test_opt_stack_format_cb); tcase_add_exit_test(tc, test_opt_stack_format_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_stack_format_cb_unset); tcase_add_test(tc, test_opt_timerlat_align_cb); + tcase_add_test(tc, test_opt_timerlat_align_cb_unset); suite_add_tcase(s, tc); tc = tcase_create("histogram"); tcase_add_test(tc, test_opt_bucket_size_cb); tcase_add_exit_test(tc, test_opt_bucket_size_min, EXIT_FAILURE); tcase_add_exit_test(tc, test_opt_bucket_size_max, EXIT_FAILURE); + tcase_add_test(tc, test_opt_bucket_size_cb_unset); tcase_add_test(tc, test_opt_entries_cb); tcase_add_exit_test(tc, test_opt_entries_min, EXIT_FAILURE); tcase_add_exit_test(tc, test_opt_entries_max, EXIT_FAILURE); + tcase_add_test(tc, test_opt_entries_cb_unset); suite_add_tcase(s, tc); return s; -- cgit From 05f4bcb2e2d9859de5d6bd2f35f909de9f8ee12f Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 29 Jun 2026 10:36:53 +0200 Subject: rtla: Add unit tests for CLI with unset Test parsing of command line that sets an option and then unsets it back to the default value in all tools. Only two CLI tests are added for each tool: short period option (-p ... --no-period) and long period option (--period ... --no-period). The logic specific for individual options is tested in opt callback tests already. Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20260629083654.1548925-3-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/unit/osnoise_hist_cli.c | 18 ++++++++++++++++++ tools/tracing/rtla/tests/unit/osnoise_top_cli.c | 18 ++++++++++++++++++ tools/tracing/rtla/tests/unit/timerlat_hist_cli.c | 18 ++++++++++++++++++ tools/tracing/rtla/tests/unit/timerlat_top_cli.c | 18 ++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/tools/tracing/rtla/tests/unit/osnoise_hist_cli.c b/tools/tracing/rtla/tests/unit/osnoise_hist_cli.c index 3661529f93dc..221985e6759f 100644 --- a/tools/tracing/rtla/tests/unit/osnoise_hist_cli.c +++ b/tools/tracing/rtla/tests/unit/osnoise_hist_cli.c @@ -37,6 +37,22 @@ START_TEST(test_period_long) } END_TEST +START_TEST(test_period_unset_short) +{ + PARSE_ARGS("osnoise", "hist", "-p", "100000", "--no-period"); + + ck_assert_int_eq(osn_params->period, 0); +} +END_TEST + +START_TEST(test_period_unset_long) +{ + PARSE_ARGS("osnoise", "hist", "--period", "100000", "--no-period"); + + ck_assert_int_eq(osn_params->period, 0); +} +END_TEST + START_TEST(test_runtime_short) { PARSE_ARGS("osnoise", "hist", "-r", "95000"); @@ -481,6 +497,8 @@ Suite *osnoise_hist_cli_suite(void) tc = tcase_create("tracing_options"); tcase_add_test(tc, test_period_short); tcase_add_test(tc, test_period_long); + tcase_add_test(tc, test_period_unset_short); + tcase_add_test(tc, test_period_unset_long); tcase_add_test(tc, test_runtime_short); tcase_add_test(tc, test_runtime_long); tcase_add_test(tc, test_stop_short); diff --git a/tools/tracing/rtla/tests/unit/osnoise_top_cli.c b/tools/tracing/rtla/tests/unit/osnoise_top_cli.c index f3a8633cc84e..057dbe574b07 100644 --- a/tools/tracing/rtla/tests/unit/osnoise_top_cli.c +++ b/tools/tracing/rtla/tests/unit/osnoise_top_cli.c @@ -37,6 +37,22 @@ START_TEST(test_period_long) } END_TEST +START_TEST(test_period_unset_short) +{ + PARSE_ARGS("osnoise", "top", "-p", "100000", "--no-period"); + + ck_assert_int_eq(osn_params->period, 0); +} +END_TEST + +START_TEST(test_period_unset_long) +{ + PARSE_ARGS("osnoise", "top", "--period", "100000", "--no-period"); + + ck_assert_int_eq(osn_params->period, 0); +} +END_TEST + START_TEST(test_runtime_short) { PARSE_ARGS("osnoise", "top", "-r", "95000"); @@ -433,6 +449,8 @@ Suite *osnoise_top_cli_suite(void) tc = tcase_create("tracing_options"); tcase_add_test(tc, test_period_short); tcase_add_test(tc, test_period_long); + tcase_add_test(tc, test_period_unset_short); + tcase_add_test(tc, test_period_unset_long); tcase_add_test(tc, test_runtime_short); tcase_add_test(tc, test_runtime_long); tcase_add_test(tc, test_stop_short); diff --git a/tools/tracing/rtla/tests/unit/timerlat_hist_cli.c b/tools/tracing/rtla/tests/unit/timerlat_hist_cli.c index 968bf962f53f..d8dd9d752636 100644 --- a/tools/tracing/rtla/tests/unit/timerlat_hist_cli.c +++ b/tools/tracing/rtla/tests/unit/timerlat_hist_cli.c @@ -55,6 +55,22 @@ START_TEST(test_period_long) } END_TEST +START_TEST(test_period_unset_short) +{ + PARSE_ARGS("timerlat", "hist", "-p", "200", "--no-period"); + + ck_assert_int_eq(tlat_params->timerlat_period_us, 0); +} +END_TEST + +START_TEST(test_period_unset_long) +{ + PARSE_ARGS("timerlat", "hist", "--period", "200", "--no-period"); + + ck_assert_int_eq(tlat_params->timerlat_period_us, 0); +} +END_TEST + START_TEST(test_stack_short) { PARSE_ARGS("timerlat", "hist", "-s", "20"); @@ -629,6 +645,8 @@ Suite *timerlat_hist_cli_suite(void) tcase_add_test(tc, test_irq_long); tcase_add_test(tc, test_period_short); tcase_add_test(tc, test_period_long); + tcase_add_test(tc, test_period_unset_short); + tcase_add_test(tc, test_period_unset_long); tcase_add_test(tc, test_stack_short); tcase_add_test(tc, test_stack_long); tcase_add_test(tc, test_thread_short); diff --git a/tools/tracing/rtla/tests/unit/timerlat_top_cli.c b/tools/tracing/rtla/tests/unit/timerlat_top_cli.c index 33aa6588d503..e9fb1a86ab8c 100644 --- a/tools/tracing/rtla/tests/unit/timerlat_top_cli.c +++ b/tools/tracing/rtla/tests/unit/timerlat_top_cli.c @@ -55,6 +55,22 @@ START_TEST(test_period_long) } END_TEST +START_TEST(test_period_unset_short) +{ + PARSE_ARGS("timerlat", "top", "-p", "200", "--no-period"); + + ck_assert_int_eq(tlat_params->timerlat_period_us, 0); +} +END_TEST + +START_TEST(test_period_unset_long) +{ + PARSE_ARGS("timerlat", "top", "--period", "200", "--no-period"); + + ck_assert_int_eq(tlat_params->timerlat_period_us, 0); +} +END_TEST + START_TEST(test_stack_short) { PARSE_ARGS("timerlat", "top", "-s", "20"); @@ -571,6 +587,8 @@ Suite *timerlat_top_cli_suite(void) tcase_add_test(tc, test_irq_long); tcase_add_test(tc, test_period_short); tcase_add_test(tc, test_period_long); + tcase_add_test(tc, test_period_unset_short); + tcase_add_test(tc, test_period_unset_long); tcase_add_test(tc, test_stack_short); tcase_add_test(tc, test_stack_long); tcase_add_test(tc, test_thread_short); -- cgit From c1bb78002a768da7dac246432edda3a36c541b12 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 29 Jun 2026 10:36:54 +0200 Subject: Documentation/rtla: Document unsetting options Add an appendix documenting how to unset options in RTLA. For options where unsetting is currently not supported, add a note into the respective section. An additional note is added for --on-threshold trace. As it is considered distinct from --trace, it is not reverted by --no-trace. Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20260629083654.1548925-4-tglozar@redhat.com Signed-off-by: Tomas Glozar --- Documentation/tools/rtla/common_appendix.txt | 17 +++++++++++++++++ Documentation/tools/rtla/common_options.txt | 13 ++++++++++++- Documentation/tools/rtla/common_osnoise_options.txt | 4 ++++ Documentation/tools/rtla/common_timerlat_options.txt | 4 ++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/Documentation/tools/rtla/common_appendix.txt b/Documentation/tools/rtla/common_appendix.txt index 68cb15840d3a..ad610ed02a24 100644 --- a/Documentation/tools/rtla/common_appendix.txt +++ b/Documentation/tools/rtla/common_appendix.txt @@ -1,5 +1,22 @@ .. SPDX-License-Identifier: GPL-2.0 +UNSETTING OPTIONS +================= + +The effect of most command line options can be reverted by prepending "no-" to +the long variant of the option, for example: + +$ rtla timerlat top -p 100 --no-period + +resets the period back to the default value of 1000 us. + +If a command line option sets multiple RTLA parameters at once, the inverted +option will revert all of them, even if they were not set by the particular +option. For example, since using "--auto" implies "--trace", specifying +"--trace --no-auto" will also disable trace output, just like if "--no-trace" +was specified. + + SIGINT BEHAVIOR =============== diff --git a/Documentation/tools/rtla/common_options.txt b/Documentation/tools/rtla/common_options.txt index 6caa51d02934..38da1cf443a4 100644 --- a/Documentation/tools/rtla/common_options.txt +++ b/Documentation/tools/rtla/common_options.txt @@ -22,10 +22,14 @@ Enable an event in the trace (**-t**) session. The argument can be a specific event, e.g., **-e** *sched:sched_switch*, or all events of a system group, e.g., **-e** *sched*. Multiple **-e** are allowed. It is only active when **-t** or **-a** are set. + This option cannot be unset. + **--filter** ** Filter the previous **-e** *sys:event* event with **. For further information about event filtering see https://www.kernel.org/doc/html/latest/trace/events.html#event-filtering. + This option cannot be unset. + **--trigger** ** Enable a trace event trigger to the previous **-e** *sys:event*. If the *hist:* trigger is activated, the output histogram will be automatically saved to a file named *system_event_hist.txt*. @@ -37,6 +41,8 @@ For further information about event trigger see https://www.kernel.org/doc/html/latest/trace/events.html#event-triggers. + This option cannot be unset. + **-P**, **--priority** *o:prio|r:prio|f:prio|d:runtime:period* Set scheduling parameters to the |tool| tracer threads, the format to set the priority are: @@ -78,7 +84,8 @@ Saves trace output, optionally taking a filename. Alternative to -t/--trace. Note that unlike -t/--trace, specifying this multiple times will result in - the trace being saved multiple times. + the trace being saved multiple times, and --no-trace will not disable trace + output when enabled through this option. - *signal,num=,pid=* @@ -107,6 +114,8 @@ |actionsperf| + This option cannot be unset. + **--on-end** *action* Defines an action to be executed at the end of tracing. @@ -124,6 +133,8 @@ This runs rtla with the default options, and saves trace output at the end. + This option cannot be unset. + **-h**, **--help** Print help menu. diff --git a/Documentation/tools/rtla/common_osnoise_options.txt b/Documentation/tools/rtla/common_osnoise_options.txt index bd3c4f499193..5fc70c001615 100644 --- a/Documentation/tools/rtla/common_osnoise_options.txt +++ b/Documentation/tools/rtla/common_osnoise_options.txt @@ -24,11 +24,15 @@ Stop the trace if a single sample is higher than the argument in microseconds. If **-T** is set, it will also save the trace to the output. + This option cannot be unset. + **-S**, **--stop-total** *us* Stop the trace if the total sample is higher than the argument in microseconds. If **-T** is set, it will also save the trace to the output. + This option cannot be unset. + **-T**, **--threshold** *us* Specify the minimum delta between two time reads to be considered noise. diff --git a/Documentation/tools/rtla/common_timerlat_options.txt b/Documentation/tools/rtla/common_timerlat_options.txt index 100840f4c0ed..e36898438a0b 100644 --- a/Documentation/tools/rtla/common_timerlat_options.txt +++ b/Documentation/tools/rtla/common_timerlat_options.txt @@ -23,10 +23,14 @@ Stop trace if the *IRQ* latency is higher than the argument in us. + This option cannot be unset. + **-T**, **--thread** *us* Stop trace if the *Thread* latency is higher than the argument in us. + This option cannot be unset. + **-s**, **--stack** *us* Save the stack trace at the *IRQ* if a *Thread* latency is higher than the -- cgit From ab43bd72f958b69045320bee8dd86fbb0eb74969 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 9 Jul 2026 11:17:55 +0200 Subject: rtla/tests: Test all tracer options in runtime tests Currently, runtime tests only test the osnoise period option (-p/--period of rtla-osnoise tools, backed by /sys/kernel/tracing/osnoise/period_us), using the check_with_osnoise_options function together with a hack relying on long period (pre-set) timing out if RTLA fails to reset it to the default value. Extend tracer option testing to all options used by RTLA; test both RTLA setting the default option by pre-setting the tracer to a different value and user-requested value. The tests are done using a script that reads the tracer values inside an --on-threshold action, like existing tests for runtime behavior already do. check_with_osnoise_option is modified to support grep filters, so that it can be used together with the script pattern. Assisted-by: Claude:claude-opus-4-6 Link: https://lore.kernel.org/r/20260709091755.58265-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/tests/engine.sh | 41 +++++++++++++--------- tools/tracing/rtla/tests/hwnoise.t | 3 ++ tools/tracing/rtla/tests/osnoise.t | 37 ++++++++++++++++--- .../rtla/tests/scripts/check-osnoise-option.sh | 16 +++++++++ .../rtla/tests/scripts/check-tracefs-value.sh | 11 ++++++ tools/tracing/rtla/tests/timerlat.t | 38 ++++++++++++++++++++ 6 files changed, 125 insertions(+), 21 deletions(-) create mode 100755 tools/tracing/rtla/tests/scripts/check-osnoise-option.sh create mode 100755 tools/tracing/rtla/tests/scripts/check-tracefs-value.sh diff --git a/tools/tracing/rtla/tests/engine.sh b/tools/tracing/rtla/tests/engine.sh index 5bf8453d354d..4287cd64ac31 100644 --- a/tools/tracing/rtla/tests/engine.sh +++ b/tools/tracing/rtla/tests/engine.sh @@ -98,30 +98,37 @@ check() { } check_with_osnoise_options() { - # Do the same as "check", but with pre-set osnoise options. - # Note: rtla should reset the osnoise options, this is used to test - # if it indeed does so. - # Save original arguments - arg1=$1 - arg2=$2 - arg3=$3 - - # Apply osnoise options (if not dry run) + # Do the same as "check", but with pre-set tracefs options. + # Resets osnoise first, then writes the given tracefs option=value + # pairs before running the check with NO_RESET_OSNOISE=1. + # Arguments: test_name command exit_code expected_output [path=value ...] + # Each path is relative to /sys/kernel/tracing/ + local arg1=$1 + local arg2=$2 + local arg3=$3 + local arg4=$4 + local opt option value + + # Apply tracefs options (if not dry run) if [ -n "$TEST_COUNT" ] then [ "$NO_RESET_OSNOISE" == 1 ] || reset_osnoise - shift - shift - while shift + shift 4 + for opt in "$@" do - [ "$1" == "" ] && continue - option=$(echo $1 | cut -d '=' -f 1) - value=$(echo $1 | cut -d '=' -f 2) - echo "$value" > "/sys/kernel/tracing/osnoise/$option" || return 1 + [ -z "$opt" ] && continue + option="${opt%%=*}" + value="${opt#*=}" + # Try to apply the option, ignore errors: when pre-setting fails + # (e.g. kernel does not know the option), the test itself will likely + # also fail. + # Throwing an error here would cause the test to be incorrectly + # skipped. + echo "$value" > "/sys/kernel/tracing/$option" done fi - NO_RESET_OSNOISE=1 check "$arg1" "$arg2" "$arg3" + NO_RESET_OSNOISE=1 check "$arg1" "$arg2" "$arg3" "$arg4" } check_top_hist() { diff --git a/tools/tracing/rtla/tests/hwnoise.t b/tools/tracing/rtla/tests/hwnoise.t index cfe687ff5ee1..b53d8d440fc5 100644 --- a/tools/tracing/rtla/tests/hwnoise.t +++ b/tools/tracing/rtla/tests/hwnoise.t @@ -18,5 +18,8 @@ check "stop the trace if a single sample is higher than 1 us" \ check "enable a trace event trigger" \ "hwnoise -t -e osnoise:irq_noise --trigger=\"hist:key=desc,duration:sort=desc,duration:vals=hitcount\" -d 10s" \ 0 "Saving event osnoise:irq_noise hist to osnoise_irq_noise_hist.txt" +check "verify OSNOISE_IRQ_DISABLE" \ + "hwnoise -S 1 --on-threshold shell,command=\"$testdir/scripts/check-osnoise-option.sh OSNOISE_IRQ_DISABLE\"" \ + 2 "^OSNOISE_IRQ_DISABLE=enabled$" test_end diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t index 346a14a860c8..f773156e758f 100644 --- a/tools/tracing/rtla/tests/osnoise.t +++ b/tools/tracing/rtla/tests/osnoise.t @@ -42,11 +42,40 @@ check "hist with --no-index" \ check "hist with --no-summary" \ "osnoise hist --no-summary -d 1s" 0 "" "^count:" -# Test setting default period by putting an absurdly high period -# and stopping on threshold. -# If default period is not set, this will time out. +# Tracer option tests - verify that rtla correctly sets tracefs options +# Default tests: poison tracefs with wrong values, verify rtla resets to defaults check_with_osnoise_options "apply default period" \ - "osnoise hist -s 1" 2 period_us=600000000 + "osnoise top -q -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/period_us\"" \ + 2 "^osnoise/period_us=1000000$" osnoise/period_us=600000000 +check_with_osnoise_options "apply default runtime" \ + "osnoise top -q -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/runtime_us\"" \ + 2 "^osnoise/runtime_us=1000000$" osnoise/runtime_us=100 +check_with_osnoise_options "apply default tracing_thresh" \ + "osnoise top -q -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh tracing_thresh\"" \ + 2 "^tracing_thresh=0$" tracing_thresh=999999 +check_with_osnoise_options "apply default stop_tracing_us" \ + "osnoise top -q -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_us\"" \ + 2 "^osnoise/stop_tracing_us=0$" osnoise/stop_tracing_us=999999 +check_with_osnoise_options "apply default stop_tracing_total_us" \ + "osnoise top -q -s 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_total_us\"" \ + 2 "^osnoise/stop_tracing_total_us=0$" osnoise/stop_tracing_total_us=999999 + +# Non-default tracer option tests: verify CLI options correctly set tracefs values +check_top_q_hist "verify -p sets period_us" \ + "osnoise TOOL -p 2000000 -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/period_us\"" \ + 2 "^osnoise/period_us=2000000$" +check_top_q_hist "verify -r sets runtime_us" \ + "osnoise TOOL -r 500000 -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/runtime_us\"" \ + 2 "^osnoise/runtime_us=500000$" +check_top_q_hist "verify -T sets tracing_thresh" \ + "osnoise TOOL -T 5 -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh tracing_thresh\"" \ + 2 "^tracing_thresh=5$" +check_top_q_hist "verify -s sets stop_tracing_us" \ + "osnoise TOOL -s 30 -S 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_us\"" \ + 2 "^osnoise/stop_tracing_us=30$" +check_top_q_hist "verify -S sets stop_tracing_total_us" \ + "osnoise TOOL -S 100 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_total_us\"" \ + 2 "^osnoise/stop_tracing_total_us=100$" # Actions tests check_top_q_hist "trace output through -t with custom filename" \ diff --git a/tools/tracing/rtla/tests/scripts/check-osnoise-option.sh b/tools/tracing/rtla/tests/scripts/check-osnoise-option.sh new file mode 100755 index 000000000000..37c62268cc15 --- /dev/null +++ b/tools/tracing/rtla/tests/scripts/check-osnoise-option.sh @@ -0,0 +1,16 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Check if osnoise options are enabled or disabled. +# Usage: check-osnoise-option.sh [ ...] +# Output: one line per option in the format "OPTION=enabled" or "OPTION=disabled" + +options=$(tr ' ' '\n' < /sys/kernel/tracing/osnoise/options) +for name in "$@"; do + if echo "$options" | grep -q "^NO_${name}$"; then + echo "$name=disabled" + elif echo "$options" | grep -q "^${name}$"; then + echo "$name=enabled" + else + echo "$name=unsupported" + fi +done diff --git a/tools/tracing/rtla/tests/scripts/check-tracefs-value.sh b/tools/tracing/rtla/tests/scripts/check-tracefs-value.sh new file mode 100755 index 000000000000..12d0eacd6a85 --- /dev/null +++ b/tools/tracing/rtla/tests/scripts/check-tracefs-value.sh @@ -0,0 +1,11 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Read tracefs values and print them. +# Usage: check-tracefs-value.sh [ ...] +# Each path is relative to /sys/kernel/tracing/ +# Output: one line per file in the format "path=value" + +for file in "$@"; do + read value < "/sys/kernel/tracing/$file" + echo "$file=$value" +done diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t index 8193048e8c8c..aa5b91d88ca4 100644 --- a/tools/tracing/rtla/tests/timerlat.t +++ b/tools/tracing/rtla/tests/timerlat.t @@ -55,6 +55,44 @@ check_top_q_hist "verify -k/--kernel-threads" \ check_top_q_hist "verify -u/--user-threads" \ "timerlat TOOL -u -c 0 -d 10s -T 1 --on-threshold shell,command=$testdir/scripts/check-user-kernel-threads.sh" 2 "0 kernel threads, 1 user threads" +# Tracer option tests - verify that rtla correctly sets tracefs options +# Default tests: poison tracefs with wrong values, verify rtla resets to defaults +check_with_osnoise_options "apply default timerlat_period_us" \ + "timerlat top -q -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/timerlat_period_us\"" \ + 2 "^osnoise/timerlat_period_us=1000$" osnoise/timerlat_period_us=999999 +check_with_osnoise_options "apply default print_stack" \ + "timerlat top -q -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/print_stack\"" \ + 2 "^osnoise/print_stack=0$" osnoise/print_stack=999999 +check_with_osnoise_options "apply default stop_tracing_us" \ + "timerlat top -q -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_us\"" \ + 2 "^osnoise/stop_tracing_us=0$" osnoise/stop_tracing_us=999999 +check_with_osnoise_options "apply default stop_tracing_total_us" \ + "timerlat top -q -i 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_total_us\"" \ + 2 "^osnoise/stop_tracing_total_us=0$" osnoise/stop_tracing_total_us=999999 + +# Non-default tracer option tests: verify CLI options correctly set tracefs values +check_top_q_hist "verify -p sets timerlat_period_us" \ + "timerlat TOOL -p 2000 -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/timerlat_period_us\"" \ + 2 "^osnoise/timerlat_period_us=2000$" +check_top_q_hist "verify -s sets print_stack" \ + "timerlat TOOL -s 5 -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/print_stack\"" \ + 2 "^osnoise/print_stack=5$" +check_top_q_hist "verify -i sets stop_tracing_us" \ + "timerlat TOOL -i 2 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_us\"" \ + 2 "^osnoise/stop_tracing_us=2$" +check_top_q_hist "verify -T sets stop_tracing_total_us" \ + "timerlat TOOL -T 2 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/stop_tracing_total_us\"" \ + 2 "^osnoise/stop_tracing_total_us=2$" +check_with_osnoise_options "apply default TIMERLAT_ALIGN" \ + "timerlat top -q -T 1 --on-threshold shell,command=\"$testdir/scripts/check-osnoise-option.sh TIMERLAT_ALIGN\"" \ + 2 "^TIMERLAT_ALIGN=disabled$" osnoise/options=TIMERLAT_ALIGN +check_top_q_hist "verify -A sets TIMERLAT_ALIGN" \ + "timerlat TOOL -A 100 -T 1 --on-threshold shell,command=\"$testdir/scripts/check-osnoise-option.sh TIMERLAT_ALIGN\"" \ + 2 "^TIMERLAT_ALIGN=enabled$" +check_top_q_hist "verify -A sets timerlat_align_us" \ + "timerlat TOOL -A 100 -T 1 --on-threshold shell,command=\"$testdir/scripts/check-tracefs-value.sh osnoise/timerlat_align_us\"" \ + 2 "^osnoise/timerlat_align_us=100$" + # Histogram tests check "hist with -b/--bucket-size" \ "timerlat hist -b 1 -d 1s" -- cgit From 92a33d5bad7485b39c7403ad1741b3b2b35a0131 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Fri, 10 Jul 2026 15:15:54 +0200 Subject: rtla/cli: Unify and improve range validation logic Several RTLA options do range validation inside the CLI parser layer (e.g. -p/--period). When RTLA migrated CLI parsing to libsubcmd, this logic was moved unchanged inside opt_*() callbacks. Unify range validation so that all options use two newly added functions, check_llong_range() and check_int_range(), to validate the range. The new range validation returns -1 from opt_*() callbacks rather than hard-exit with fatal(), allowing the help message for the specific option to be automatically displayed by libsubcmd logic. Many options no longer need a custom callback, as they use the unified range validation of opt_llong_callback() and opt_int_callback(). Validation for several other options is improved: - timerlat -p/--period: lower bound raised from 1 to 100 us to match the kernel's timerlat_min_period in trace_osnoise.c. - timerlat -A/--aligned: reject negative values. - timerlat --deepest-idle-state: add range [-1, INT_MAX]; previously, values <= -2 were read as "option not set". - timerlat -p/--period, -A/--aligned, -b/--bucket-size: properly reject negative values instead of passing them to the tracer. Remove unit tests for removed callbacks and test the new range validation functionality of opt_llong_callback() and opt_int_callback(). Update runtime tests for histogram options to account for the new error messages and exit value. Assisted-by: Claude:claude-opus-4-6 Link: https://lore.kernel.org/r/20260710131554.338335-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/cli_p.h | 234 +++++++-------- tools/tracing/rtla/tests/osnoise.t | 2 +- tools/tracing/rtla/tests/timerlat.t | 2 +- tools/tracing/rtla/tests/unit/cli_opt_callback.c | 353 ++++++++--------------- 4 files changed, 215 insertions(+), 376 deletions(-) diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h index 3a93dba60215..b7688559ca53 100644 --- a/tools/tracing/rtla/src/cli_p.h +++ b/tools/tracing/rtla/src/cli_p.h @@ -5,6 +5,7 @@ #error "Private header file included outside of cli.c module" #endif +#include #include #include @@ -32,9 +33,73 @@ static const int default_bucket_size = 1; static const int default_entries = 256; static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE; +/* + * Range checking for long long and int option callbacks. + * + * Pass a pointer to a const struct as opt->data to enable range checking. + * If opt->data is NULL, no range check is performed. + */ +struct llong_range { + long long min; + long long max; +}; + +struct int_range { + int min; + int max; +}; + +#define LLONG_RANGE(lo, hi) \ + (&(const struct llong_range){ .min = (lo), .max = (hi) }) + +#define INT_RANGE(lo, hi) \ + (&(const struct int_range){ .min = (lo), .max = (hi) }) + +static int check_llong_range(const struct option *opt, long long value) +{ + const struct llong_range *range = opt->data; + + if (!range) + return 0; + if (value < range->min || value > range->max) { + fprintf(stderr, " Error: --%s value %lld is out of range [%lld, %lld]\n", + opt->long_name, value, range->min, range->max); + return -1; + } + return 0; +} + +static int check_int_range(const struct option *opt, int value) +{ + const struct int_range *range = opt->data; + + if (!range) + return 0; + if (value < range->min || value > range->max) { + fprintf(stderr, " Error: --%s value %d is out of range [%d, %d]\n", + opt->long_name, value, range->min, range->max); + return -1; + } + return 0; +} + +/* + * OPT_CALLBACK variant that populates .data (for range checking). + */ +#define RTLA_OPT_CALLBACK_DATA(s, l, v, a, h, f, d) \ + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \ + .value = (v), .argh = (a), .help = (h), .callback = (f), \ + .data = (void *)(d) } + +#define RTLA_OPT_CALLBACK_DATA_DEFVAL(s, l, v, a, h, f, d, dv) \ + { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \ + .value = (v), .argh = (a), .help = (h), .callback = (f), \ + .data = (void *)(d), .defval = (intptr_t)(dv) } + /* * Shorthand macros for integer/long long command line options using - * opt_int_callback/opt_llong_callback, with variants that set defval. + * opt_int_callback/opt_llong_callback, with variants that set defval + * and/or data (for range checking). * * Note: defval's type is intptr_t. opt_int_callback interprets it directly as * an int, opt_llong_callback interprets it as a pointer to a long long, as @@ -47,6 +112,10 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE; .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ .help = (h), .callback = opt_llong_callback, .defval = (intptr_t)(d) } +#define RTLA_OPT_LLONG_DATA(s, l, v, a, h, d) { .type = OPTION_CALLBACK, \ + .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ + .help = (h), .callback = opt_llong_callback, .data = (void *)(d) } + #define RTLA_OPT_INT(s, l, v, a, h) \ OPT_CALLBACK(s, l, v, a, h, opt_int_callback) @@ -54,6 +123,11 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE; .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ .help = (h), .callback = opt_int_callback, .defval = (intptr_t)(d) } +#define RTLA_OPT_INT_DATA_DEFVAL(s, l, v, a, h, d, dv) { .type = OPTION_CALLBACK, \ + .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \ + .help = (h), .callback = opt_int_callback, \ + .data = (void *)(d), .defval = (intptr_t)(dv) } + /* * Macros for command line options common to all tools * @@ -182,6 +256,8 @@ static int opt_llong_callback(const struct option *opt, const char *arg, int uns return -1; *value = get_llong_from_str((char *)arg); + if (check_llong_range(opt, *value)) + return -1; return 0; } @@ -199,6 +275,8 @@ static int opt_int_callback(const struct option *opt, const char *arg, int unset if (strtoi(arg, value)) return -1; + if (check_int_range(opt, *value)) + return -1; return 0; } @@ -359,13 +437,13 @@ static int opt_filter_cb(const struct option *opt, const char *arg, int unset) /* * Macros for command line options specific to osnoise */ -#define OSNOISE_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->period, "us", \ +#define OSNOISE_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->period, "us", \ "osnoise period in us", \ - opt_osnoise_period_cb) + LLONG_RANGE(1, 10000000)) -#define OSNOISE_OPT_RUNTIME OPT_CALLBACK('r', "runtime", ¶ms->runtime, "us", \ +#define OSNOISE_OPT_RUNTIME RTLA_OPT_LLONG_DATA('r', "runtime", ¶ms->runtime, "us", \ "osnoise runtime in us", \ - opt_osnoise_runtime_cb) + LLONG_RANGE(100, LLONG_MAX)) #define OSNOISE_OPT_THRESHOLD RTLA_OPT_LLONG('T', "threshold", ¶ms->threshold, "us", \ "the minimum delta to be considered a noise") @@ -400,44 +478,6 @@ static int opt_osnoise_auto_cb(const struct option *opt, const char *arg, int un return 0; } -static int opt_osnoise_period_cb(const struct option *opt, const char *arg, int unset) -{ - unsigned long long *period = opt->value; - - if (unset) { - *period = 0; - return 0; - } - - if (!arg) - return -1; - - *period = get_llong_from_str((char *)arg); - if (*period > 10000000) - fatal("Period longer than 10 s"); - - return 0; -} - -static int opt_osnoise_runtime_cb(const struct option *opt, const char *arg, int unset) -{ - unsigned long long *runtime = opt->value; - - if (unset) { - *runtime = 0; - return 0; - } - - if (!arg) - return -1; - - *runtime = get_llong_from_str((char *)arg); - if (*runtime < 100) - fatal("Runtime shorter than 100 us"); - - return 0; -} - static int opt_osnoise_trace_output_cb(const struct option *opt, const char *arg, int unset) { const char **trace_output = opt->value; @@ -492,9 +532,9 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int /* * Macros for command line options specific to timerlat */ -#define TIMERLAT_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->timerlat_period_us, "us", \ +#define TIMERLAT_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->timerlat_period_us, "us", \ "timerlat period in us", \ - opt_timerlat_period_cb) + LLONG_RANGE(100, 1000000)) #define TIMERLAT_OPT_STACK RTLA_OPT_LLONG('s', "stack", ¶ms->print_stack, "us", \ "save the stack trace at the IRQ if a thread latency is higher than the argument in us") @@ -503,14 +543,15 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int "display data in nanoseconds", \ opt_nano_cb) -#define TIMERLAT_OPT_DMA_LATENCY OPT_CALLBACK(0, "dma-latency", ¶ms->dma_latency, "us", \ +#define TIMERLAT_OPT_DMA_LATENCY RTLA_OPT_INT_DATA_DEFVAL(0, "dma-latency", \ + ¶ms->dma_latency, "us", \ "set /dev/cpu_dma_latency latency to reduce exit from idle latency", \ - opt_dma_latency_cb) + INT_RANGE(0, 10000), default_dma_latency) -#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DEFVAL(0, "deepest-idle-state", \ +#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DATA_DEFVAL(0, "deepest-idle-state", \ ¶ms->deepest_idle_state, "n", \ "only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", \ - default_deepest_idle_state) + INT_RANGE(-1, INT_MAX), default_deepest_idle_state) #define TIMERLAT_OPT_AA_ONLY OPT_CALLBACK(0, "aa-only", params, "us", \ "stop if latency is hit, only printing the auto analysis (reduces CPU usage)", \ @@ -530,33 +571,14 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int "set the stack format (truncate, skip, full)", \ opt_stack_format_cb) -#define TIMERLAT_OPT_ALIGNED OPT_CALLBACK('A', "aligned", params, "us", \ +#define TIMERLAT_OPT_ALIGNED RTLA_OPT_CALLBACK_DATA('A', "aligned", params, "us", \ "align thread wakeups to a specific offset", \ - opt_timerlat_align_cb) + opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX)) /* * Callback functions for command line options for timerlat tools */ -static int opt_timerlat_period_cb(const struct option *opt, const char *arg, int unset) -{ - long long *period = opt->value; - - if (unset) { - *period = 0; - return 0; - } - - if (!arg) - return -1; - - *period = get_llong_from_str((char *)arg); - if (*period > 1000000) - fatal("Period longer than 1 s"); - - return 0; -} - static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int unset) { struct timerlat_cb_data *cb_data = opt->value; @@ -585,28 +607,6 @@ static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int u return 0; } -static int opt_dma_latency_cb(const struct option *opt, const char *arg, int unset) -{ - int *dma_latency = opt->value; - int retval; - - if (unset) { - *dma_latency = default_dma_latency; - return 0; - } - - if (!arg) - return -1; - - retval = strtoi((char *)arg, dma_latency); - if (retval) - fatal("Invalid -dma-latency %s", arg); - if (*dma_latency < 0 || *dma_latency > 10000) - fatal("--dma-latency needs to be >= 0 and <= 10000"); - - return 0; -} - static int opt_aa_only_cb(const struct option *opt, const char *arg, int unset) { struct timerlat_params *params = opt->value; @@ -748,6 +748,8 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int params->timerlat_align = true; params->timerlat_align_us = get_llong_from_str((char *)arg); + if (check_llong_range(opt, params->timerlat_align_us)) + return -1; return 0; } @@ -756,14 +758,15 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int * Macros for command line options specific to histogram-based tools */ -#define HIST_OPT_BUCKET_SIZE OPT_CALLBACK('b', "bucket-size", \ +#define HIST_OPT_BUCKET_SIZE RTLA_OPT_INT_DATA_DEFVAL('b', "bucket-size", \ ¶ms->common.hist.bucket_size, "N", \ "set the histogram bucket size (default 1)", \ - opt_bucket_size_cb) + INT_RANGE(1, 999999), default_bucket_size) -#define HIST_OPT_ENTRIES OPT_CALLBACK('E', "entries", ¶ms->common.hist.entries, "N", \ +#define HIST_OPT_ENTRIES RTLA_OPT_INT_DATA_DEFVAL('E', "entries", \ + ¶ms->common.hist.entries, "N", \ "set the number of entries of the histogram (default 256)", \ - opt_entries_cb) + INT_RANGE(10, 9999999), default_entries) #define HIST_OPT_NO_IRQ OPT_BOOLEAN_FLAG(0, "no-irq", ¶ms->common.hist.no_irq, \ "ignore IRQ latencies", PARSE_OPT_NOAUTONEG) @@ -783,42 +786,3 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int #define HIST_OPT_WITH_ZEROS OPT_BOOLEAN(0, "with-zeros", ¶ms->common.hist.with_zeros, \ "print zero only entries") -/* Histogram-specific callbacks */ - -static int opt_bucket_size_cb(const struct option *opt, const char *arg, int unset) -{ - int *bucket_size = opt->value; - - if (unset) { - *bucket_size = default_bucket_size; - return 0; - } - - if (!arg) - return -1; - - *bucket_size = get_llong_from_str((char *)arg); - if (*bucket_size == 0 || *bucket_size >= 1000000) - fatal("Bucket size needs to be > 0 and <= 1000000"); - - return 0; -} - -static int opt_entries_cb(const struct option *opt, const char *arg, int unset) -{ - int *entries = opt->value; - - if (unset) { - *entries = default_entries; - return 0; - } - - if (!arg) - return -1; - - *entries = get_llong_from_str((char *)arg); - if (*entries < 10 || *entries > 9999999) - fatal("Entries must be > 10 and < 10000000"); - - return 0; -} diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t index f773156e758f..214fd40fc610 100644 --- a/tools/tracing/rtla/tests/osnoise.t +++ b/tools/tracing/rtla/tests/osnoise.t @@ -32,7 +32,7 @@ check "hist with -b/--bucket-size" \ check "hist with -E/--entries" \ "osnoise hist -E 10 -d 1s" check "hist with -E/--entries out of range" \ - "osnoise hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$" + "osnoise hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]" check "hist with --no-header" \ "osnoise hist --no-header -d 1s" 0 "" "RTLA osnoise histogram" check "hist with --with-zeros" \ diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t index aa5b91d88ca4..5116857d7da4 100644 --- a/tools/tracing/rtla/tests/timerlat.t +++ b/tools/tracing/rtla/tests/timerlat.t @@ -99,7 +99,7 @@ check "hist with -b/--bucket-size" \ check "hist with -E/--entries" \ "timerlat hist -E 10 -d 1s" check "hist with -E/--entries out of range" \ - "timerlat hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$" + "timerlat hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]" check "hist with --no-header" \ "timerlat hist --no-header -d 1s" 0 "" "RTLA timerlat histogram" check "hist with --with-zeros" \ diff --git a/tools/tracing/rtla/tests/unit/cli_opt_callback.c b/tools/tracing/rtla/tests/unit/cli_opt_callback.c index 413a04f898fb..8439fb5c6f0b 100644 --- a/tools/tracing/rtla/tests/unit/cli_opt_callback.c +++ b/tools/tracing/rtla/tests/unit/cli_opt_callback.c @@ -9,6 +9,12 @@ #include "cli_params_assert.h" #define TEST_CALLBACK(value, cb) OPT_CALLBACK('t', "test", value, "test value", "test help", cb) +#define TEST_LLONG_RANGE(value, lo, hi) \ + RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \ + opt_llong_callback, LLONG_RANGE(lo, hi)) +#define TEST_INT_RANGE(value, lo, hi) \ + RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \ + opt_int_callback, INT_RANGE(lo, hi)) START_TEST(test_opt_llong_callback_simple) { @@ -137,6 +143,90 @@ START_TEST(test_opt_int_callback_unset_defval) } END_TEST +START_TEST(test_opt_llong_callback_range_in) +{ + long long test_value = 0; + const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100); + + ck_assert_int_eq(opt_llong_callback(&opt, "50", 0), 0); + ck_assert_int_eq(test_value, 50); +} +END_TEST + +START_TEST(test_opt_llong_callback_range_below) +{ + long long test_value = 0; + const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_llong_callback(&opt, "9", 0), -1); +} +END_TEST + +START_TEST(test_opt_llong_callback_range_above) +{ + long long test_value = 0; + const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_llong_callback(&opt, "101", 0), -1); +} +END_TEST + +START_TEST(test_opt_llong_callback_range_boundary) +{ + long long test_value = 0; + const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100); + + ck_assert_int_eq(opt_llong_callback(&opt, "10", 0), 0); + ck_assert_int_eq(test_value, 10); + ck_assert_int_eq(opt_llong_callback(&opt, "100", 0), 0); + ck_assert_int_eq(test_value, 100); +} +END_TEST + +START_TEST(test_opt_int_callback_range_in) +{ + int test_value = 0; + const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000); + + ck_assert_int_eq(opt_int_callback(&opt, "5000", 0), 0); + ck_assert_int_eq(test_value, 5000); +} +END_TEST + +START_TEST(test_opt_int_callback_range_below) +{ + int test_value = 0; + const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_int_callback(&opt, "-1", 0), -1); +} +END_TEST + +START_TEST(test_opt_int_callback_range_above) +{ + int test_value = 0; + const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_int_callback(&opt, "10001", 0), -1); +} +END_TEST + +START_TEST(test_opt_int_callback_range_boundary) +{ + int test_value = 0; + const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000); + + ck_assert_int_eq(opt_int_callback(&opt, "0", 0), 0); + ck_assert_int_eq(test_value, 0); + ck_assert_int_eq(opt_int_callback(&opt, "10000", 0), 0); + ck_assert_int_eq(test_value, 10000); +} +END_TEST + START_TEST(test_opt_cpus_cb) { struct common_params params = {0}; @@ -388,67 +478,6 @@ START_TEST(test_opt_osnoise_auto_cb_unset) } END_TEST -START_TEST(test_opt_osnoise_period_cb) -{ - unsigned long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb); - - ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0); - ck_assert_int_eq(period, 1000000); -} -END_TEST - -START_TEST(test_opt_osnoise_period_cb_invalid) -{ - unsigned long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_osnoise_period_cb(&opt, "10000001", 0); -} -END_TEST - -START_TEST(test_opt_osnoise_period_cb_unset) -{ - unsigned long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb); - - ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0); - ck_assert_int_eq(opt_osnoise_period_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(period, 0); -} -END_TEST - -START_TEST(test_opt_osnoise_runtime_cb) -{ - unsigned long long runtime = 0; - const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb); - - ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0); - ck_assert_int_eq(runtime, 900000); -} -END_TEST - -START_TEST(test_opt_osnoise_runtime_cb_invalid) -{ - unsigned long long runtime = 0; - const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_osnoise_runtime_cb(&opt, "99", 0); -} -END_TEST - -START_TEST(test_opt_osnoise_runtime_cb_unset) -{ - unsigned long long runtime = 0; - const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb); - - ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0); - ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(runtime, 0); -} -END_TEST START_TEST(test_opt_osnoise_trace_output_cb) { @@ -525,37 +554,6 @@ START_TEST(test_opt_osnoise_on_end_cb_invalid) } END_TEST -START_TEST(test_opt_timerlat_period_cb) -{ - long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb); - - ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0); - ck_assert_int_eq(period, 1000); -} -END_TEST - -START_TEST(test_opt_timerlat_period_cb_invalid) -{ - long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_timerlat_period_cb(&opt, "1000001", 0); -} -END_TEST - -START_TEST(test_opt_timerlat_period_cb_unset) -{ - long long period = 0; - const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb); - - ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0); - ck_assert_int_eq(opt_timerlat_period_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(period, 0); -} -END_TEST - START_TEST(test_opt_timerlat_auto_cb) { struct timerlat_params params = {0}; @@ -585,46 +583,6 @@ START_TEST(test_opt_timerlat_auto_cb_unset) } END_TEST -START_TEST(test_opt_dma_latency_cb) -{ - int dma_latency = 0; - const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb); - - ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0); - ck_assert_int_eq(dma_latency, 1000); -} -END_TEST - -START_TEST(test_opt_dma_latency_cb_min) -{ - int dma_latency = 0; - const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_dma_latency_cb(&opt, "-1", 0); -} -END_TEST - -START_TEST(test_opt_dma_latency_cb_max) -{ - int dma_latency = 0; - const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_dma_latency_cb(&opt, "10001", 0); -} -END_TEST - -START_TEST(test_opt_dma_latency_cb_unset) -{ - int dma_latency = 0; - const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb); - - ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0); - ck_assert_int_eq(opt_dma_latency_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(dma_latency, default_dma_latency); -} -END_TEST START_TEST(test_opt_aa_only_cb) { @@ -775,7 +733,8 @@ END_TEST START_TEST(test_opt_timerlat_align_cb) { struct timerlat_params params = {0}; - const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb); + const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us", + "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX)); ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0); ck_assert(params.timerlat_align); @@ -783,10 +742,22 @@ START_TEST(test_opt_timerlat_align_cb) } END_TEST +START_TEST(test_opt_timerlat_align_cb_invalid) +{ + struct timerlat_params params = {0}; + const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us", + "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX)); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_timerlat_align_cb(&opt, "-1", 0), -1); +} +END_TEST + START_TEST(test_opt_timerlat_align_cb_unset) { struct timerlat_params params = {0}; - const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb); + const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us", + "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX)); ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0); ck_assert_int_eq(opt_timerlat_align_cb(&opt, NULL, 1), 0); @@ -826,87 +797,6 @@ START_TEST(test_opt_stack_format_cb_unset) } END_TEST -START_TEST(test_opt_bucket_size_cb) -{ - int bucket_size = 0; - const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb); - - ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0); - ck_assert_int_eq(bucket_size, 100); -} -END_TEST - -START_TEST(test_opt_bucket_size_min) -{ - int bucket_size = 0; - const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_bucket_size_cb(&opt, "0", 0); -} -END_TEST - -START_TEST(test_opt_bucket_size_max) -{ - int bucket_size = 0; - const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_bucket_size_cb(&opt, "1000001", 0); -} -END_TEST - -START_TEST(test_opt_bucket_size_cb_unset) -{ - int bucket_size = 0; - const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb); - - ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0); - ck_assert_int_eq(opt_bucket_size_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(bucket_size, default_bucket_size); -} -END_TEST - -START_TEST(test_opt_entries_cb) -{ - int entries = 0; - const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb); - - ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0); - ck_assert_int_eq(entries, 100); -} -END_TEST - -START_TEST(test_opt_entries_min) -{ - int entries = 0; - const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_entries_cb(&opt, "9", 0); -} -END_TEST - -START_TEST(test_opt_entries_max) -{ - int entries = 0; - const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb); - - assert(freopen("/dev/null", "w", stderr)); - opt_entries_cb(&opt, "10000000", 0); -} -END_TEST - -START_TEST(test_opt_entries_cb_unset) -{ - int entries = 0; - const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb); - - ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0); - ck_assert_int_eq(opt_entries_cb(&opt, NULL, 1), 0); - ck_assert_int_eq(entries, default_entries); -} -END_TEST Suite *cli_opt_callback_suite(void) { @@ -919,6 +809,10 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_llong_callback_min); tcase_add_test(tc, test_opt_llong_callback_unset); tcase_add_test(tc, test_opt_llong_callback_unset_defval); + tcase_add_test(tc, test_opt_llong_callback_range_in); + tcase_add_test(tc, test_opt_llong_callback_range_below); + tcase_add_test(tc, test_opt_llong_callback_range_above); + tcase_add_test(tc, test_opt_llong_callback_range_boundary); tcase_add_test(tc, test_opt_int_callback_simple); tcase_add_test(tc, test_opt_int_callback_max); tcase_add_test(tc, test_opt_int_callback_min); @@ -926,6 +820,10 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_int_callback_non_numeric_suffix); tcase_add_test(tc, test_opt_int_callback_unset); tcase_add_test(tc, test_opt_int_callback_unset_defval); + tcase_add_test(tc, test_opt_int_callback_range_in); + tcase_add_test(tc, test_opt_int_callback_range_below); + tcase_add_test(tc, test_opt_int_callback_range_above); + tcase_add_test(tc, test_opt_int_callback_range_boundary); tcase_add_test(tc, test_opt_cpus_cb); tcase_add_exit_test(tc, test_opt_cpus_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_cgroup_cb); @@ -951,12 +849,6 @@ Suite *cli_opt_callback_suite(void) tc = tcase_create("osnoise"); tcase_add_test(tc, test_opt_osnoise_auto_cb); tcase_add_test(tc, test_opt_osnoise_auto_cb_unset); - tcase_add_test(tc, test_opt_osnoise_period_cb); - tcase_add_test(tc, test_opt_osnoise_period_cb_unset); - tcase_add_exit_test(tc, test_opt_osnoise_period_cb_invalid, EXIT_FAILURE); - tcase_add_test(tc, test_opt_osnoise_runtime_cb); - tcase_add_exit_test(tc, test_opt_osnoise_runtime_cb_invalid, EXIT_FAILURE); - tcase_add_test(tc, test_opt_osnoise_runtime_cb_unset); tcase_add_test(tc, test_opt_osnoise_trace_output_cb); tcase_add_test(tc, test_opt_osnoise_trace_output_cb_noarg); tcase_add_test(tc, test_opt_osnoise_trace_output_cb_unset); @@ -967,15 +859,8 @@ Suite *cli_opt_callback_suite(void) suite_add_tcase(s, tc); tc = tcase_create("timerlat"); - tcase_add_test(tc, test_opt_timerlat_period_cb); - tcase_add_exit_test(tc, test_opt_timerlat_period_cb_invalid, EXIT_FAILURE); - tcase_add_test(tc, test_opt_timerlat_period_cb_unset); tcase_add_test(tc, test_opt_timerlat_auto_cb); tcase_add_test(tc, test_opt_timerlat_auto_cb_unset); - tcase_add_test(tc, test_opt_dma_latency_cb); - tcase_add_exit_test(tc, test_opt_dma_latency_cb_min, EXIT_FAILURE); - tcase_add_exit_test(tc, test_opt_dma_latency_cb_max, EXIT_FAILURE); - tcase_add_test(tc, test_opt_dma_latency_cb_unset); tcase_add_test(tc, test_opt_aa_only_cb); tcase_add_test(tc, test_opt_aa_only_cb_unset); tcase_add_test(tc, test_opt_timerlat_trace_output_cb); @@ -993,19 +878,9 @@ Suite *cli_opt_callback_suite(void) tcase_add_exit_test(tc, test_opt_stack_format_cb_invalid, EXIT_FAILURE); tcase_add_test(tc, test_opt_stack_format_cb_unset); tcase_add_test(tc, test_opt_timerlat_align_cb); + tcase_add_test(tc, test_opt_timerlat_align_cb_invalid); tcase_add_test(tc, test_opt_timerlat_align_cb_unset); suite_add_tcase(s, tc); - tc = tcase_create("histogram"); - tcase_add_test(tc, test_opt_bucket_size_cb); - tcase_add_exit_test(tc, test_opt_bucket_size_min, EXIT_FAILURE); - tcase_add_exit_test(tc, test_opt_bucket_size_max, EXIT_FAILURE); - tcase_add_test(tc, test_opt_bucket_size_cb_unset); - tcase_add_test(tc, test_opt_entries_cb); - tcase_add_exit_test(tc, test_opt_entries_min, EXIT_FAILURE); - tcase_add_exit_test(tc, test_opt_entries_max, EXIT_FAILURE); - tcase_add_test(tc, test_opt_entries_cb_unset); - suite_add_tcase(s, tc); - return s; } -- cgit From 0ad45018ace734e3a21c8b15280df65b429135d1 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 16 Jul 2026 16:49:01 +0200 Subject: rtla/cli: Unify and improve handling of invalid option arguments The current handling of invalid command line option arguments is inconsistent: - opt_llong_callback() treats non-numerical input the same as "-1", which might or might not be rejected later. - opt_int_callback() returns -1 on non-numerical input without an error message, which makes parsing fail silently (libsubcmd will automatically print the usage of the option only, no error message). - custom callbacks abort command line parsing using fatal(), which displays an error message and exits, without libsubcmd printing the usage. Unify this such that all invalid options, regardless of the format, print an error message similar to the out of range case: Error: --opt: 'value' is not a valid XY followed by the usage of the option, e.g.: $ rtla timerlat hist --period=1us Error: --period: '1us' is not a valid number Usage: rtla timerlat hist [] [-h|--help] -p, --period timerlat period in us As this is a libsubcmd help path, all option parsing failures now return the exit code of 129 (help). The unified handling is implemented using a new error message helper, opt_err(), which is called from two new CLI-specific parsing functions, strtoll_safe() and strtoi_safe(), as well as from custom helpers. Option callback tests are updated to cover the new behavior. Assisted-by: Claude:claude-opus-4-6 Reviewed-by: Wander Lairson Costa Link: https://lore.kernel.org/r/20260716144901.1187474-1-tglozar@redhat.com Signed-off-by: Tomas Glozar --- tools/tracing/rtla/src/cli_p.h | 126 +++++++++++++++++------ tools/tracing/rtla/tests/unit/cli_opt_callback.c | 70 +++++++++---- 2 files changed, 144 insertions(+), 52 deletions(-) diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h index b7688559ca53..661240d44ad9 100644 --- a/tools/tracing/rtla/src/cli_p.h +++ b/tools/tracing/rtla/src/cli_p.h @@ -5,6 +5,7 @@ #error "Private header file included outside of cli.c module" #endif +#include #include #include #include @@ -239,6 +240,41 @@ static int check_int_range(const struct option *opt, int value) #define RTLA_OPT_DEBUG OPT_BOOLEAN('D', "debug", &config_debug, \ "print debug info") +/* + * Helper functions for parsing numeric option arguments. + */ +static void opt_err(const struct option *opt, const char *arg, const char *msg) +{ + fprintf(stderr, " Error: --%s: '%s' %s\n", opt->long_name, arg, msg); +} + +static int strtoll_safe(const struct option *opt, const char *arg, long long *value) +{ + long long tmp; + char *end; + + errno = 0; + tmp = strtoll(arg, &end, 10); + if (errno || *end || end == arg) { + opt_err(opt, arg, "is not a valid number"); + return -1; + } + *value = tmp; + return 0; +} + +static int strtoi_safe(const struct option *opt, const char *arg, int *value) +{ + int tmp; + + if (strtoi(arg, &tmp)) { + opt_err(opt, arg, "is not a valid number"); + return -1; + } + *value = tmp; + return 0; +} + /* * Common callback functions for command line options */ @@ -255,7 +291,8 @@ static int opt_llong_callback(const struct option *opt, const char *arg, int uns if (!arg) return -1; - *value = get_llong_from_str((char *)arg); + if (strtoll_safe(opt, arg, value)) + return -1; if (check_llong_range(opt, *value)) return -1; return 0; @@ -273,7 +310,7 @@ static int opt_int_callback(const struct option *opt, const char *arg, int unset if (!arg) return -1; - if (strtoi(arg, value)) + if (strtoi_safe(opt, arg, value)) return -1; if (check_int_range(opt, *value)) return -1; @@ -296,8 +333,10 @@ static int opt_cpus_cb(const struct option *opt, const char *arg, int unset) return -1; retval = parse_cpu_set((char *)arg, ¶ms->monitored_cpus); - if (retval) - fatal("Invalid -c cpu list"); + if (retval) { + opt_err(opt, arg, "is not a valid cpu set"); + return -1; + } params->cpus = (char *)arg; return 0; @@ -335,8 +374,10 @@ static int opt_duration_cb(const struct option *opt, const char *arg, int unset) return -1; params->duration = parse_seconds_duration((char *)arg); - if (!params->duration) - fatal("Invalid -d duration"); + if (!params->duration) { + opt_err(opt, arg, "is not a valid duration"); + return -1; + } return 0; } @@ -376,8 +417,10 @@ static int opt_housekeeping_cb(const struct option *opt, const char *arg, int un params->hk_cpus = 1; retval = parse_cpu_set((char *)arg, ¶ms->hk_cpu_set); - if (retval) - fatal("Error parsing house keeping CPUs"); + if (retval) { + opt_err(opt, arg, "is not a valid cpu set"); + return -1; + } return 0; } @@ -397,8 +440,10 @@ static int opt_priority_cb(const struct option *opt, const char *arg, int unset) return -1; retval = parse_prio((char *)arg, ¶ms->sched_param); - if (retval == -1) - fatal("Invalid -P priority"); + if (retval == -1) { + opt_err(opt, arg, "is not a valid priority"); + return -1; + } params->set_sched = 1; return 0; @@ -411,8 +456,10 @@ static int opt_trigger_cb(const struct option *opt, const char *arg, int unset) if (unset || !arg) return -1; - if (!*events) - fatal("--trigger requires a previous -e"); + if (!*events) { + opt_err(opt, arg, "has no previous event to apply to"); + return -1; + } trace_event_add_trigger(*events, (char *)arg); @@ -426,8 +473,10 @@ static int opt_filter_cb(const struct option *opt, const char *arg, int unset) if (unset || !arg) return -1; - if (!*events) - fatal("--filter requires a previous -e"); + if (!*events) { + opt_err(opt, arg, "has no previous event to apply to"); + return -1; + } trace_event_add_filter(*events, (char *)arg); @@ -468,7 +517,8 @@ static int opt_osnoise_auto_cb(const struct option *opt, const char *arg, int un if (!arg) return -1; - auto_thresh = get_llong_from_str((char *)arg); + if (strtoll_safe(opt, arg, &auto_thresh)) + return -1; params->common.stop_us = auto_thresh; params->threshold = 1; @@ -508,8 +558,10 @@ static int opt_osnoise_on_threshold_cb(const struct option *opt, const char *arg return -1; retval = actions_parse(actions, (char *)arg, "osnoise_trace.txt"); - if (retval) - fatal("Invalid action %s", arg); + if (retval) { + opt_err(opt, arg, "is not a valid action"); + return -1; + } return 0; } @@ -523,8 +575,10 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int return -1; retval = actions_parse(actions, (char *)arg, "osnoise_trace.txt"); - if (retval) - fatal("Invalid action %s", arg); + if (retval) { + opt_err(opt, arg, "is not a valid action"); + return -1; + } return 0; } @@ -596,7 +650,8 @@ static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int u if (!arg) return -1; - auto_thresh = get_llong_from_str((char *)arg); + if (strtoll_safe(opt, arg, &auto_thresh)) + return -1; params->common.stop_total_us = auto_thresh; params->common.stop_us = auto_thresh; params->print_stack = auto_thresh; @@ -623,7 +678,8 @@ static int opt_aa_only_cb(const struct option *opt, const char *arg, int unset) if (!arg) return -1; - auto_thresh = get_llong_from_str((char *)arg); + if (strtoll_safe(opt, arg, &auto_thresh)) + return -1; params->common.stop_total_us = auto_thresh; params->common.stop_us = auto_thresh; params->print_stack = auto_thresh; @@ -662,8 +718,10 @@ static int opt_timerlat_on_threshold_cb(const struct option *opt, const char *ar return -1; retval = actions_parse(actions, (char *)arg, "timerlat_trace.txt"); - if (retval) - fatal("Invalid action %s", arg); + if (retval) { + opt_err(opt, arg, "is not a valid action"); + return -1; + } return 0; } @@ -677,8 +735,10 @@ static int opt_timerlat_on_end_cb(const struct option *opt, const char *arg, int return -1; retval = actions_parse(actions, (char *)arg, "timerlat_trace.txt"); - if (retval) - fatal("Invalid action %s", arg); + if (retval) { + opt_err(opt, arg, "is not a valid action"); + return -1; + } return 0; } @@ -727,8 +787,10 @@ static int opt_stack_format_cb(const struct option *opt, const char *arg, int un *format = parse_stack_format((char *)arg); - if (*format == -1) - fatal("Invalid --stack-format option"); + if (*format == -1) { + opt_err(opt, arg, "is not a valid stack format"); + return -1; + } return 0; } @@ -736,6 +798,7 @@ static int opt_stack_format_cb(const struct option *opt, const char *arg, int un static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int unset) { struct timerlat_params *params = opt->value; + long long val; if (unset) { params->timerlat_align = false; @@ -746,10 +809,13 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int if (!arg) return -1; - params->timerlat_align = true; - params->timerlat_align_us = get_llong_from_str((char *)arg); - if (check_llong_range(opt, params->timerlat_align_us)) + if (strtoll_safe(opt, arg, &val)) return -1; + if (check_llong_range(opt, val)) + return -1; + + params->timerlat_align = true; + params->timerlat_align_us = val; return 0; } diff --git a/tools/tracing/rtla/tests/unit/cli_opt_callback.c b/tools/tracing/rtla/tests/unit/cli_opt_callback.c index 8439fb5c6f0b..70e2576b1336 100644 --- a/tools/tracing/rtla/tests/unit/cli_opt_callback.c +++ b/tools/tracing/rtla/tests/unit/cli_opt_callback.c @@ -46,6 +46,28 @@ START_TEST(test_opt_llong_callback_min) } END_TEST +START_TEST(test_opt_llong_callback_non_numeric) +{ + long long test_value = 0; + const struct option opt = TEST_CALLBACK(&test_value, opt_llong_callback); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_llong_callback(&opt, "abc", 0), -1); + ck_assert_int_eq(test_value, 0); +} +END_TEST + +START_TEST(test_opt_llong_callback_non_numeric_suffix) +{ + long long test_value = 0; + const struct option opt = TEST_CALLBACK(&test_value, opt_llong_callback); + + assert(freopen("/dev/null", "w", stderr)); + ck_assert_int_eq(opt_llong_callback(&opt, "1234567890abc", 0), -1); + ck_assert_int_eq(test_value, 0); +} +END_TEST + START_TEST(test_opt_llong_callback_unset) { long long test_value = 0; @@ -105,6 +127,7 @@ START_TEST(test_opt_int_callback_non_numeric) int test_value = 0; const struct option opt = TEST_CALLBACK(&test_value, opt_int_callback); + assert(freopen("/dev/null", "w", stderr)); ck_assert_int_eq(opt_int_callback(&opt, "abc", 0), -1); ck_assert_int_eq(test_value, 0); } @@ -115,6 +138,7 @@ START_TEST(test_opt_int_callback_non_numeric_suffix) int test_value = 0; const struct option opt = TEST_CALLBACK(&test_value, opt_int_callback); + assert(freopen("/dev/null", "w", stderr)); ck_assert_int_eq(opt_int_callback(&opt, "1234567890abc", 0), -1); ck_assert_int_eq(test_value, 0); } @@ -245,7 +269,7 @@ START_TEST(test_opt_cpus_cb_invalid) nr_cpus = 4; assert(freopen("/dev/null", "w", stderr)); - opt_cpus_cb(&opt, "0-3,5", 0); + ck_assert_int_eq(opt_cpus_cb(&opt, "0-3,5", 0), -1); } END_TEST @@ -299,7 +323,7 @@ START_TEST(test_opt_duration_cb_invalid) const struct option opt = TEST_CALLBACK(¶ms, opt_duration_cb); assert(freopen("/dev/null", "w", stderr)); - opt_duration_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_duration_cb(&opt, "abc", 0), -1); } END_TEST @@ -361,7 +385,7 @@ START_TEST(test_opt_housekeeping_cb_invalid) nr_cpus = 4; assert(freopen("/dev/null", "w", stderr)); - opt_housekeeping_cb(&opt, "0-3,5", 0); + ck_assert_int_eq(opt_housekeeping_cb(&opt, "0-3,5", 0), -1); } END_TEST @@ -395,7 +419,7 @@ START_TEST(test_opt_priority_cb_invalid) const struct option opt = TEST_CALLBACK(¶ms, opt_priority_cb); assert(freopen("/dev/null", "w", stderr)); - opt_priority_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_priority_cb(&opt, "abc", 0), -1); } END_TEST @@ -427,7 +451,7 @@ START_TEST(test_opt_trigger_cb_no_event) const struct option opt = TEST_CALLBACK(&events, opt_trigger_cb); assert(freopen("/dev/null", "w", stderr)); - opt_trigger_cb(&opt, "stacktrace", 0); + ck_assert_int_eq(opt_trigger_cb(&opt, "stacktrace", 0), -1); } END_TEST @@ -447,7 +471,7 @@ START_TEST(test_opt_filter_cb_no_event) const struct option opt = TEST_CALLBACK(&events, opt_filter_cb); assert(freopen("/dev/null", "w", stderr)); - opt_filter_cb(&opt, "comm ~ \"rtla\"", 0); + ck_assert_int_eq(opt_filter_cb(&opt, "comm ~ \"rtla\"", 0), -1); } END_TEST @@ -528,7 +552,7 @@ START_TEST(test_opt_osnoise_on_threshold_cb_invalid) const struct option opt = TEST_CALLBACK(&actions, opt_osnoise_on_threshold_cb); assert(freopen("/dev/null", "w", stderr)); - opt_osnoise_on_threshold_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_osnoise_on_threshold_cb(&opt, "abc", 0), -1); } END_TEST @@ -550,7 +574,7 @@ START_TEST(test_opt_osnoise_on_end_cb_invalid) const struct option opt = TEST_CALLBACK(&actions, opt_osnoise_on_end_cb); assert(freopen("/dev/null", "w", stderr)); - opt_osnoise_on_end_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_osnoise_on_end_cb(&opt, "abc", 0), -1); } END_TEST @@ -660,7 +684,7 @@ START_TEST(test_opt_timerlat_on_threshold_cb_invalid) const struct option opt = TEST_CALLBACK(&actions, opt_timerlat_on_threshold_cb); assert(freopen("/dev/null", "w", stderr)); - opt_timerlat_on_threshold_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_timerlat_on_threshold_cb(&opt, "abc", 0), -1); } END_TEST @@ -682,7 +706,7 @@ START_TEST(test_opt_timerlat_on_end_cb_invalid) const struct option opt = TEST_CALLBACK(&actions, opt_timerlat_on_end_cb); assert(freopen("/dev/null", "w", stderr)); - opt_timerlat_on_end_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_timerlat_on_end_cb(&opt, "abc", 0), -1); } END_TEST @@ -782,7 +806,7 @@ START_TEST(test_opt_stack_format_cb_invalid) const struct option opt = TEST_CALLBACK(&stack_format, opt_stack_format_cb); assert(freopen("/dev/null", "w", stderr)); - opt_stack_format_cb(&opt, "abc", 0); + ck_assert_int_eq(opt_stack_format_cb(&opt, "abc", 0), -1); } END_TEST @@ -807,6 +831,8 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_llong_callback_simple); tcase_add_test(tc, test_opt_llong_callback_max); tcase_add_test(tc, test_opt_llong_callback_min); + tcase_add_test(tc, test_opt_llong_callback_non_numeric); + tcase_add_test(tc, test_opt_llong_callback_non_numeric_suffix); tcase_add_test(tc, test_opt_llong_callback_unset); tcase_add_test(tc, test_opt_llong_callback_unset_defval); tcase_add_test(tc, test_opt_llong_callback_range_in); @@ -825,25 +851,25 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_int_callback_range_above); tcase_add_test(tc, test_opt_int_callback_range_boundary); tcase_add_test(tc, test_opt_cpus_cb); - tcase_add_exit_test(tc, test_opt_cpus_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_cpus_cb_invalid); tcase_add_test(tc, test_opt_cgroup_cb); tcase_add_test(tc, test_opt_cgroup_cb_equals); tcase_add_test(tc, test_opt_cgroup_cb_unset); tcase_add_test(tc, test_opt_duration_cb); tcase_add_test(tc, test_opt_duration_cb_unset); - tcase_add_exit_test(tc, test_opt_duration_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_duration_cb_invalid); tcase_add_test(tc, test_opt_event_cb); tcase_add_test(tc, test_opt_event_cb_multiple); tcase_add_test(tc, test_opt_housekeeping_cb); - tcase_add_exit_test(tc, test_opt_housekeeping_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_housekeeping_cb_invalid); tcase_add_test(tc, test_opt_housekeeping_cb_unset); tcase_add_test(tc, test_opt_priority_cb); - tcase_add_exit_test(tc, test_opt_priority_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_priority_cb_invalid); tcase_add_test(tc, test_opt_priority_cb_unset); tcase_add_test(tc, test_opt_trigger_cb); - tcase_add_exit_test(tc, test_opt_trigger_cb_no_event, EXIT_FAILURE); + tcase_add_test(tc, test_opt_trigger_cb_no_event); tcase_add_test(tc, test_opt_filter_cb); - tcase_add_exit_test(tc, test_opt_filter_cb_no_event, EXIT_FAILURE); + tcase_add_test(tc, test_opt_filter_cb_no_event); suite_add_tcase(s, tc); tc = tcase_create("osnoise"); @@ -853,9 +879,9 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_osnoise_trace_output_cb_noarg); tcase_add_test(tc, test_opt_osnoise_trace_output_cb_unset); tcase_add_test(tc, test_opt_osnoise_on_threshold_cb); - tcase_add_exit_test(tc, test_opt_osnoise_on_threshold_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_osnoise_on_threshold_cb_invalid); tcase_add_test(tc, test_opt_osnoise_on_end_cb); - tcase_add_exit_test(tc, test_opt_osnoise_on_end_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_osnoise_on_end_cb_invalid); suite_add_tcase(s, tc); tc = tcase_create("timerlat"); @@ -867,15 +893,15 @@ Suite *cli_opt_callback_suite(void) tcase_add_test(tc, test_opt_timerlat_trace_output_cb_noarg); tcase_add_test(tc, test_opt_timerlat_trace_output_cb_unset); tcase_add_test(tc, test_opt_timerlat_on_threshold_cb); - tcase_add_exit_test(tc, test_opt_timerlat_on_threshold_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_timerlat_on_threshold_cb_invalid); tcase_add_test(tc, test_opt_timerlat_on_end_cb); - tcase_add_exit_test(tc, test_opt_timerlat_on_end_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_timerlat_on_end_cb_invalid); tcase_add_test(tc, test_opt_user_threads_cb); tcase_add_test(tc, test_opt_user_threads_cb_unset); tcase_add_test(tc, test_opt_nano_cb); tcase_add_test(tc, test_opt_nano_cb_unset); tcase_add_test(tc, test_opt_stack_format_cb); - tcase_add_exit_test(tc, test_opt_stack_format_cb_invalid, EXIT_FAILURE); + tcase_add_test(tc, test_opt_stack_format_cb_invalid); tcase_add_test(tc, test_opt_stack_format_cb_unset); tcase_add_test(tc, test_opt_timerlat_align_cb); tcase_add_test(tc, test_opt_timerlat_align_cb_invalid); -- cgit