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

Re: [PATCH 2/7] tools/rtla: Move top/hist union members elsewhere

From: Tomas Glozar <tglozar@redhat.com>
Date: 2025-08-27 06:51:24

út 26. 8. 2025 v 20:06 odesílatel Costa Shulyupin
[off-list ref] napsal:
Moving `hist_params` into `common_params` could lead to
the `common_params` struct becoming a "god object" antipattern.
The `common_params` struct is intended to be tool-agnostic.

I suggest we use `hist_params` within the tools themselves,
rather than in `common_params`.
I wouldn't worry about that at this point. Currently, both
rtla-osnoise and rtla-timerlat, which are the only tools in rtla, use
histograms, and share most of the histogram params. Furthermore,
histograms can be seen as extra data compared to top mode. In this
way, it is natural for hist_params to be a field in common_params.

See, for example, this part of the patch (in timerlat_bpf.c):

-       if (params->entries != 0) {
+       if (params->common.hist.entries != 0) {
                /* Pass histogram options */
-               bpf->rodata->bucket_size = params->bucket_size;
+               bpf->rodata->bucket_size = params->common.hist.bucket_size;

There is more information now in the code, making it clear "entries"
is related to the histogram, plus if BPF sample collection is ever
extended to osnoise, this code can be shared. That would not be
possible if hist_params was moved to osnoise_params/timerlat_params.

If tools that do not use a histogram are added in the future, this
question can be revisited of course.

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