From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: "Knight, Frederick" To: Christoph Hellwig , "Popuri, Sriram" CC: "linux-nvme@lists.infradead.org" , "Jens Axboe" , Keith Busch , "linux-block@vger.kernel.org" , Sagi Grimberg , Hannes Reinecke Subject: RE: [PATCH 13/14] nvme: use the changed namespaces list log to clear ns data changed AENs Date: Sat, 26 May 2018 13:02:51 +0000 Message-ID: References: <20180526102735.31404-1-hch@lst.de> <20180526102735.31404-14-hch@lst.de> <20180526122137.GA21257@lst.de> In-Reply-To: <20180526122137.GA21257@lst.de> Content-Type: multipart/alternative; boundary="_000_DM6PR06MB38337075BFA7E11BC2D9BA92F1680DM6PR06MB3833namp_" MIME-Version: 1.0 Return-Path: Frederick.Knight@netapp.com List-ID: --_000_DM6PR06MB38337075BFA7E11BC2D9BA92F1680DM6PR06MB3833namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Section 5.2: Asynchronous events are grouped into event types. The event type informatio= n is indicated in Dword 0 of the completion queue entry for the Asynchronou= s Event Request command. 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. An event is clea= red by reading the log page associated with that event using the Get Log Pa= ge command (refer to section 5.14). The event is cleared by reading the log page associated with that event ...= I don't know how the event can be cleared if the associated log page is = not supported (the masking of subsequent events of that type would turn it = into a one-shot). Interesting however, in the list (that reports "the foll= owing event types are defined"), there are several of the defined events mi= ssing. Looks like an ECN opportunity to flush out that list of defined eve= nt types. In addition, how is this bit set: 95:92 M Optional Asynchronous Events Supported (OAES): This field i= ndicates the optional asynchronous events supported by the controller. A co= ntroller shall not send optional asynchronous events before they are enable= d by host software. Bits 31:10 are reserved. Bit 9 is set to '1' if the controller supports sending Firmware Activation = Notices. If cleared to '0', then the controller does not support the Firmwa= re Activation Notices event. Bit 8 is set to '1' if the controller supports sending Namespace Attribute = Notices and the associated Changed Namespace List log page. If cleared to '= 0', then the controller does not support the Namespace Attribute Notices ev= ent nor the associated Changed Namespace List log page. Bits 7:0 are reserved. Not sure how you set that bit if you support the AEN but not the log page. Fred -----Original Message----- From: Christoph Hellwig Sent: Saturday, May 26, 2018 8:22 AM To: Popuri, Sriram Cc: Christoph Hellwig ; linux-nvme@lists.infradead.org; Knight,= Frederick ; Jens Axboe ; Kei= th Busch ; linux-block@vger.kernel.org; Sagi Grimber= g ; Hannes Reinecke Subject: Re: [PATCH 13/14] nvme: use the changed namespaces list log to cle= ar ns data changed AENs On Sat, May 26, 2018 at 12:05:02PM +0000, Popuri, Sriram wrote: > Reading the spec it looks like ns log is alternate approach: > > "Namespace Attribute Changed: The Identify Namespace data structure for o= ne or more namespaces, as well as the Namespace List returned when the Iden= tify command is issued with the CNS field set to 02h, have changed. Host so= ftware may use this event as an indication that it should read the Identify= Namespace data structures for each namespace to determine what has changed= . > > Alternatively, host software may request the Changed Namespace List (Log = Identifier 04h) to determine which namespaces in this controller have chang= ed Identify Namespace information since the last time the log page was read= ." > > My question is: Does the target needs to implement log page 04h as this i= s an optional log page and the text above suggests its used in alternate wa= y? Section 5.2 clearly states that each AER comes with a log page to clear it, and the above does not contain any language to explicitly override it. So my reading of the spec is that to clear the event you need to read the log page, but to find out what has changed you might issue Identify commands instead. > If target is not required to implement, then I guess your change should s= till work because if log page 04h fails, it fails back to rescan. Correct? Independ of what the spec actually requires, the code tries to robust as possible and falls back to doing the manual scan. > I think you can optimize this by checking the "Log Page Identifier" field= in your result and accordingly setting EVENT_NS_CHANGED. I assume target w= ould clear this if log page 04h is not supported. We can try that, but I'm not sure what it is going to buy us. --_000_DM6PR06MB38337075BFA7E11BC2D9BA92F1680DM6PR06MB3833namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
Section 5.2:
 
