linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Crystal Wood <crwood@redhat.com>
To: Costa Shulyupin <costa.shul@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Tomas Glozar <tglozar@redhat.com>, John Kacur <jkacur@redhat.com>,
	Eder Zulian <ezulian@redhat.com>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	Jan Stancek <jstancek@redhat.com>,
	 linux-trace-kernel@vger.kernel.org,
	linux-kernel@vger.kernel.org,  bpf@vger.kernel.org
Subject: Re: [PATCH v2] tools/rtla: Consolidate common parameters into shared structure
Date: Mon, 04 Aug 2025 13:18:33 -0500	[thread overview]
Message-ID: <0faa958ef9cc4b834a5ecdc92acd89520f522d44.camel@redhat.com> (raw)
In-Reply-To: <20250726072455.289445-1-costa.shul@redhat.com>

On Sat, 2025-07-26 at 10:24 +0300, Costa Shulyupin wrote:
> timerlat_params and osnoise_params structures contain 15 identical
> fields.
> 
> Introduce a common_params structure and move those fields into it to
> eliminate the code duplication and improve maintainability.
> 
> Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>

FWIW I have a bigger consolidation patchset in the works, that merges a
lot of the codepaths as well as having everything use osnoise_params
(with some members being tool-specific, indicated by comments).  If you
want, I could rebase that on this and use container_of() to for tool-
specific params... but then that adds complexity with the top and hist-
specific params, most of which are common between timerlat and osnoise
(and not merged by this patch).  So we might want to just keep it simple
with one big struct.

Any thoughts?

> diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> index a2a6f89f342d..4c99a3746380 100644
> --- a/tools/tracing/rtla/src/utils.h
> +++ b/tools/tracing/rtla/src/utils.h
> @@ -59,6 +59,32 @@ struct sched_attr {
>  };
>  #endif /* SCHED_ATTR_SIZE_VER0 */
>  
> +/*
> + * common_params - Parameters shared between timerlat_params and osnoise_params
> + */
> +struct common_params {

I'm not sure that util.h makes sense for this... it's pretty core rtla
stuff rather than helper utilities.  I'd just put it in osnoise.h (or a
new common.h if we want to keep the actual-osnoise-tracer stuff
separate, though currently it's a jumble).

Do we have any naming conventions for the actual osnoise tracer as
opposed to the broader osnoise family?  I don't know if it's likely
we'll ever try to put something outside the osnoise family to rtla, but
if we do "common" could be a bit too generic.  Not sure if that's worth
worrying about at this point.  Certainly better than using "osnoise" for
both without clarifying.

-Crystal


  parent reply	other threads:[~2025-08-04 18:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-26  7:24 [PATCH v2] tools/rtla: Consolidate common parameters into shared structure Costa Shulyupin
2025-07-30 14:18 ` Tomas Glozar
2025-08-04 18:18 ` Crystal Wood [this message]
2025-08-05  7:03   ` Costa Shulyupin
2025-08-05 16:49     ` Crystal Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0faa958ef9cc4b834a5ecdc92acd89520f522d44.camel@redhat.com \
    --to=crwood@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=costa.shul@redhat.com \
    --cc=dan.carpenter@linaro.org \
    --cc=ezulian@redhat.com \
    --cc=jkacur@redhat.com \
    --cc=jstancek@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglozar@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).