[PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Martin Belanger]

From: Martin Belanger <martin.belanger@dell.com>

Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
(Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).

Signed-off-by: Martin Belanger <Martin_Belanger@dell.com>


---
 drivers/nvme/host/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2f45e8fcdd7c..bd37ddb172de 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1115,7 +1115,7 @@ EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);

 /*
  * Recommended frequency for KATO commands per NVMe 1.4 section 7.12.1:
- *
+ *
  *   The host should send Keep Alive commands at half of the Keep Alive Timeout
  *   accounting for transport roundtrip times [..].
  */
@@ -4133,6 +4133,7 @@ static void nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
 	case NVME_AER_NOTICE_NS_CHANGED:
 		set_bit(NVME_AER_NOTICE_NS_CHANGED, &ctrl->events);
 		nvme_queue_scan(ctrl);
+		ctrl->aen_result = result;
 		break;
 	case NVME_AER_NOTICE_FW_ACT_STARTING:
 		/*
@@ -4148,6 +4149,7 @@ static void nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
 		if (!ctrl->ana_log_buf)
 			break;
 		queue_work(nvme_wq, &ctrl->ana_work);
+		ctrl->aen_result = result;
 		break;
 #endif
 	case NVME_AER_NOTICE_DISC_CHANGED:
--
2.25.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Chaitanya Kulkarni]

On 4/28/21 13:28, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger@dell.com>
>
> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
> the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
> (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).
>
> Signed-off-by: Martin Belanger <Martin_Belanger@dell.com>
>

Please run checkpatch.pl and resend with the fixes.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Chaitanya Kulkarni]

Also, subject line should start with "nvme:" and not "nvme-fabrics:"
as this code belongs to nvme/host/core.c and not nvme/host/fabrics.c

On 4/28/21 13:28, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger@dell.com>
>
> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
> the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
> (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).
>
> Signed-off-by: Martin Belanger <Martin_Belanger@dell.com>
>
>
> ---
>  drivers/nvme/host/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 2f45e8fcdd7c..bd37ddb172de 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1115,7 +1115,7 @@ EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);
>
>  /*
>   * Recommended frequency for KATO commands per NVMe 1.4 section 7.12.1:
> - *
> + *

Please don't add extra lines.

>   *   The host should send Keep Alive commands at half of the Keep Alive Timeout
>   *   accounting for transport roundtrip times [..].
>   */
> @@ -4133,6 +4133,7 @@ static void nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
>  	case NVME_AER_NOTICE_NS_CHANGED:
>  		set_bit(NVME_AER_NOTICE_NS_CHANGED, &ctrl->events);
>  		nvme_queue_scan(ctrl);
> +		ctrl->aen_result = result;
>  		break;
>  	case NVME_AER_NOTICE_FW_ACT_STARTING:
>  		/*
> @@ -4148,6 +4149,7 @@ static void nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
>  		if (!ctrl->ana_log_buf)
>  			break;
>  		queue_work(nvme_wq, &ctrl->ana_work);
> +		ctrl->aen_result = result;
>  		break;
>  #endif
>  	case NVME_AER_NOTICE_DISC_CHANGED:
> --
> 2.25.1
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
>


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Christoph Hellwig]

On Wed, Apr 28, 2021 at 04:18:25PM -0400, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
> the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
> (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).

So the problem with these events is that they are only really useful
to then issue the Get Log Page command for the corresponding log page.
But reading the log page will clear the contents of the log page.  That
is if userspace races to do this first it will completely mess up the
funtionality in the kernel.

This is a bit of an unfortunately design issue in the NVMe protocol.

Maybe you can explain you use case in a little more detail so that we
can figure out what we can do instead.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Belanger, Martin]

++Vinay Rao, Stuart Hayes.

________________________________________
From: Christoph Hellwig <hch@lst.de>
Sent: Thursday, April 29, 2021 02:30
To: Martin Belanger
Cc: linux-nvme@lists.infradead.org; kbusch@kernel.org; axboe@fb.com; hch@lst.de; sagi@grimberg.me; Belanger, Martin; Belanger, Martin
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL]

On Wed, Apr 28, 2021 at 04:18:25PM -0400, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger@dell.com>
>
> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
> the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
> (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).

So the problem with these events is that they are only really useful
to then issue the Get Log Page command for the corresponding log page.
But reading the log page will clear the contents of the log page.  That
is if userspace races to do this first it will completely mess up the
funtionality in the kernel.

This is a bit of an unfortunately design issue in the NVMe protocol.

Maybe you can explain you use case in a little more detail so that we
can figure out what we can do instead.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Chaitanya Kulkarni]

On 4/29/21 10:09, Belanger, Martin wrote:
> ++Vinay Rao, Stuart Hayes.
>
> ________________________________________
> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, April 29, 2021 02:30
> To: Martin Belanger
> Cc: linux-nvme@lists.infradead.org; kbusch@kernel.org; axboe@fb.com; hch@lst.de; sagi@grimberg.me; Belanger, Martin; Belanger, Martin
> Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs
>
>
> [EXTERNAL EMAIL]
>
> On Wed, Apr 28, 2021 at 04:18:25PM -0400, Martin Belanger wrote:
>> From: Martin Belanger <martin.belanger@dell.com>
>>
>> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and ANA. Today
>> the uevent handler is not capturing on NVME_AER_NOTICE_NS_CHANGED
>> (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).
> So the problem with these events is that they are only really useful
> to then issue the Get Log Page command for the corresponding log page.
> But reading the log page will clear the contents of the log page.  That
> is if userspace races to do this first it will completely mess up the
> funtionality in the kernel.
>
> This is a bit of an unfortunately design issue in the NVMe protocol.
>
> Maybe you can explain you use case in a little more detail so that we
> can figure out what we can do instead.

From the patch that I sent on the same topic to enforce a particular
behavior
either way, I remember that we've decided not to change the kernel code
for usespace.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

+ Madhu

Hi Chaitanya, 

Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 

It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic. 

Vinay

-----Original Message-----
From: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com> 
Sent: Thursday, April 29, 2021 11:52 PM
To: Belanger, Martin; Rao, Vinay; Hayes, Stuart
Cc: Christoph Hellwig; Martin Belanger; linux-nvme@lists.infradead.org; kbusch@kernel.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On 4/29/21 10:09, Belanger, Martin wrote:
> ++Vinay Rao, Stuart Hayes.
>
> ________________________________________
> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, April 29, 2021 02:30
> To: Martin Belanger
> Cc: linux-nvme@lists.infradead.org; kbusch@kernel.org; axboe@fb.com; 
> hch@lst.de; sagi@grimberg.me; Belanger, Martin; Belanger, Martin
> Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace 
> AENs
>
>
> [EXTERNAL EMAIL]
>
> On Wed, Apr 28, 2021 at 04:18:25PM -0400, Martin Belanger wrote:
>> From: Martin Belanger <martin.belanger@dell.com>
>>
>> Generate uevent on NVMe Async Event Notifications for NS_CHANGED and 
>> ANA. Today the uevent handler is not capturing on 
>> NVME_AER_NOTICE_NS_CHANGED (Namespace Changed) and NVME_AER_NOTICE_ANA (Asymmetric Namespace Access).
> So the problem with these events is that they are only really useful 
> to then issue the Get Log Page command for the corresponding log page.
> But reading the log page will clear the contents of the log page.  
> That is if userspace races to do this first it will completely mess up 
> the funtionality in the kernel.
>
> This is a bit of an unfortunately design issue in the NVMe protocol.
>
> Maybe you can explain you use case in a little more detail so that we 
> can figure out what we can do instead.

From the patch that I sent on the same topic to enforce a particular behavior either way, I remember that we've decided not to change the kernel code for usespace.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Keith Busch]

On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
> + Madhu
> 
> Hi Chaitanya, 
> 
> Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
> 
> It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.

The namespace attribute change event pairs with the Changed Namespace
List log, and reading that log will change the result for a subsequent
reader. User space racing with the kernel on a log access when there are
read side effects creates non-deterministic behavior, and that is
problematic.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Christoph Hellwig]

On Fri, Apr 30, 2021 at 09:21:04AM -0700, Keith Busch wrote:
> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
> > + Madhu
> > 
> > Hi Chaitanya, 
> > 
> > Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
> > 
> > It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.
> 
> The namespace attribute change event pairs with the Changed Namespace
> List log, and reading that log will change the result for a subsequent
> reader. User space racing with the kernel on a log access when there are
> read side effects creates non-deterministic behavior, and that is
> problematic.

The only way I could think of making this work is by:

forcing the RAE bit on for all log pages that the kernel cares about,
and only delivering the uevent on controllers that actually do support
the RAE bit (IIRC it got added in 1.3, but I'd have to check).

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

Forcing RAE bit on all the log pages that kernel cares about is one of the option.

Can we also think also think about an another mechanism where the kernel would generate uevent after kernel does a get log page. This would make kernel always to get the get log data first from the controller. 

-----Original Message-----
From: Christoph Hellwig <hch@lst.de> 
Sent: Monday, May 3, 2021 12:23 PM
To: Keith Busch
Cc: Rao, Vinay; Chaitanya Kulkarni; Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Christoph Hellwig; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On Fri, Apr 30, 2021 at 09:21:04AM -0700, Keith Busch wrote:
> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
> > + Madhu
> > 
> > Hi Chaitanya,
> > 
> > Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
> > 
> > It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.
> 
> The namespace attribute change event pairs with the Changed Namespace 
> List log, and reading that log will change the result for a subsequent 
> reader. User space racing with the kernel on a log access when there 
> are read side effects creates non-deterministic behavior, and that is 
> problematic.

The only way I could think of making this work is by:

forcing the RAE bit on for all log pages that the kernel cares about, and only delivering the uevent on controllers that actually do support the RAE bit (IIRC it got added in 1.3, but I'd have to check).




_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Chaitanya Kulkarni]

On 5/3/21 00:21, Rao, Vinay wrote:
> Forcing RAE bit on all the log pages that kernel cares about is one of the option.
>
> Can we also think also think about an another mechanism where the kernel would generate uevent after kernel does a get log page. This would make kernel always to get the get log data first from the controller. 

Based on the comment I've got, please don't top post, most people don't like
it.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Chaitanya Kulkarni]

On 5/3/21 00:21, Rao, Vinay wrote:
> Forcing RAE bit on all the log pages that kernel cares about is one of the option.
>
> Can we also think also think about an another mechanism where the kernel would generate uevent after kernel does a get log page. This would make kernel always to get the get log data first from the controller. 

to read empty log page ?



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

-----Original Message-----
From: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com> 
Sent: Tuesday, May 4, 2021 4:52 AM
To: Rao, Vinay; Christoph Hellwig; Keith Busch
Cc: Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On 5/3/21 00:21, Rao, Vinay wrote:
> Forcing RAE bit on all the log pages that kernel cares about is one of the option.
>
> Can we also think also think about an another mechanism where the kernel would generate uevent after kernel does a get log page. This would make kernel always to get the get log data first from the controller. 

to read empty log page ?

We are interested in these two events NVME_AER_NOTICE_NS_CHANGED and NVME_AER_NOTICE_ANA. 
-- NVME_AER_NOTICE_NS_CHANGED event would require the host software to do an nvme identify namespace to see the change in the namespace. 
-- NVME_AER_NOTICE_ANA event would require the host software to do a ANA LOG page command to get the ANA log page from the controller. Even if the ANA log page is sent from userland after the kernel sends it first the controller would still return the ANA log page. It will not return an empty log page. 



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

-----Original Message-----
From: Rao, Vinay 
Sent: Tuesday, May 4, 2021 2:03 PM
To: Chaitanya Kulkarni; Christoph Hellwig; Keith Busch
Cc: Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs



-----Original Message-----
From: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com> 
Sent: Tuesday, May 4, 2021 4:52 AM
To: Rao, Vinay; Christoph Hellwig; Keith Busch
Cc: Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On 5/3/21 00:21, Rao, Vinay wrote:
> Forcing RAE bit on all the log pages that kernel cares about is one of the option.
>
> Can we also think also think about an another mechanism where the kernel would generate uevent after kernel does a get log page. This would make kernel always to get the get log data first from the controller. 

to read empty log page ?

We are interested in these two events NVME_AER_NOTICE_NS_CHANGED and NVME_AER_NOTICE_ANA. 
-- NVME_AER_NOTICE_NS_CHANGED event would require the host software to do an nvme identify namespace to see the change in the namespace. 
-- NVME_AER_NOTICE_ANA event would require the host software to do a ANA LOG page command to get the ANA log page from the controller. Even if the ANA log page is sent from userland after the kernel sends it first the controller would still return the ANA log page. It will not return an empty log page. 

Any comments on my last email? 


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

-----Original Message-----
From: Christoph Hellwig <hch@lst.de> 
Sent: Monday, May 3, 2021 12:23 PM
To: Keith Busch
Cc: Rao, Vinay; Chaitanya Kulkarni; Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Christoph Hellwig; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On Fri, Apr 30, 2021 at 09:21:04AM -0700, Keith Busch wrote:
> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
> > + Madhu
> > 
> > Hi Chaitanya,
> > 
> > Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
> > 
> > It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.
> 
> The namespace attribute change event pairs with the Changed Namespace 
> List log, and reading that log will change the result for a subsequent 
> reader. User space racing with the kernel on a log access when there 
> are read side effects creates non-deterministic behavior, and that is 
> problematic.

The only way I could think of making this work is by:

forcing the RAE bit on for all log pages that the kernel cares about, and only delivering the uevent on controllers that actually do support the RAE bit (IIRC it got added in 1.3, but I'd have to check).

>> Just wanted to check if there is any plan to implement the code change of forcing the RAE bit for all log pages that the kernel cares about is taken up?  Can you please let us know what the plan is to address the problem. 

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Hannes Reinecke]

On 5/3/21 8:52 AM, Christoph Hellwig wrote:
> On Fri, Apr 30, 2021 at 09:21:04AM -0700, Keith Busch wrote:
>> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
>>> + Madhu
>>>
>>> Hi Chaitanya, 
>>>
>>> Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
>>>
>>> It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.
>>
>> The namespace attribute change event pairs with the Changed Namespace
>> List log, and reading that log will change the result for a subsequent
>> reader. User space racing with the kernel on a log access when there are
>> read side effects creates non-deterministic behavior, and that is
>> problematic.
> 
> The only way I could think of making this work is by:
> 
> forcing the RAE bit on for all log pages that the kernel cares about,
> and only delivering the uevent on controllers that actually do support
> the RAE bit (IIRC it got added in 1.3, but I'd have to check).
> 
Yep, that sounds like a good idea.
I would even go so far as to inhibit any user-space commands which read
those log pages and which have the RAE bit _not_ set, as this has the
potential of messing up the kernel operations...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Hannes Reinecke]

On 5/3/21 8:52 AM, Christoph Hellwig wrote:
> On Fri, Apr 30, 2021 at 09:21:04AM -0700, Keith Busch wrote:
>> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
>>> + Madhu
>>>
>>> Hi Chaitanya, 
>>>
>>> Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
>>>
>>> It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating this above to user space is problematic.
>>
>> The namespace attribute change event pairs with the Changed Namespace
>> List log, and reading that log will change the result for a subsequent
>> reader. User space racing with the kernel on a log access when there are
>> read side effects creates non-deterministic behavior, and that is
>> problematic.
> 
> The only way I could think of making this work is by:
> 
> forcing the RAE bit on for all log pages that the kernel cares about,
> and only delivering the uevent on controllers that actually do support
> the RAE bit (IIRC it got added in 1.3, but I'd have to check).
> 

... and you are correct, it went into the base spec into 1.3.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Hannes Reinecke]

On 4/30/21 6:21 PM, Keith Busch wrote:
> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
>> + Madhu
>>
>> Hi Chaitanya, 
>>
>> Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
>>
>> It would be good to consider these events to be propagated up to user space. I am not yet convinced on how propagating
>> this above to user space is problematic.
> 
> The namespace attribute change event pairs with the Changed Namespace
> List log, and reading that log will change the result for a subsequent
> reader. User space racing with the kernel on a log access when there are
> read side effects creates non-deterministic behavior, and that is
> problematic.
> 
It would, if we were relying on the Changed Namespace List log.
But in the end we're just reading it to clear the AEN flag; the contents
are completely immaterial to us as we're doing a full namespace scan
afterwards anyway.
So I don't think that that'll be a problem for us.

What will be problematic, though, is if we were to allow userspace to
register an AEN; as soon as the controller allows more than one pending
AERs there nothing preventing it from doing so.
If he does our internal states will be messed up as we're not receiving
all events. I'd rather error out any attempt here, but then we can't do
a full passthrough, and I guess things like SPDK might become unhappy.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs

[Rao, Vinay]

-----Original Message-----
From: Hannes Reinecke <hare@suse.de> 
Sent: Thursday, June 24, 2021 4:19 PM
To: Keith Busch; Rao, Vinay
Cc: Chaitanya Kulkarni; Belanger, Martin; Hayes, Stuart; Tarikere, Madhu; Christoph Hellwig; Martin Belanger; linux-nvme@lists.infradead.org; axboe@fb.com; sagi@grimberg.me
Subject: Re: [PATCH 1/1] nvme-fabrics: Generate uevent on namespace AENs


[EXTERNAL EMAIL] 

On 4/30/21 6:21 PM, Keith Busch wrote:
> On Fri, Apr 30, 2021 at 05:14:55AM +0000, Rao, Vinay wrote:
>> + Madhu
>>
>> Hi Chaitanya,
>>
>> Some of the AEN's like ANA state change and Name Space attribute change events are of interest outside the NVME driver. 
>>
>> It would be good to consider these events to be propagated up to user 
>> space. I am not yet convinced on how propagating this above to user space is problematic.
> 
> The namespace attribute change event pairs with the Changed Namespace 
> List log, and reading that log will change the result for a subsequent 
> reader. User space racing with the kernel on a log access when there 
> are read side effects creates non-deterministic behavior, and that is 
> problematic.
> 
It would, if we were relying on the Changed Namespace List log.
But in the end we're just reading it to clear the AEN flag; the contents are completely immaterial to us as we're doing a full namespace scan afterwards anyway.
So I don't think that that'll be a problem for us.

What will be problematic, though, is if we were to allow userspace to register an AEN; as soon as the controller allows more than one pending AERs there nothing preventing it from doing so.
If he does our internal states will be messed up as we're not receiving all events. I'd rather error out any attempt here, but then we can't do a full passthrough, and I guess things like SPDK might become unhappy.
> Wanted to check if there is a timeline by when we can expect the AEN's to be propagated to user space. 


Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme