út 26. 8. 2025 v 20:06 odesílatel Costa Shulyupin
<costa.s...@redhat.com> 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
