From: Dave Jiang <dave.jiang@intel.com>
To: inux-cxl@vger.kernel.org, linux-perf-users@vger.kernel.org
Cc: jic23@kernel.org, will@kernel.org, mark.rutland@arm.com,
dave@stgolabs.net, robin.murphy@arm.com, icheng@nvidia.com,
sashiko-bot@kernel.org
Subject: [PATCH v4 03/11] perf/cxl: Fix the counter overflow delta fixup
Date: Wed, 5 Aug 2026 08:54:53 -0700 [thread overview]
Message-ID: <20260805155501.1294472-4-dave.jiang@intel.com> (raw)
In-Reply-To: <20260805155501.1294472-1-dave.jiang@intel.com>
The counter is masked to counter_width, so the subtraction in
__cxl_pmu_read() throws away the bit that records the wrap. A delta of one
whole period reads back as 0, the same as no events at all, and only the
overflow status tells the two apart - which is why __cxl_pmu_read() takes
an overflow argument.
The fixup keys off the delta rather than the operands:
delta = (new_cnt - prev_cnt) & GENMASK_ULL(counter_width - 1, 0);
if (overflow && delta < GENMASK_ULL(counter_width - 1, 0))
delta += (1UL << counter_width);
so it cannot tell which of these it is looking at:
event_start polled read wrap
ctr = 0 .......... ctr = P/2 ......... mask -> 0 (+r)
prev = 0 prev = P/2 IRQ reads new = r
prev = 0 (no read yet): new >= prev, fell short -> add period
prev = P/2 (polled): new < prev, spans wrap -> add nothing
Both rows are the same interrupt and the old guard adds a period in both,
so a mid-period read makes the event over-count. 'perf stat -I' hits that.
Condition the fixup on new_cnt >= prev_cnt, the one case the subtraction
cannot express. Dropping it outright would break the first row. It stays
exact whatever the residual r is, which matters because some events
increment by more than 1 per cycle (CXL r4.0 8.2.7.2.1, Threshold) and can
step past 0 as they wrap.
Use mask + 1 for the period rather than a shift. It is 0 for a 64-bit
counter, and avoids the old 1UL << counter_width - undefined for width 64,
and for >= 32 on 32-bit kernels.
Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/perf/cxl_pmu.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index b16e2e4090a3..40741e529d9b 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -689,7 +689,7 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
{
struct cxl_pmu_info *info = pmu_to_cxl_pmu_info(event->pmu);
struct hw_perf_event *hwc = &event->hw;
- u64 new_cnt, prev_cnt, delta;
+ u64 new_cnt, prev_cnt, delta, mask;
do {
prev_cnt = local64_read(&hwc->prev_count);
@@ -697,12 +697,16 @@ static void __cxl_pmu_read(struct perf_event *event, bool overflow)
} while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) != prev_cnt);
/*
- * If we know an overflow occur then take that into account.
- * Note counter is not reset as that would lose events
+ * The mask discards the bit that says the counter wrapped, so a delta of
+ * one whole period reads back as 0 - the same as no events at all. Only
+ * the overflow status separates them, and new_cnt >= prev_cnt is that
+ * case, so add the period back. mask + 1 is 2^counter_width, which comes
+ * out as 0 for a 64-bit counter and avoids an undefined 1 << 64.
*/
- delta = (new_cnt - prev_cnt) & GENMASK_ULL(info->counter_width - 1, 0);
- if (overflow && delta < GENMASK_ULL(info->counter_width - 1, 0))
- delta += (1UL << info->counter_width);
+ mask = GENMASK_ULL(info->counter_width - 1, 0);
+ delta = (new_cnt - prev_cnt) & mask;
+ if (overflow && new_cnt >= prev_cnt)
+ delta += mask + 1;
local64_add(delta, &event->count);
}
--
2.54.0
next prev parent reply other threads:[~2026-08-05 15:55 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-05 15:54 [PATCH v4 00/11] perf/cxlpmu: Misc sashiko raised issues fixes Dave Jiang
2026-08-05 15:54 ` [PATCH v4 01/11] perf/cxl: Program the requested event group on configurable counters Dave Jiang
2026-08-05 15:54 ` [PATCH v4 02/11] perf/cxl: Clear stale event fields before reprogramming a counter Dave Jiang
2026-08-05 16:07 ` sashiko-bot
2026-08-05 15:54 ` Dave Jiang [this message]
2026-08-05 16:07 ` [PATCH v4 03/11] perf/cxl: Fix the counter overflow delta fixup sashiko-bot
2026-08-05 15:54 ` [PATCH v4 04/11] perf/cxl: Accept an overflow interrupt on MSI message number 0 Dave Jiang
2026-08-05 16:06 ` sashiko-bot
2026-08-05 15:54 ` [PATCH v4 05/11] perf/cxl: Split the MSI vector out of info->irq Dave Jiang
2026-08-05 16:07 ` sashiko-bot
2026-08-05 15:54 ` [PATCH v4 06/11] cxl/pci: Add the PMUs after configuring events Dave Jiang
2026-08-05 15:54 ` [PATCH v4 07/11] perf/cxl: Don't share the overflow interrupt, and keep it pinned Dave Jiang
2026-08-05 15:54 ` [PATCH v4 08/11] perf/cxl: Unfreeze counters after handling an overflow interrupt Dave Jiang
2026-08-05 15:54 ` [PATCH v4 09/11] perf/cxl: Validate the hardware-reported counter width Dave Jiang
2026-08-05 15:55 ` [PATCH v4 10/11] perf/cxl: Don't log through pmu.dev in the overflow interrupt handler Dave Jiang
2026-08-05 16:16 ` sashiko-bot
2026-08-05 15:55 ` [PATCH v4 11/11] perf/cxl: Clear stale overflow status before using a counter Dave Jiang
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=20260805155501.1294472-4-dave.jiang@intel.com \
--to=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=icheng@nvidia.com \
--cc=inux-cxl@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robin.murphy@arm.com \
--cc=sashiko-bot@kernel.org \
--cc=will@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.