[PATCH] nvme: Remove namespace when nvme_identify_ns_descs() failed

[Nilay Shroff]

On 12/6/24 6:11 PM, Hannes Reinecke wrote:
> On 12/5/24 17:15, Keith Busch wrote:
>> On Thu, Dec 05, 2024 at 01:30:39PM +0100, Hannes Reinecke wrote:
>>> On 12/4/24 17:39, Keith Busch wrote:
>>>>> 1) AEN triggers a rescan
>>>>> 2) List of active namespace is retrieved
>>>>> -> NSID A gets unmapped (or moved to another node in the cluster)
>>>>> 3) Scan of NSID A returns an error with DNR set.
>>>>> Without this patch we keep the namespace around, so eventually we'll
>>>>> trip over the 'non-matching UUID' check once the NSID is reused.
>>>>
>>>> I'm still not sure that makes sense. The target shouldn't attach the new
>>>> namespace until the host acknowledges the removal of the older NSID via
>>>> the Namespace Change List log. Until the log is read, the inventory for
>>>> removed namespaces should be latched. Otherwise, timing might remove+add
>>>> a specific NSID before the host requests the NS Descriptor for the
>>>> racing removal, then it would just get the "non-matching UUID" issue
>>>> anyway.
>>>
>>> But we read the Namespace Change List log in step 2)
>>> (Not that we're doing anything with it, but that's another story...)
>>> Hmm?
>>
>> Indeed. So maybe we should just move the log page retrevial *after* we
>> scan the identify active namespace list processing?
> 
> Not sure how that would help. We are getting an 'ANA inaccessible' with DNR set status when retrieving the NS descriptor list for the namespace.
> And this has to happen after we read the list of active namespace.
> Perfectly legit, but doesn't tell us anything if the namespace is present at all.
> All we know is that we cannot get information about that, and my argument is that we should treat this as equivalent to a namespace
> not present.
> 
I think when a nsid is in "ANA inaccessible" state sending any command which 
has that nsid described in it would be aborted by the controller. 
Per the NVMe 2.0  spec (quoting a snippet from section 8.1.3.3 ANA 
Inaccessible state):

"A controller shall abort commands, other than those described in section 8.1.4, with a status code of
Asymmetric Access Inaccessible if those commands are submitted while the relationship between the
namespace specified by the command and the controller processing the command is in this state.

While ANA Inaccessible state is reported by a controller for the namespace, the host should retry the
command on a different controller that is reporting ANA Optimized state or ANA Non-Optimized state. If no
controllers are reporting ANA Optimized state or ANA Non-Optimized state, then a transition may be
occurring such that a controller reporting the Inaccessible state may become accessible and the host should
retry the command on the controller reporting Inaccessible state for at least ANATT seconds (refer to Figure
275). Refer to section 8.10.2."

So as we can see above, removing nsid immediately just because ns-descriptor-list command
failed with status "ANA inaccessible and DNR set" may not be correct. Because it's possible
that ANA state may transition back to optimized/non-optimized state, So instead of removing
ns from host, we may retry that command on another controller which is reporting ANA optimized/
non-optimized state if that nsid is attached to more than one controller. If nsid is private
(means attached only to one controller) then we may not have any option but to skip this nsid
during scan and wait until either ANATT timer expires or nsid transition back from ANA 
inaccessible to ANA optimized/non-optimized state. 

Yes it might be possible that while nsid is in ANA inaccessible state, it might be un-mapped 
from the target controller. But in that case target should send namespace change notice to the 
host and that shall trigger ns scan. And as Keith proposed, we probably want to move the changed
log ns retrieval just after we get active list of ns.

Thanks,
--Nilay