mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Gabriele Monaco <gmonaco@redhat.com>
To: Tobias Schaffner <tobias.schaffner@siemens.com>,
	Steven Rostedt <rostedt@goodmis.org>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	 Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	 linux-trace-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	 linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	rpm@xenomai.org, 	jan.kiszka@siemens.com
Subject: Re: [PATCH v2 3/3] rv: collect per-edge dwell time for per-cpu DA/HA monitors
Date: Thu, 24 Sep 2026 09:18:24 +0200	[thread overview]
Message-ID: <c8bf231a39b5303bb283b12648a9e23b5c4bd07e.camel@redhat.com> (raw)
In-Reply-To: <20260911123439.463052-4-tobias.schaffner@siemens.com>

On Fri, 2026-09-11 at 14:34 +0200, Tobias Schaffner wrote:
> With the core facility in place, hook it into the DA/HA layer so that any
> per-CPU automaton monitor also reports how long it lingers in each state.
> 
> Pack the automaton state and its entry timestamp into one word so the
> transition cmpxchg updates both atomically. This prevents nested events
> from attributing dwell time to the wrong edge and requires a native
> 64-bit cmpxchg.
> 
> Account a transition as soon as its state change is committed. An HA
> constraint that rejects the transition can therefore reset the monitor
> without leaving an accounting window for nested events.
> 
> Add a selftest that enables a per-CPU monitor, checks the stats file
> appears and is populated under load, and skips cleanly otherwise.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---

Sorry for taking that long, I could just have a quick look at your patches, the
approach looks fine but I'm going to need more time to check them carefully,
likely after the LPC conference.

I'm not too fond of how you split into patches. What's the reason for splitting
1/3 and 2/3 ? It seems they both add the new functionality, one step at a time.

...
> +/* Edge statistics pack the state and its entry timestamp into one word. */
> +#ifdef CONFIG_RV_EDGE_STAT
> +#define da_state_t	unsigned long
> +#else
> +#define da_state_t	unsigned int
> +#endif

Also try to define things in order, da_state_t is used in 2/3 but not defined
until 3/3. This doesn't break builds (the header is unused until 3/3) but is
just harder to review.

I find it more natural to split it into 1 add new functionality, 2 wire it, 3
test it. What do you think?

Thanks,
Gabriele


      reply	other threads:[~2026-09-24  7:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 12:34 [PATCH v2 0/3] rv: per-edge dwell-time statistics for per-cpu monitors Tobias Schaffner
2026-09-11 12:34 ` [PATCH v2 1/3] rv: add per-edge dwell-time statistics primitive Tobias Schaffner
2026-09-24  7:21   ` Gabriele Monaco
2026-09-11 12:34 ` [PATCH v2 2/3] rv: add per-monitor edge-stat facility and stats file Tobias Schaffner
2026-09-24  7:47   ` Gabriele Monaco
2026-09-11 12:34 ` [PATCH v2 3/3] rv: collect per-edge dwell time for per-cpu DA/HA monitors Tobias Schaffner
2026-09-24  7:18   ` Gabriele Monaco [this message]

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=c8bf231a39b5303bb283b12648a9e23b5c4bd07e.camel@redhat.com \
    --to=gmonaco@redhat.com \
    --cc=corbet@lwn.net \
    --cc=jan.kiszka@siemens.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=rpm@xenomai.org \
    --cc=skhan@linuxfoundation.org \
    --cc=tobias.schaffner@siemens.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

all inboxes | Powered by JetHome®