Asynchronous events are grouped into event types. The event type in= formation is indicated in Dword 0 of the completion queue entry for the Asy= nchronous Event Request command. When the controller posts a completion que= ue 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. An event is cleared by reading the log page associated with that= event using the Get Log Page command (refer to section 5.14).
 
The event is cleared by reading the log page a= ssociated with that event ...   I don't know how the event can be= cleared if the associated log page is not supported (the masking of subseq= uent events of that type would turn it into a one-shot). Interesting however, in the list (that reports “the fol= lowing event types are defined”), there are several of the defined ev= ents missing.  Looks like an ECN opportunity to flush out that list of= defined event types.
 
In addition, how is this bit set:
 
95:92 M Optional Asynchronous Events Suppo= rted (OAES): This field indicates the optional asynchronous events supp= orted by the controller. A controller shall not send optional asynchronous events before they are enabled by host softw= are.=20
Bits 31:10 are reserve= d.
Bit 9= is set to ‘1’ if the controller supports sending Firmware Acti= vation Notices. If cleared to ‘0’, then the controller does not= support the Firmware Activation Notices event.
Bit 8 is set to ‘= ;1’ if the controller suppor= ts sending Namespace Attribute Notices and the associated Changed Namespace List log page. If cleared to ‘0’, then the control= ler does not support the Na= mespace Attribute Notices event nor the associated Changed Namespace List log page.
Bits = 7:0 are reserved.
 
Not sure how you set that bit if you support t= he AEN but not the log page.
 
        Fre= d
 
-----Original Message-----
From: Christoph Hellwig <hch@lst.de>
Sent: Saturday, May 26, 2018 8:22 AM
To: Popuri, Sriram <Sriram.Popuri@netapp.com>
Cc: Christoph Hellwig <hch@lst.de>; linux-nvme@lists.infradead.org; K= night, Frederick <Frederick.Knight@netapp.com>; Jens Axboe <axboe@= kernel.dk>; Keith Busch <keith.busch@intel.com>; linux-block@vger.= kernel.org; Sagi Grimberg <sagi@grimberg.me>; Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH 13/14] nvme: use the changed namespaces list log to cle= ar ns data changed AENs
 
On Sat, May 26, 2018 at 12:05:02PM +0000, Popuri, Sriram wrote:
> Reading the spec it looks like ns log is alternate approach:
>
> "Namespace Attribute Changed: The Identify Namespace data st= ructure for one or more namespaces, as well as the Namespace List returned = when the Identify command is issued with the CNS field set to 02h, have cha= nged. Host software may use this event as an indication that it should read the Identify Namespace data structures fo= r each namespace to determine what has changed.
>
> Alternatively, host software may request the Changed Namespace Li= st (Log Identifier 04h) to determine which namespaces in this controller ha= ve changed Identify Namespace information since the last time the log page = was read."
>
> My question is: Does the target needs to implement log page 04h a= s this is an optional log page and the text above suggests its used in alte= rnate way?
 
Section 5.2 clearly states that each AER comes with a log page to
clear it, and the above does not contain any language to explicitly
override it. So my reading of the spec is that to clear the event you = need
to read the log page, but to find out what has changed you might issue=
Identify commands instead.
 
> If target is not required to implement, then I guess your change = should still work because if log page 04h fails, it fails back to rescan. C= orrect?
 
Independ of what the spec actually requires, the code tries to robust<= /div>
as possible and falls back to doing the manual scan.
 
> I think you can optimize this by checking the "Log Page Iden= tifier" field in your result and accordingly setting EVENT_NS_CHANGED.= I assume target would clear this if log page 04h is not supported.
 
We can try that, but I'm not sure what it is going to buy us.
 
 --_000_DM6PR06MB38337075BFA7E11BC2D9BA92F1680DM6PR06MB3833namp_--