Thread (27 messages) 27 messages, 3 authors, 2025-09-03

Re: [PATCH 4/7] tools/rtla: Consolidate code between osnoise/timerlat and hist/top

From: Crystal Wood <hidden>
Date: 2025-08-29 20:46:40

On Wed, 2025-08-27 at 15:34 +0200, Tomas Glozar wrote:
čt 21. 8. 2025 v 5:58 odesílatel Crystal Wood [off-list ref] napsal:
quoted
Some bits like aa_only and the user stuff are in common even though they
are only used by timerlat.  In the case of user stuff I plan to submit
patches for supporting that on osnoise, and it doesn't seem worthwhile to
jump through hoops to move some of that code out of code that is otherwise
able to be shared.  For aa_only, making it non-common would have precluded
sharing some code, and the code that the flag inhibits isn't tool-specific
(unlike no_aa).  Plus, there's other common code referring to tool->aa.
---
aa_only is implemented only for top, because the behavior would be the
same for hist: just do auto-analysis, no histogram, no top. Of course,
it doesn't hurt that it is included as a common parameter.
quoted
+       stopped = osnoise_trace_is_off(tool, tool->record) && !stop_tracing;
+       if (stopped) {
+               printf("rtla hit stop tracing\n");
+               return_value = FAILED;
+       }
+
Is the output change necessary? The original "osnoise hit stop
tracing"/"timerlat hit stop tracing" refers to the osnoise and
timerlat tracers, which stop tracing on threshold (unless mode is
TRACING_MODE_BPF
). We already have ops->tracer for the tracer name, that can be used here.
Certainly not necessary, but it didn't seem like an important
distinction to maintain -- unless there are scripts out there (beyond
our test cases) checking for it that we don't want to break.  Though
for not-yet-written scripts I'd imagine having to deal with different
names could be an annoyance as well.
quoted
 /*
  * timerlat_top_bpf_main_loop - main loop to process events (BPF variant)
  */
 static int
-timerlat_top_bpf_main_loop(struct osnoise_tool *top,
-                          struct osnoise_tool *record,
-                          struct osnoise_tool *aa,
-                          struct timerlat_params *params,
-                          struct timerlat_u_params *params_u)
+__timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
The naming here should be consistent with the comment and with
timerlat-hist, that is, without the __ prefix, there is no other
timerlat_top_bpf_main_loop that would create a conflict in this
version of the patch.
Oops... I think there used to be a non-underscore version at some point
during development.  Will fix.

-Crystal
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help