Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Leo Yan <leo.yan@arm.com>
To: James Clark <james.clark@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@arm.com>, Suyash Mahar <smahar@meta.com>,
	Yeoreum Yun <yeoreum.yun@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Qi Liu <liuqi115@huawei.com>, Junhao He <hejunhao3@huawei.com>,
	coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH v3 4/8] coresight: tmc-etr: Prevent per-thread events from sharing a sink
Date: Tue, 25 Aug 2026 18:32:20 +0100	[thread overview]
Message-ID: <20260825173220.GG8904@e132581.arm.com> (raw)
In-Reply-To: <a7e11c2c-fd83-4ea5-8218-29a5af768578@linaro.org>

On Thu, Aug 20, 2026 at 12:09:25PM +0100, James Clark wrote:

[...]

> > I am just wandering if we can improve the sink driver to only allocate
> > a single bounce buffer that is independent of any threads (and any
> > associated events).
> > 
> >                  | T1 |
> >    CPU0        ------------------------------
> >                     |   T2      |
> >    CPU1        ------------------------------
> >                                 `> T2 stops and can sync trace
> >                                    from the shared bounce buffer
> >                                    to AUX_BUF(T2).
> > 
> >    AUX_BUF(T1) |                            |
> >    AUX_BUF(T2) |                            |
> > 
> >    ETR_BUF     | Bounce buf                 |  -> Used by H/W trace
> > 
> > This might also simplify the CPU-wide case. Each CPU would still have
> > its own AUX buffer, but the ETR driver would maintain only one bounce
> > buffer for the shared sink. A reference count could track how many
> > events are using the sink, with the final event responsible for
> 
> Isn't this how it's already working? get_perf_etr_buf_cpu_wide() allocates a
> single shared buffer with a refcount. I didn't change this, I only changed
> the rules about what is considered shared or not so that it matches the
> semantics of the perf events that back the tracing session.

I think this is slightly different from my point.

The CPU-wide path already uses a shared buffer with a reference count
to support multiple events, while the per-thread path does not.

For the longe term, I would prefer to unify the sink buffer management.
Ideally, ETR/ETF/ETB should manage the sink buffer in the same way
regardless of whether the users come from CPU-wide or per-thread modes.
This would keep perf event semantics out of the low-level sink drivers
as much as possible. However, this would be a larger change and we could
defer in the future.

Now I treat the multiple events in per-thread mode as an implementation
limitation. For the immediate fix, we just reject this case instead.

> > We use a central place etm_event_build_path() to record and compare
> > event's owner and target process, then we don't need to spread the
> > check into sink drivers. We only care about if owner and target must
> > be consistent.

I experimented with moving the check to a common place during buffer
allocation: https://termbin.com/dib3r

It needs locking to keep the check in atomicity, but seems doable. We
don't need to spread event checks across the different sink drivers.

> But we don't know where the target will run when the event is created.
> That's why the check is delayed until etm_event_start() and the process has
> been scheduled. Where it runs needs to be taken into account to calculate if
> this sink can be shared.

Adding the check in etm_event_start() makes the result depend on task
scheduling.

I understand some cases you mentioned may benefit from this, but it also
makes the behaviour less deterministic. I would prefer to reject
unsupported cases explicitly when opening the events.

Thanks,
Leo


  parent reply	other threads:[~2026-08-25 17:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 15:00 [PATCH v3 0/8] coresight: Prevent per-thread events from sharing a sink James Clark
2026-07-28 15:00 ` [PATCH v3 1/8] coresight: tmc-etr: Don't stop Perf cleanup for active sysfs reads James Clark
2026-08-12 16:45   ` Leo Yan
2026-08-13  9:02     ` James Clark
2026-07-28 15:00 ` [PATCH v3 2/8] coresight: configfs: Don't assume active until cscfg_mgr is set James Clark
2026-08-13  9:09   ` Leo Yan
2026-08-13  9:24     ` James Clark
2026-07-28 15:00 ` [PATCH v3 3/8] coresight: etm-perf: Flush workqueue before unloading module James Clark
2026-08-13 14:19   ` Leo Yan
2026-08-14  8:55     ` James Clark
2026-07-28 15:00 ` [PATCH v3 4/8] coresight: tmc-etr: Prevent per-thread events from sharing a sink James Clark
2026-08-13 16:05   ` Leo Yan
2026-08-14  9:09     ` James Clark
2026-08-19  8:45       ` Leo Yan
2026-08-20 11:09         ` James Clark
2026-08-20 11:27           ` James Clark
2026-08-25 17:32           ` Leo Yan [this message]
2026-08-26  8:40             ` James Clark
2026-07-28 15:00 ` [PATCH v3 5/8] coresight: tmc-etr: Use session ID for buffer ownership James Clark
2026-07-28 15:00 ` [PATCH v3 6/8] coresight: tmc-etf: Prevent per-thread events from sharing a sink James Clark
2026-07-28 15:00 ` [PATCH v3 7/8] coresight: etb10: " James Clark
2026-07-28 15:00 ` [PATCH v3 8/8] coresight: ultrasoc-smb: " James Clark

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=20260825173220.GG8904@e132581.arm.com \
    --to=leo.yan@arm.com \
    --cc=coresight@lists.linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hejunhao3@huawei.com \
    --cc=james.clark@linaro.org \
    --cc=jic23@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuqi115@huawei.com \
    --cc=mike.leach@arm.com \
    --cc=smahar@meta.com \
    --cc=suzuki.poulose@arm.com \
    --cc=yeoreum.yun@arm.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