[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c8fad013-b022-4037-94e8-1f0536535469@linaro.org>
Date: Tue, 16 Dec 2025 18:27:27 +0200
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
<mike.leach@...aro.org>, Anshuman Khandual <anshuman.khandual@....com>,
Yeoreum Yun <yeoreum.yun@....com>, Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>, Tamas Petz <tamas.petz@....com>,
Tamas Zsoldos <tamas.zsoldos@....com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH 16/19] coresight: trbe: Support trigger mode
On 01/12/2025 13:22, Leo Yan wrote:
> The buffer currently operates in fill mode, where tracing stops when it
> reaches the end of the buffer and a maintenance interrupt is raised.
> However, due to IRQ latency, trace data may be lost during the window in
> which tracing is halted but the program continues to run.
>
> To mitigate the issue, this commit enables the trigger count to support
> buffer maintenance without disabling tracing. This is fulfilled with
> two modes:
>
> 1) Set a trigger count as a watermark and use fill mode to prevent the
> buffer from being overwritten. Once the count is decremented to
> zero, an interrupt is raised for buffer maintenance, but the hardware
> continues collecting trace data until limit.
>
> head watermark tail
> +----+---------------+---------+-------+
> |$$$$| | |$$$$$$$|
> +----+---------------+---------+-------+
> base `---- count ----' limit base + nr_pages
>
> $$$ : Filled trace data
>
> 2) Use wrap mode so that tracing continues when reach the top of the
> buffer. The trigger count is configured as "Stop on trigger" to
> guard the trace data not to be overwritten.
>
> watermark tail head
> +--------+-----------+---------+-------+
> | | |$$$$$$$$$| |
> +--------+-----------+---------+-------+
> base base + nr_pages
> limit
>
> `------->
> >-- counter ---------'
>
> $$$ : Filled trace data
>
> The modes are selected by comparing the limit with the trigger position.
>
> An extra TRBE_FAULT_ACT_TRIG state is introduced for fault action, it is
> used to distinguish the trigger event from the WRAP event.
>
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 101 +++++++++++++++++++++------
> drivers/hwtracing/coresight/coresight-trbe.h | 14 ++++
> 2 files changed, 94 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 8390d0a8fe23d35945610df15f21751279ee37ee..0551ea9b4f8286c156e3c9c7ac94e2ecd3b9dc3f 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -48,6 +48,7 @@
> #define TRBE_TRACE_MIN_BUF_SIZE 64
>
> enum trbe_fault_action {
> + TRBE_FAULT_ACT_TRIG,
> TRBE_FAULT_ACT_WRAP,
> TRBE_FAULT_ACT_SPURIOUS,
> TRBE_FAULT_ACT_FATAL,
> @@ -67,6 +68,7 @@ struct trbe_buf {
> unsigned long trbe_hw_base;
> unsigned long trbe_limit;
> unsigned long trbe_write;
> + unsigned long trbe_count;
> int nr_pages;
> void **pages;
> bool snapshot;
> @@ -478,6 +480,10 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
> if (head < tail)
> limit = round_down(tail, PAGE_SIZE);
>
> + /* If trigger mode is enabled, no need to use limit for watermark */
> + if (!static_branch_unlikely(&trbe_trigger_mode_bypass))
> + goto out;
> +
> /*
> * Wakeup may be arbitrarily far into the future. If it's not in the
> * current generation, either we'll wrap before hitting it, or it's
> @@ -495,6 +501,7 @@ static unsigned long __trbe_normal_offset(struct perf_output_handle *handle)
> if (handle->wakeup < (handle->head + handle->size) && head <= wakeup)
> limit = min(limit, round_up(wakeup, PAGE_SIZE));
I kept wondering how this limit was relvant now in wrap mode. But I see
it's skipped from the goto above, but that only jumps over one
statement. Can we have that static branch as a regular if statement
guarding the limit calculation instead of the goto?
Negating "trigger_mode_bypass" and renaming it into just "trigger_mode"
might help too. The double negative of "not trigger_trigger_mode_bypass
goto" is a bit hard to understand.
>
> +out:
> /*
>
Powered by blists - more mailing lists