[PATCH] nvme: log when APST skips a state due to latency cap

[PATCH] nvme: log when APST skips a state due to latency cap

[Mike Bommarito]

nvme_configure_apst() silently skips any non-operational power state
whose exit_lat exceeds ctrl->ps_max_latency_us (set from the
default_ps_max_latency_us module parameter, default 100,000 us).

When the kernel skips a drive's deepest non-op state for this reason,
the user gets no signal that anything happened.  This is a frequent
gotcha on Intel platforms that depend on the drive reaching its
deepest power state to allow the PCIe root port to enter L1.x and the
PCH Southport to power-gate, which in turn is a prerequisite for the
platform reaching S0ix during s2idle suspend.

Add a dev_info_once() at the skip site that names the state, prints
the actual exit_lat versus the current cap, and points the user at
the module parameter to raise it.  Using _once keeps the line to a
single instance per controller; users that want to see every probe
event can still enable dev_dbg.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Mike Bommarito <michael.bommarito@gmail.com>
---
Build- and boot-tested on Framework Laptop 13 (Intel Core Ultra 5 125H)
running Linux 7.1-rc4 with this patch applied, across multiple boots
over three days.  No regressions observed in NVMe probe or APST
configuration with the installed WD Black SN850X (15b7:5030).

Design context: the conservative default cap and the per-device-quirk
mitigation were discussed in 2017 at

    https://lore.kernel.org/all/20170606095454.GA13569@infradead.org/

and retained.  This patch only adds observability for the silent-skip
case; behavior is unchanged.

 drivers/nvme/host/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c3032d6ad6b1..220e844e852c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2943,8 +2943,13 @@ static int nvme_configure_apst(struct nvme_ctrl *ctrl)
 			continue;
 
 		exit_latency_us = (u64)le32_to_cpu(ctrl->psd[state].exit_lat);
-		if (exit_latency_us > ctrl->ps_max_latency_us)
+		if (exit_latency_us > ctrl->ps_max_latency_us) {
+			dev_info_once(ctrl->device,
+				      "APST: skipping non-op PS%d (exit_lat %llu us > cap %llu us); raise nvme_core.default_ps_max_latency_us to enable\n",
+				      state, exit_latency_us,
+				      ctrl->ps_max_latency_us);
 			continue;
+		}
 
 		total_latency_us = exit_latency_us +
 			le32_to_cpu(ctrl->psd[state].entry_lat);
-- 
2.53.0

Re: [PATCH] nvme: log when APST skips a state due to latency cap

[Christoph Hellwig]

On Sat, May 23, 2026 at 07:45:52AM -0400, Mike Bommarito wrote:
> Assisted-by: Claude:claude-opus-4-7

Either you take charge of this and drop these stupid annotations,
or you don't and we'll have to ignore it.

Re: [PATCH] nvme: log when APST skips a state due to latency cap

[Michael Bommarito]

On Mon, May 25, 2026 at 1:59 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Sat, May 23, 2026 at 07:45:52AM -0400, Mike Bommarito wrote:
> > Assisted-by: Claude:claude-opus-4-7
>
> Either you take charge of this and drop these stupid annotations,
> or you don't and we'll have to ignore it.

You mean that crediting Claude for a one-line debug print is
unnecessary?  Or you mean that the verbosity of the comments/etc. is
too much?

Thanks,
Mike

Re: [PATCH] nvme: log when APST skips a state due to latency cap

[Keith Busch]

On Sat, May 23, 2026 at 07:45:52AM -0400, Mike Bommarito wrote:
> Add a dev_info_once() at the skip site that names the state, prints
> the actual exit_lat versus the current cap, and points the user at
> the module parameter to raise it.  Using _once keeps the line to a
> single instance per controller; users that want to see every probe
> event can still enable dev_dbg.

The "_once" is not actually once for each controller; it's just once.
Some people have found other similar debug prints to be confusing when
they have multiple controllers since it looks like the seemingly
problematic controller is not consistent across reboots.

Re: [PATCH] nvme: log when APST skips a state due to latency cap

[Michael Bommarito]

On Tue, May 26, 2026 at 9:47 AM Keith Busch <kbusch@kernel.org> wrote:
> The "_once" is not actually once for each controller; it's just once.
> Some people have found other similar debug prints to be confusing when
> they have multiple controllers since it looks like the seemingly
> problematic controller is not consistent across reboots.

Maybe it makes sense to move this back to print every time, but gate
it behind a boot-time parameter so it doesn't spam normal users?

My use case has been debugging bad suspend/hibernate/PM across
different laptops to figure out which device is bad or needs a quirk.
It would be easy enough to boot with verbose=1 or print_apst_skip=1 or
something like that, collect the details needed to fix the hw, then
reboot.

Thanks,
Mike