mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "guanghuifeng@linux.alibaba.com" <guanghuifeng@linux.alibaba.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
	jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
	james.clark@linaro.org, tglx@kernel.org, bp@alien8.de,
	dave.hansen@linux.intel.com, hpa@zytor.com, seanjc@google.com,
	kai.huang@intel.com, radu@rendec.net, x86@kernel.org,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf/x86/amd: Move NMI latency window to last-resort suppression
Date: Wed, 16 Sep 2026 21:18:23 +0800	[thread overview]
Message-ID: <16657cb9-eb83-4279-9f3a-46789c5cb8ef@linux.alibaba.com> (raw)
In-Reply-To: <20260916093217.GH4121339@noisy.programming.kicks-ass.net>

You are right — v1 as posted does re-introduce that problem. The perf
handler returning NMI_DONE unconditionally means every latent/spurious
NMI falls through to the reason port path (raw_spin_trylock on
nmi_reason_lock + inb 0x61), which is exactly what df4d29732fda was
designed to avoid.

v2 takes a different approach: consult the latency window in
default_do_nmi() before the reason port read, not after. When the
window is open, the expensive I/O and the global lock are skipped —
identical to the upstream behaviour. The difference is that the
dispatch path does NOT take the "handled" short-circuit exit; instead
it continues into unknown_nmi_error(), which gives NMI_UNKNOWN
handlers (hpwdt, etc.) a chance to identify the NMI.

The cost breakdown per spurious NMI within the window:

upstream (v0): perf claims → goto out
cost: ~10ns (time_after check only)

v1 (broken): perf NMI_DONE → trylock + inb(0x61) → ... → suppress
cost: ~600-1500ns (I/O + lock contention)

v2: perf NMI_DONE → window check → skip reason port
→ nmi_handle(NMI_UNKNOWN) → suppress
cost: ~60-110ns (list traversal, no I/O, no lock)

The only additional work compared to upstream is traversing the
NMI_UNKNOWN handler list. On a typical system that is 1-2 handlers
doing a quick per-CPU variable check each. No I/O port access, no
global lock, no cross-CPU cache line bouncing.

The SERR#/IOCHK# trade-off is unchanged from upstream: when the window
is open, the reason port is not read. This was already the case with
the original mitigation (perf claiming the NMI skips everything after
it). v2 does not make this worse.

v2 will follow as a separate posting. Key changes from v1:
·Add perf_nmi_window_active() check in default_do_nmi() between
the NMI_LOCAL return and the reason port block. The entire reason
port section (trylock, get_nmi_reason, SERR/IOCHK dispatch,
reassert_nmi, unlock) is wrapped in if (!perf_nmi_window_active()).

·Guard perf_nmi_window_active() with "if (!perf_nmi_window) return
false" so that on non-AMD platforms (where amd_core_pmu_init()
never runs and perf_nmi_window stays 0) the strong symbol cannot
accidentally suppress NMIs due to a zero-initialized
perf_nmi_tstamp.

·Initialize per-CPU perf_nmi_tstamp to (jiffies - 1) in
amd_core_pmu_init() so the window starts definitively closed.
Without this, on 32-bit kernels where INITIAL_JIFFIES places
jiffies near the 32-bit wrap point, time_after(jiffies, 0)
evaluates to false for the first 5 minutes of uptime, falsely
indicating an open window.


Thanks


在 2026/9/16 17:32, Peter Zijlstra 写道:
> On Wed, Sep 16, 2026 at 02:57:23PM +0800, Guanghui Feng wrote:
>> The upstream amd_pmu_adjust_nmi_window() mitigation claims every NMI
>> that arrives within a 100ms window opened after a PMC overflow. When no
>> counter overflowed, the handler still returns NMI_HANDLED, which makes
>> the NMI dispatch path take its "handled" exit. Everything that comes
>> after the perf handler is then skipped: the NMI reason port, which may
>> hold a latched SERR#/IOCHK# error, and all NMI_UNKNOWN handlers, such
>> as hpwdt. As a result unrelated NMIs are silently dropped for the whole
>> duration of the window.
>>
>> Stop claiming NMIs from within the perf handler. When no counter
>> overflowed there is nothing to claim, so always report NMI_DONE and let
>> the remaining NMI sources be probed first. Defer the window check to the
>> very end of the dispatch path, where it is used only as a last resort.
>>
>> Introduce perf_nmi_window_active(), which reports whether the latency
>> window opened by the last PMC overflow on this CPU is still open. It is
>> called from unknown_nmi_error() only after all NMI_LOCAL handlers, the
>> reason port and all NMI_UNKNOWN handlers failed to identify the NMI.
>> Only then is the bogus "unknown NMI" report suppressed. A __weak
>> fallback in arch/x86/kernel/nmi.c always returns false, so non-AMD
>> platforms are unaffected.
>>
>> nmi_stats.unknown is still incremented before the suppression, so the
>> dropped NMIs remain observable through debugfs.
> 
> So the point was that AMD hardware was generating these 'spurious' PMIs
> quite frequently, and hitting the reason port at any frequency from
> multiple CPUs is a massive performance problem.
> 
> How are you not re-introducing that?


      parent reply	other threads:[~2026-09-16 13:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16  6:57 Guanghui Feng
2026-09-16  9:32 ` Peter Zijlstra
2026-09-16 12:50   ` [PATCH v2] " Guanghui Feng
2026-09-16 13:04     ` Peter Zijlstra
2026-09-16 13:18   ` guanghuifeng [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=16657cb9-eb83-4279-9f3a-46789c5cb8ef@linux.alibaba.com \
    --to=guanghuifeng@linux.alibaba.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kai.huang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=radu@rendec.net \
    --cc=seanjc@google.com \
    --cc=tglx@kernel.org \
    --cc=x86@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 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®