From: Gabriele Monaco <gmonaco@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org,
Steven Rostedt <rostedt@goodmis.org>,
linux-trace-kernel@vger.kernel.org,
Ingo Molnar <mingo@redhat.com>, Nam Cao <namcao@linutronix.de>,
Tomas Glozar <tglozar@redhat.com>, Juri Lelli <jlelli@redhat.com>,
Clark Williams <williams@redhat.com>,
John Kacur <jkacur@redhat.com>
Subject: Re: [PATCH v3 01/17] tools/rv: Do not skip idle in trace
Date: Wed, 16 Jul 2025 15:13:43 +0200 [thread overview]
Message-ID: <77bf0cb4262af4edf78dd0b5233788b358bdc79b.camel@redhat.com> (raw)
In-Reply-To: <20250716130846.GC3429938@noisy.programming.kicks-ass.net>
On Wed, 2025-07-16 at 15:08 +0200, Peter Zijlstra wrote:
> > > > > > - if (config_has_id && (config_my_pid == id))
> > > > > > + if (config_my_pid && config_has_id &&
> > > > > > (config_my_pid == id))
> > >
> > > But should we then not write:
> > >
> > > if (config_has_id && (config_my_pid == id))
> >
> > Sorry, got a bit confused, I flipped the two while describing:
> > * -s shows traces from RV but skips from pid-0 (unintended)
> > * omitting -s skips events from RV (correct)
> >
> > If we are running a per-task monitor config_has_id is always true,
> > we pass -s,
> > which makes config_my_pid = 0 (intended /not/ to skip RV).
> > Now when we are about to trace an event from idle (id=0), we skip
> > it, although
> > we really shouldn't.
> > That's why we also needs to check for config_my_pid not being 0.
> >
> > Does it make sense?
>
> Sorta, but would it not make sense to use has_pid := -1 for the
> invalid case, instead of 0, which is a valid pid?
Yeah that's another option, I reckon even cleaner since it's currently
misleading..
next prev parent reply other threads:[~2025-07-16 13:13 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20250715071434.22508-1-gmonaco@redhat.com>
2025-07-15 7:14 ` [PATCH v3 01/17] tools/rv: Do not skip idle in trace Gabriele Monaco
2025-07-16 11:50 ` Peter Zijlstra
2025-07-16 12:18 ` Gabriele Monaco
2025-07-16 12:41 ` Peter Zijlstra
2025-07-16 13:05 ` Gabriele Monaco
2025-07-16 13:08 ` Peter Zijlstra
2025-07-16 13:13 ` Gabriele Monaco [this message]
2025-07-15 7:14 ` [PATCH v3 02/17] tools/rv: Stop gracefully also on SIGTERM Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 03/17] rv: Add da_handle_start_run_event_ to per-task monitors Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 04/17] rv: Remove trailing whitespace from tracepoint string Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 05/17] rv: Return init error when registering monitors Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 06/17] rv: Use strings in da monitors tracepoints Gabriele Monaco
2025-07-15 14:11 ` Nam Cao
2025-07-15 7:14 ` [PATCH v3 07/17] rv: Adjust monitor dependencies Gabriele Monaco
2025-07-16 8:19 ` Nam Cao
2025-07-16 8:30 ` Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 08/17] verification/rvgen: Organise Kconfig entries for nested monitors Gabriele Monaco
2025-07-15 14:48 ` Nam Cao
2025-07-16 7:40 ` Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 09/17] tools/dot2c: Fix generated files going over 100 column limit Gabriele Monaco
2025-07-15 15:01 ` Nam Cao
[not found] ` <d69862275becf1d296c80a08b29b2081857a85a1.camel@redhat.com>
2025-07-16 9:34 ` Nam Cao
2025-07-15 7:14 ` [PATCH v3 10/17] rv: " Gabriele Monaco
2025-07-15 15:08 ` Nam Cao
2025-07-15 15:24 ` Gabriele Monaco
2025-07-16 8:13 ` Nam Cao
2025-07-16 9:07 ` Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 11/17] rv: Retry when da monitor detects race conditions Gabriele Monaco
2025-07-15 15:23 ` Nam Cao
2025-07-16 8:20 ` Gabriele Monaco
2025-07-16 8:27 ` Nam Cao
2025-07-16 8:38 ` Gabriele Monaco
2025-07-16 8:45 ` Nam Cao
2025-07-16 8:59 ` Gabriele Monaco
2025-07-16 9:02 ` Nam Cao
2025-07-15 7:14 ` [PATCH v3 12/17] sched: Adapt sched tracepoints for RV task model Gabriele Monaco
2025-07-16 12:38 ` Peter Zijlstra
2025-07-16 13:40 ` Gabriele Monaco
2025-07-16 13:45 ` Peter Zijlstra
2025-07-16 14:07 ` Gabriele Monaco
2025-07-16 14:19 ` Peter Zijlstra
2025-07-16 14:38 ` Gabriele Monaco
2025-07-16 15:31 ` Peter Zijlstra
2025-07-16 16:14 ` Gabriele Monaco
2025-07-16 15:09 ` Gabriele Monaco
2025-07-16 15:47 ` Peter Zijlstra
2025-07-15 7:14 ` [PATCH v3 13/17] rv: Adapt the sco monitor to the new set_state Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 14/17] rv: Extend snroc model Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 15/17] rv: Replace tss monitor with more complete sts Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 16/17] rv: Add nrp and sssw per-task monitors Gabriele Monaco
2025-07-15 7:14 ` [PATCH v3 17/17] rv: Add opid per-cpu monitor Gabriele Monaco
2025-07-16 9:38 ` Nam Cao
2025-07-16 10:00 ` Gabriele Monaco
2025-07-18 10:26 ` Gabriele Monaco
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=77bf0cb4262af4edf78dd0b5233788b358bdc79b.camel@redhat.com \
--to=gmonaco@redhat.com \
--cc=jkacur@redhat.com \
--cc=jlelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namcao@linutronix.de \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglozar@redhat.com \
--cc=williams@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).