| Message ID | 20250116144931.649593-4-tglozar@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rtla/timerlat: Stop on signal properly when overloaded | expand |
On Thu, 16 Jan 2025 15:49:29 +0100
Tomas Glozar <tglozar@redhat.com> wrote:
> Apply the changes from the previous patch also to timerlat-top.
A change log should never reference another patch. This is meaningless when
seen in a git log. All change logs must be complete stand alone.
I copied the previous patch change log here:
rtla/timerlat_top: Stop timerlat tracer on signal
Currently, when either SIGINT from the user or SIGALRM from the duration
timer is caught by rtla-timerlat, stop_tracing is set to break out of
the main loop. This is not sufficient for cases where the timerlat
tracer is producing more data than rtla can consume, since in that case,
rtla is looping indefinitely inside tracefs_iterate_raw_events, never
reaches the check of stop_tracing and hangs.
In addition to setting stop_tracing, also stop the timerlat tracer on
received signal (SIGINT or SIGALRM). This will stop new samples so that
the existing samples may be processed and tracefs_iterate_raw_events
eventually exits.
-- Steve
pá 17. 1. 2025 v 1:56 odesílatel Steven Rostedt <rostedt@goodmis.org> napsal: > > On Thu, 16 Jan 2025 15:49:29 +0100 > Tomas Glozar <tglozar@redhat.com> wrote: > > > Apply the changes from the previous patch also to timerlat-top. > > A change log should never reference another patch. This is meaningless when > seen in a git log. All change logs must be complete stand alone. If you look up "previous patch" in the Linux commit log, you will find a considerable amount of patches which do this: $ git log master | grep 'previous patch' | wc -l 3006 But you are right, if I apply this patch for example to fix an old version of rtla that does not have timerlat-hist, the log will make no sense. I will stop doing it. > > I copied the previous patch change log here: > Thank you. Tomas
On Fri, 17 Jan 2025 08:13:26 +0100 Tomas Glozar <tglozar@redhat.com> wrote: > > A change log should never reference another patch. This is meaningless when > > seen in a git log. All change logs must be complete stand alone. > > If you look up "previous patch" in the Linux commit log, you will find > a considerable amount of patches which do this: > > $ git log master | grep 'previous patch' | wc -l > 3006 As Linus has scolded me before with "Just because someone else did it wrong doesn't give you the excuse to do it wrong too" ;-) -- Steve
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 059b468981e4..d21a21053917 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -900,9 +900,12 @@ static struct osnoise_tool } static int stop_tracing; +static struct trace_instance *top_inst = NULL; static void stop_top(int sig) { stop_tracing = 1; + if (top_inst) + trace_instance_stop(top_inst); } /* @@ -950,6 +953,13 @@ int timerlat_top_main(int argc, char *argv[]) } trace = &top->trace; + /* + * Save trace instance into global variable so that SIGINT can stop + * the timerlat tracer. + * Otherwise, rtla could loop indefinitely when overloaded. + */ + top_inst = trace; + retval = enable_timerlat(trace); if (retval) { @@ -1131,7 +1141,7 @@ int timerlat_top_main(int argc, char *argv[]) return_value = 0; - if (trace_is_off(&top->trace, &record->trace)) { + if (trace_is_off(&top->trace, &record->trace) && !stop_tracing) { printf("rtla timerlat hit stop tracing\n"); if (!params->no_aa)
Apply the changes from the previous patch also to timerlat-top. Cc: stable@vger.kernel.org Fixes: a828cd18bc4a ("rtla: Add timerlat tool and timelart top mode") Signed-off-by: Tomas Glozar <tglozar@redhat.com> --- tools/tracing/rtla/src/timerlat_top.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)