Issue with AWUPF when using multiple controllers in a subsystem

[Chaitanya Kulkarni]

On 4/9/25 02:05, Christoph Hellwig wrote:
> On Fri, Apr 04, 2025 at 03:42:50PM -0700, alan.adamson at oracle.com wrote:
>> When dumping the awupf value with nvme-cli, the values match what was setup
>> in qemu, but when each devices atomic queue limits was displayed, it didn't
>> match its awupf. Currently the awupf is saved in the nvme_subsystem, but the
>> awupf is specific to the controller, not the subsystem. Two controllers in a
>> subsystem can have different atomic parameters.
>>
>> The commit 81adb8633491 ("nvme: set physical block size and optimal I/O
>> size") added subsys->awupf.
>>
>> I think the awupf belongs in the nvme_ctrl structure.
> No, that would be extremely dangerous.  But as NVMe has not explicit
> language to forbid different values for different controllers (even
> if that is really stupid) we'll need to reject adding controllers
> that have lower than the initial value to not break setups.
>
> Sigh..  I'll also reach out to NVMe to see if we can ECN this, but
> we'll still need to work around this.

I wonder how many of these fields suffering from similar scope issue,
hope we could fix that in one ECN if there other fields suffering from
this issues.

> Alternatively we could stop supporting AWUPF entirely and require
> NAWUPF which must be the same for all attachments of the controller.
>
>

if we stop supporting AWUPF and move to NAWUPF then will it break where
exiting controller's that doesn't support NAWUPF but only supports
controller wide AWUPF, IOW NSFEAT bit 1 == 0 meaning per-namespace
atomic info is not valid ? [1]

Looking at the spec NAWUPF values can differ for namespaces attached
to the same controller and those values can also differ from
AWUPF ? [2]

I couldn't find anything where it states these values must be the same.

Perhaps using the minimum of all AWUPF and rejecting any ctrl which are
lower than the initial value is right fix ?

-ck

[1]

nvme_update_disk_info()

...

2056         /*
2057          * Bit 1 indicates whether NAWUPF is defined for this namespace
2058          * and whether it should be used instead of AWUPF. If NAWUPF ==
2059          * 0 then AWUPF must be used instead.
2060          */
2061         if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf)
2062                 atomic_bs = (1 + le16_to_cpu(id->nawupf)) * bs;
2063         else
2064                 atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
2065

...

[2]

NVM Express NVM Command Set Specification, Revision 1.1 (2024.08.05)
Section: |2.1.4.3 AWUPF/NAWUPF*:-
*
|An NVM subsystem may report per namespace values for
these atomicity parameters that are specific to the namespace and
are indicated in the Identify Namespace data structure..."

"... If an NVM subsystem reports a per namespace value, then that
value shall be *greater than or equal to the corresponding baseline
value* indicated in the Identify Controller data structure
data structure (refer to Figure 117)"/

/My interpretation :-

The spec *allows different values* for:
|AWUPF| (controller-level baseline)
|NAWUPF| (namespace-specific override)
The only requirement is:
|||NAWUPF ≥ AWUPF
|There is *no mandate* that |NAWUPF == AWUPF


|