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