From: Costa Shulyupin <costa.shul@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>,
Tomas Glozar <tglozar@redhat.com>,
Costa Shulyupin <costa.shul@redhat.com>,
Crystal Wood <crwood@redhat.com>, John Kacur <jkacur@redhat.com>,
Jan Stancek <jstancek@redhat.com>,
Tiezhu Yang <yangtiezhu@loongson.cn>,
linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v1 1/5] tools/rtla: Add fatal() and replace error handling pattern
Date: Wed, 8 Oct 2025 22:59:01 +0300 [thread overview]
Message-ID: <20251008195905.333514-2-costa.shul@redhat.com> (raw)
In-Reply-To: <20251008195905.333514-1-costa.shul@redhat.com>
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@redhat.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
next prev parent reply other threads:[~2025-10-08 19:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-08 19:59 [PATCH v1 0/5] tools/rtla: Improve argument processing Costa Shulyupin
2025-10-08 19:59 ` Costa Shulyupin [this message]
2025-10-08 21:55 ` [PATCH v1 1/5] tools/rtla: Add fatal() and replace error handling pattern Crystal Wood
2025-10-10 7:20 ` Costa Shulyupin
2025-10-10 17:50 ` Crystal Wood
2025-10-08 19:59 ` [PATCH v1 2/5] tools/rtla: Replace timerlat_top_usage("...") with fatal("...") Costa Shulyupin
2025-10-08 19:59 ` [PATCH v1 3/5] tools/rtla: Replace timerlat_hist_usage("...") " Costa Shulyupin
2025-10-08 19:59 ` [PATCH v1 4/5] tools/rtla: Replace osnoise_hist_usage("...") " Costa Shulyupin
2025-10-08 19:59 ` [PATCH v1 5/5] tools/rtla: Replace osnoise_top_usage("...") " Costa Shulyupin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251008195905.333514-2-costa.shul@redhat.com \
--to=costa.shul@redhat.com \
--cc=crwood@redhat.com \
--cc=jkacur@redhat.com \
--cc=jstancek@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglozar@redhat.com \
--cc=yangtiezhu@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).