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

[Andrew Morton <akpm@linux-foundation.org>]

On Tue, 10 Nov 2026 23:15:13 +0530 Vishal Badole <Vishal.Badole@amd.com> 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.
> 
> ...
>
> Fix this by snapshotting cluster and SMT masks together under
> cpus_read_trylock() when all present CPUs are online. Latch the first
> capture attempt, permanently using CPU-order grouping if it fails, and
> propagate later allocation errors instead of switching algorithms. This
> keeps successful spreads stable across CPU offline/online cycles for an
> unchanged group count, present/possible CPU sets and NUMA topology.
> 

Thanks, I grabbed this and I'll push it into -next for testing.

It's hard to get review for group_cpus.c.  Wangyang, if you have time
please.


fwiw, I fed it into my evolving AI patch reviewbot and it liked it.  We
already have Sashiko - I'm trying to get this thing to take a broader,
design-level view using wider resources (commit history, mailing list
discussions, previous iterations, similar/related patches, related bug
reports, etc).

: The bug is a mismatch between two inputs to IRQ grouping. 
: group_cpus_evenly() assigns groups across present and possible CPUs,
: but the live cluster and SMT masks change as CPUs go offline.  When
: NVMe reallocates its IRQs with CPUs 311 offline, the same queue count
: can therefore produce a different CPU partition from the one
: established at probe.
: 
: The patch makes that partition stable.  On the first grouping call, it
: either copies both masks while all present CPUs are online, or
: permanently selects CPU-order grouping.  The failure path uses
: cpumask_of(cpu) rather than a live SMT mask, so it cannot change its
: answer on a later call.  The mutex and release/acquire publication
: protect the one-time decision; the captured masks remain allocated for
: the kernels lifetime.
: 
: V7s specific change is sound: a positive ncluster means cluster
: grouping succeeded, zero selects CPU-order grouping, and a negative
: allocation error unwinds.  The caller adds nv->ngroups once on either
: successful path.  I found no remaining ownership, cleanup, or
: arithmetic defect in those changes.
: 
: The tradeoff is explicit: if the first capture occurs with an offline
: CPU, under hotplug-lock contention, or during allocation failure,
: cluster-aware grouping is lost for that boot.  The stability claim also
: assumes unchanged group count, present/possible CPU sets, and NUMA
: topology; CPU hot-add is outside it.


And I fed it into my evolving LLM-driven live kernel testing setup. 
Everything passed.  This thing is still being beaten into shape - the
test planning isn't as nasty as I've told it to be, because it's
annoyingly lazy and disobedient and forgetful.  It's a work in
progress.

But I think it gave the patch a pretty good workout anyway, wdyt?


: Patch "lib/group_cpus: snapshot cluster masks to keep grouping hotplug invariant"
: Link: https://lore.kernel.org/20261110174513.461743-1-Vishal.Badole@amd.com
: Tested-by: Andrew Morton <akpm@linux-foundation.org>
: 
: I tested v7 against the same tree with this one patch reversed. Both
: kernels had the same configuration and booted a Debian guest with 12
: vCPUs (one socket, six cores, SMT2, one virtual NUMA node). The host
: was a 64-core, SMT2 Threadripper PRO 3995WX with one NUMA node. The
: kernel was 7.4.0-rc1-g99b6d17012de, with CPU hotplug, NVMe, KASAN and
: lock checking enabled. QEMU 8.2.2 supplied an eight-I/O-queue NVMe
: device and a separate root disk.
: 
: Both kernels initially returned the same eight group_cpus_evenly(8)
: masks. I offlined CPUs 3-11 while the present and possible masks stayed
: at 0-11. All eight baseline groups changed, while all eight v7 groups
: remained unchanged. For example, baseline group 0 moved from CPUs 2-3
: to 0-1; v7 group 0 stayed at 2-3. Reonline restored the original masks.
: The guest exposed six two-CPU cluster masks and SMT sibling pairs.
: 
: I reset NVMe while CPUs 3-11 were offline. The controller retained
: nine queues (one admin and eight I/O) in both kernels. All eight I/O
: queue managed IRQ masks changed across the baseline reset; none changed
: with v7. Effective IRQ affinity moved onto online CPUs as expected.
: After bringing the CPUs back, direct I/O from CPU 8 succeeded and the
: 8 MiB readback SHA-256 matched on both kernels. There were no NVMe
: command timeouts or kernel warnings in these guest runs.
: 
: For the fallback path, I booted v7 with only CPUs 0,2,4 online.
: Because some CPUs were offline during the first grouping call, v7 did
: not create a topology snapshot. It used the same CPU-order groups for
: that call and every later call, even after the other CPUs came online.
: Stack-filtered slab injection in a separate v7 build failed two
: distinct outer and three distinct inner grouping allocations. Each
: call returned NULL; a subsequent normal call returned the original
: eight groups without a warning.
: 
: A separate diagnostic build used a temporary boot-selectable hook to
: force failure of the first snapshot array, sibling array, a per-CPU
: mask after earlier allocations, and the trylock-failure cleanup
: branch. A no-failure control captured a non-null snapshot. In every
: forced-failure case, v7 selected CPU-order grouping for the rest of
: the boot. Its snapshot pointer stayed NULL, the groups remained the
: same through CPU hotplug, and there was no KASAN or lock warning. The
: trylock branch was forced by the hook; actual contention on the CPU
: hotplug lock was not reproduced. QEMU did not reproduce the physical
: controller's s2idle simple-suspend quirk or its reported two-minute
: timeout. The A/B tests reproduced the changing affinity-map mechanism,
: not that hardware stall.