[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251008195905.333514-2-costa.shul@redhat.com>
Date: Wed, 8 Oct 2025 22:59:01 +0300
From: Costa Shulyupin <costa.shul@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>,
Tomas Glozar <tglozar@...hat.com>,
Costa Shulyupin <costa.shul@...hat.com>,
Crystal Wood <crwood@...hat.com>,
John Kacur <jkacur@...hat.com>,
Jan Stancek <jstancek@...hat.com>,
Tiezhu Yang <yangtiezhu@...ngson.cn>,
linux-trace-kernel@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH v1 1/5] tools/rtla: Add fatal() and replace error handling pattern
The code contains some technical debt in error handling,
which complicates the consolidation of duplicated code.
Introduce an fatal() function to replace the common pattern of
err_msg() followed by exit(EXIT_FAILURE), reducing the length of an
already long function.
Further patches using fatal() follow.
Signed-off-by: Costa Shulyupin <costa.shul@...hat.com>
---
tools/tracing/rtla/src/osnoise_hist.c | 42 ++++++++--------------
tools/tracing/rtla/src/osnoise_top.c | 42 ++++++++--------------
tools/tracing/rtla/src/timerlat_hist.c | 50 +++++++++-----------------
tools/tracing/rtla/src/timerlat_top.c | 48 +++++++++----------------
tools/tracing/rtla/src/timerlat_u.c | 12 +++----
tools/tracing/rtla/src/utils.c | 14 ++++++++
tools/tracing/rtla/src/utils.h | 1 +
7 files changed, 80 insertions(+), 129 deletions(-)
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index dffb6d0a98d7..43c323521f55 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -592,10 +592,8 @@ static struct common_params
break;
case 'e':
tevent = trace_event_alloc(optarg);
- if (!tevent) {
- err_msg("Error alloc trace event");
- exit(EXIT_FAILURE);
- }
+ if (!tevent)
+ fatal("Error alloc trace event");
if (params->common.events)
tevent->next = params->common.events;
@@ -615,10 +613,8 @@ static struct common_params
case 'H':
params->common.hk_cpus = 1;
retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set);
- if (retval) {
- err_msg("Error parsing house keeping CPUs\n");
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error parsing house keeping CPUs\n");
break;
case 'p':
params->period = get_llong_from_str(optarg);
@@ -671,10 +667,8 @@ static struct common_params
case '4': /* trigger */
if (params->common.events) {
retval = trace_event_add_trigger(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding trigger %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding trigger %s\n", optarg);
} else {
osnoise_hist_usage("--trigger requires a previous -e\n");
}
@@ -682,10 +676,8 @@ static struct common_params
case '5': /* filter */
if (params->common.events) {
retval = trace_event_add_filter(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding filter %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding filter %s\n", optarg);
} else {
osnoise_hist_usage("--filter requires a previous -e\n");
}
@@ -699,18 +691,14 @@ static struct common_params
case '8':
retval = actions_parse(¶ms->common.threshold_actions, optarg,
"osnoise_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
case '9':
retval = actions_parse(¶ms->common.end_actions, optarg,
"osnoise_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
default:
osnoise_hist_usage("Invalid option");
@@ -720,10 +708,8 @@ static struct common_params
if (trace_output)
actions_add_trace_output(¶ms->common.threshold_actions, trace_output);
- if (geteuid()) {
- err_msg("rtla needs root permission\n");
- exit(EXIT_FAILURE);
- }
+ if (geteuid())
+ fatal("rtla needs root permission\n");
if (params->common.hist.no_index && !params->common.hist.with_zeros)
osnoise_hist_usage("no-index set and with-zeros not set - it does not make sense");
diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index 95418f7ecc96..43fb50b5d936 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -426,10 +426,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
break;
case 'e':
tevent = trace_event_alloc(optarg);
- if (!tevent) {
- err_msg("Error alloc trace event");
- exit(EXIT_FAILURE);
- }
+ if (!tevent)
+ fatal("Error alloc trace event");
if (params->common.events)
tevent->next = params->common.events;
@@ -443,10 +441,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
case 'H':
params->common.hk_cpus = 1;
retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set);
- if (retval) {
- err_msg("Error parsing house keeping CPUs\n");
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error parsing house keeping CPUs\n");
break;
case 'p':
params->period = get_llong_from_str(optarg);
@@ -490,10 +486,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
case '0': /* trigger */
if (params->common.events) {
retval = trace_event_add_trigger(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding trigger %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding trigger %s\n", optarg);
} else {
osnoise_top_usage(params, "--trigger requires a previous -e\n");
}
@@ -501,10 +495,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
case '1': /* filter */
if (params->common.events) {
retval = trace_event_add_filter(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding filter %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding filter %s\n", optarg);
} else {
osnoise_top_usage(params, "--filter requires a previous -e\n");
}
@@ -518,18 +510,14 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
case '4':
retval = actions_parse(¶ms->common.threshold_actions, optarg,
"osnoise_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
case '5':
retval = actions_parse(¶ms->common.end_actions, optarg,
"osnoise_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
default:
osnoise_top_usage(params, "Invalid option");
@@ -539,10 +527,8 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
if (trace_output)
actions_add_trace_output(¶ms->common.threshold_actions, trace_output);
- if (geteuid()) {
- err_msg("osnoise needs root permission\n");
- exit(EXIT_FAILURE);
- }
+ if (geteuid())
+ fatal("osnoise needs root permission\n");
return ¶ms->common;
}
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 606c1688057b..6504556be5e4 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -913,10 +913,8 @@ static struct common_params
break;
case 'e':
tevent = trace_event_alloc(optarg);
- if (!tevent) {
- err_msg("Error alloc trace event");
- exit(EXIT_FAILURE);
- }
+ if (!tevent)
+ fatal("Error alloc trace event");
if (params->common.events)
tevent->next = params->common.events;
@@ -936,10 +934,8 @@ static struct common_params
case 'H':
params->common.hk_cpus = 1;
retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set);
- if (retval) {
- err_msg("Error parsing house keeping CPUs\n");
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error parsing house keeping CPUs\n");
break;
case 'i':
params->common.stop_us = get_llong_from_str(optarg);
@@ -1005,10 +1001,8 @@ static struct common_params
case '6': /* trigger */
if (params->common.events) {
retval = trace_event_add_trigger(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding trigger %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding trigger %s\n", optarg);
} else {
timerlat_hist_usage("--trigger requires a previous -e\n");
}
@@ -1016,20 +1010,16 @@ static struct common_params
case '7': /* filter */
if (params->common.events) {
retval = trace_event_add_filter(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding filter %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding filter %s\n", optarg);
} else {
timerlat_hist_usage("--filter requires a previous -e\n");
}
break;
case '8':
params->dma_latency = get_llong_from_str(optarg);
- if (params->dma_latency < 0 || params->dma_latency > 10000) {
- err_msg("--dma-latency needs to be >= 0 and < 10000");
- exit(EXIT_FAILURE);
- }
+ if (params->dma_latency < 0 || params->dma_latency > 10000)
+ fatal("--dma-latency needs to be >= 0 and < 10000");
break;
case '9':
params->no_aa = 1;
@@ -1049,31 +1039,25 @@ static struct common_params
case '\5':
retval = actions_parse(¶ms->common.threshold_actions, optarg,
"timerlat_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
case '\6':
retval = actions_parse(¶ms->common.end_actions, optarg,
"timerlat_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
default:
- timerlat_hist_usage("Invalid option");
+ fatal("Invalid option\n");
}
}
if (trace_output)
actions_add_trace_output(¶ms->common.threshold_actions, trace_output);
- if (geteuid()) {
- err_msg("rtla needs root permission\n");
- exit(EXIT_FAILURE);
- }
+ if (geteuid())
+ fatal("rtla needs root permission\n");
if (params->common.hist.no_irq && params->common.hist.no_thread)
timerlat_hist_usage("no-irq and no-thread set, there is nothing to do here");
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index fc479a0dcb59..03ecc38d0719 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -671,10 +671,8 @@ static struct common_params
break;
case 'e':
tevent = trace_event_alloc(optarg);
- if (!tevent) {
- err_msg("Error alloc trace event");
- exit(EXIT_FAILURE);
- }
+ if (!tevent)
+ fatal("Error alloc trace event");
if (params->common.events)
tevent->next = params->common.events;
@@ -687,10 +685,8 @@ static struct common_params
case 'H':
params->common.hk_cpus = 1;
retval = parse_cpu_set(optarg, ¶ms->common.hk_cpu_set);
- if (retval) {
- err_msg("Error parsing house keeping CPUs\n");
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error parsing house keeping CPUs\n");
break;
case 'i':
params->common.stop_us = get_llong_from_str(optarg);
@@ -741,10 +737,8 @@ static struct common_params
case '0': /* trigger */
if (params->common.events) {
retval = trace_event_add_trigger(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding trigger %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding trigger %s\n", optarg);
} else {
timerlat_top_usage("--trigger requires a previous -e\n");
}
@@ -752,20 +746,16 @@ static struct common_params
case '1': /* filter */
if (params->common.events) {
retval = trace_event_add_filter(params->common.events, optarg);
- if (retval) {
- err_msg("Error adding filter %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Error adding filter %s\n", optarg);
} else {
timerlat_top_usage("--filter requires a previous -e\n");
}
break;
case '2': /* dma-latency */
params->dma_latency = get_llong_from_str(optarg);
- if (params->dma_latency < 0 || params->dma_latency > 10000) {
- err_msg("--dma-latency needs to be >= 0 and < 10000");
- exit(EXIT_FAILURE);
- }
+ if (params->dma_latency < 0 || params->dma_latency > 10000)
+ fatal("--dma-latency needs to be >= 0 and < 10000");
break;
case '3': /* no-aa */
params->no_aa = 1;
@@ -785,18 +775,14 @@ static struct common_params
case '9':
retval = actions_parse(¶ms->common.threshold_actions, optarg,
"timerlat_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
case '\1':
retval = actions_parse(¶ms->common.end_actions, optarg,
"timerlat_trace.txt");
- if (retval) {
- err_msg("Invalid action %s\n", optarg);
- exit(EXIT_FAILURE);
- }
+ if (retval)
+ fatal("Invalid action %s\n", optarg);
break;
default:
timerlat_top_usage("Invalid option");
@@ -806,10 +792,8 @@ static struct common_params
if (trace_output)
actions_add_trace_output(¶ms->common.threshold_actions, trace_output);
- if (geteuid()) {
- err_msg("rtla needs root permission\n");
- exit(EXIT_FAILURE);
- }
+ if (geteuid())
+ fatal("rtla needs root permission\n");
/*
* Auto analysis only happens if stop tracing, thus:
diff --git a/tools/tracing/rtla/src/timerlat_u.c b/tools/tracing/rtla/src/timerlat_u.c
index 01dbf9a6b5a5..2f85c242e1d5 100644
--- a/tools/tracing/rtla/src/timerlat_u.c
+++ b/tools/tracing/rtla/src/timerlat_u.c
@@ -51,10 +51,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
if (!params->sched_param) {
retval = sched_setscheduler(0, SCHED_FIFO, &sp);
- if (retval < 0) {
- err_msg("Error setting timerlat u default priority: %s\n", strerror(errno));
- exit(1);
- }
+ if (retval < 0)
+ fatal("Error setting timerlat u default priority: %s\n", strerror(errno));
} else {
retval = __set_sched_attr(getpid(), params->sched_param);
if (retval) {
@@ -78,10 +76,8 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
snprintf(buffer, sizeof(buffer), "osnoise/per_cpu/cpu%d/timerlat_fd", cpu);
timerlat_fd = tracefs_instance_file_open(NULL, buffer, O_RDONLY);
- if (timerlat_fd < 0) {
- err_msg("Error opening %s:%s\n", buffer, strerror(errno));
- exit(1);
- }
+ if (timerlat_fd < 0)
+ fatal("Error opening %s:%s\n", buffer, strerror(errno));
debug_msg("User-space timerlat pid %d on cpu %d\n", gettid(), cpu);
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index d6ab15dcb490..bf3662313cde 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -38,6 +38,20 @@ void err_msg(const char *fmt, ...)
fprintf(stderr, "%s", message);
}
+/*
+ * fatal - print an error message to stderr and exit with ERROR
+ */
+void fatal(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(stderr, fmt, ap);
+ va_end(ap);
+
+ exit(ERROR);
+}
+
/*
* debug_msg - print a debug message to stderr if debug is set
*/
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index a2a6f89f342d..1be095f9a7e6 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -19,6 +19,7 @@
extern int config_debug;
void debug_msg(const char *fmt, ...);
void err_msg(const char *fmt, ...);
+void fatal(const char *fmt, ...);
long parse_seconds_duration(char *val);
void get_duration(time_t start_time, char *output, int output_size);
--
2.51.0
Powered by blists - more mailing lists