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
prev parent 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®