[PATCH] nvme-core: update NS Attr Changed AEN handling for ANA group

[Knight, Frederick]

Attached is the proposed clarification.  This has had ZERO review time in the NVMe committee.  So just FYI at this point.

Feel free to share thoughts.

	Fred

-----Original Message-----
From: Sagi Grimberg <sagi at grimberg.me> 
Sent: Wednesday, January 13, 2021 10:09 PM
To: Keith Busch <kbusch at kernel.org>; Hannes Reinecke <hare at suse.de>
Cc: hch at lst.de; Knight, Frederick <Frederick.Knight at netapp.com>; Meneghini, John <John.Meneghini at netapp.com>; George, Martin <Martin.George at netapp.com>; linux-nvme at lists.infradead.org; Hannes Reinecke <hare at suse.com>
Subject: Re: [PATCH] nvme-core: update NS Attr Changed AEN handling for ANA group

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.




Hey all,

Joining late to the discussion, ramping up on an old old backlog...

>>> On Wed, Dec 09, 2020 at 01:34:37PM -0800, Keith Busch wrote:
>>>> Linux hosts currently want an ANA AEN anytime the host needs to 
>>>> refresh the ANA log. That includes any condition that adds groups 
>>>> that didn't exist from the host's previous reading.
>>>>
>>>> If the Namespace Attach occurs to an ANA group the host already 
>>>> knows about, then you don't need to send an ANA AEN because there's 
>>>> nothing new in the log that the host requires. You just need to 
>>>> send the NS Notify AEN.
>>>>
>>>> But if a side effect of attaching a namespace results in a new ANA 
>>>> group becoming visible to the host, then that group creation is 
>>>> considered a separate event, so the host wants both AENs. I believe 
>>>> this is where Christoph is trying to steer the interpretation and the text.
>>>
>>> Exactly.  Although as Hannes pointed out, this language:
>>>
>>>     If, for an ANA Group, there are no namespaces attached to the
>>>     controller processing the command, then no ANA Group Descriptor is
>>>     returned for that ANA Group (i.e., an ANA Group Descriptor is returned
>>>     only if that ANA Group contains namespaces that are attached to the
>>>     controller processing the command.
>>>
>>> creates a bit of a chicken an egg problem in that case of a newly 
>>> attached namespace that is the first one referencing an ANA group.

FWIW, the subsystems I'm familiar with are sending both events in this case and the interpretation of this paragraph was that although the ANA group existed since day-0, the host was never informed about it, so just like lots of other internal resources, it doesn't exist from an interface perspective.

The alternative would have been to tell the host about every ANA group that the subsystem is managing, which could be a very large number and just creates a churn of completely useless huge log pages.

>> Yes, this was what I tried to outline.
>>
>> Personally I don't mind whether the controller is required to send an 
>> ANA change AEN together with the NS change AEN.
>> Requiring both will pose an ordering issue (as on fabrics we can't 
>> control the completion ordering), so effectively we'll have to 
>> implement handling for AENs arriving in any order.
>
> I was thinking more about this, and it's actually not a problem based 
> on how the controller is allowed to send AEN completions:
>
>    When the controller posts a completion queue entry for an outstanding
>    Asynchronous Event Request command and thus reports an asynchronous
>    event, subsequent events of that event type are automatically masked by
>    the controller until the host clears that event.
>
> These are both "Notice" event types, so the controller is not allowed 
> to send both at the same time. The host must acknowledge the first 
> before the second can be seen, so we can enforce a deterministic sequence.

Well that is easy today because there is only one aen at a time...

> And while it may make sense if the first event seen is the ANA one 
> before the NS, it doesn't look like Linux cares if we observe them the 
> other way around. The only thing the driver needs to do is synchronize 
> the work queues for each event.

Yes, the order doesn't matter and we should keep it that way...

>> But I'm also fine with just getting one AEN; the patch will solve 
>> that situation just fine.
>
> I'm also okay with that patch, too.

I would actually be in favor of this patch, would be nice to skip sending duplicate events for this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: NVMe - NVMe 1.4 ECN-008 2021.01.13.docx
Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document
Size: 81284 bytes
Desc: NVMe - NVMe 1.4 ECN-008 2021.01.13.docx
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20210114/dd699af9/attachment-0001.docx>