Re: [PATCH] lib/group_cpus: Snapshot cluster masks to keep grouping hotplug invariant

On Fri, 7 Aug 2026 13:06:12 +0530 Vishal Badole <[email protected]> wrote:

> group_cpus_evenly() builds the managed-IRQ affinity spread used by
> multi-queue devices such as NVMe. That spread is meant to be a property
> of the static CPU topology: it walks cpu_present_mask and then
> cpu_possible_mask so every hardware queue owns a fixed set of CPUs,
> including CPUs that are offline at the time. A driver depends on that
> partition staying stable across re-computation - the CPUs a queue is
> given at probe must still describe the same queue after the device is
> later reset and its affinity recomputed.
> 
> On an AMD system that stability breaks across an s2idle cycle. With CPUs
> 3-11 offlined and only CPUs 0-2 left online, the machine is suspended to
> s2idle and resumed. The NVMe controller uses the simple-suspend quirk, so
> resume fully re-initialises it and recomputes the affinity spread. The
> system then hangs for roughly two minutes and stays sluggish afterwards,
> the controller only making progress through its command-timeout poll:
> 
>   nvme nvme0: I/O tag 898 (3382) QID 9 timeout, completion polled
>   nvme nvme0: I/O tag 398 (618e) QID 11 timeout, completion polled
> 
> QID 9 and QID 11 are the queues whose CPUs were offline when the spread
> was recomputed. "completion polled" means the commands did finish in
> hardware, but their interrupts were never delivered to a CPU that was
> watching the queue, so nothing reaped them until the timeout fired.
> 
> It happens because commit 89802ca36c96 ("lib/group_cpus: make group CPU
> cluster aware") derives the cluster groups from topology_cluster_cpumask(),
> which lists only the cluster siblings that are online when it is called.
> The resulting partition therefore depends on the transient online mask
> rather than on the topology alone. Recomputed on resume while the non-boot
> CPUs are still offline, it no longer matches the boot-time partition, and a
> queue is left with an affinity that does not cover the CPU it is meant to
> serve once that CPU comes back online. The dependence is on the online
> mask, not on any AMD-specific behaviour, so the same stall is reproducible
> on Intel platforms as well.

Thanks for the careful description of what is wrong.  It helps.

And things do sounds very wrong.  Could people@intel please prioritize
their review and testing of this fix?

> Make the cluster grouping depend on the complete cluster topology rather
> than on whichever CPUs happen to be online. Snapshot the cluster masks
> once while every CPU is online and reuse that view for every later spread.
> A fully-online view is the complete cluster membership, so the partition
> derived from it is identical no matter which CPUs are online when the
> controller is reset, which is exactly the invariance the callers already
> assume.
> 
> ...
>
> +	/*
> +	 * Only a fully-online view is the complete cluster membership; if any

minor: the above sentence is hard to understand.

> +	if (!data_race(cpumask_equal(cpu_possible_mask, cpu_online_mask)))

AI review suggests using cpu_present_mask here:

	https://sashiko.dev/#/patchset/[email protected]