Questions on the correct Asynchronous Event Request Interface

Questions on the correct Asynchronous Event Request Interface

[INDRANEEL MUKHERJEE]

A couple of patches have already been posted on the Asynchronous Event Request(AER) admin command.
But there is still no clarity on the what is the correct interface to the application.

Here'r some questions to help summarise the requirements & finalise an interface so that a patch can follow.
(References to kfifo are based on Keith's patch - http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html )

1. Should the Driver return both the Async Event Completion & the corresponding GetLogPage together to User Space?
This can help to avoid the Async Event getting masked accidently. It's possible that any Application reads the Async Event from kfifo but fails to read the corresponding Log Page masking the event indefinitely.

2. How to have provision in driver to set the 'Temperature Threshold' & 'Asynchronous Event Configuration' before sending the AER command?
This is needed as Spec says these should be set before the Async Event Request command is sent.
One way to achieve this is by making these module parameters and doing a SetFeature during Probe & Resume operations. If these module parameters are provided by user during insmod, driver should ensure that these are set before the kthread submits the AER.

3. Should the contents kfifo be preserved when the system goes into Suspend?

4. Only one application will get any event (kfifo ensures this). Is this expected?

5. How to have provision in the driver to cancel the current AER cmds and send new ones after doing some new Set Feature?
This will need the CmdIds for current AREs to be exported to User Space so that they can be aborted. Should the Driver support this?

6. Is the IOCtl interface enough? Or we want to keep the Read/Poll interface?

7. Is there any reference utility that can be looked at to see what such monitoring applications expect?

-Indro

Questions on the correct Asynchronous Event Request Interface

[Keith Busch]

On Wed, 11 Jun 2014, INDRANEEL MUKHERJEE wrote:
> A couple of patches have already been posted on the Asynchronous Event
> Request(AER) admin command.  But there is still no clarity on the what is the
> correct interface to the application.
>
> Here'r some questions to help summarise the requirements & finalise an
> interface so that a patch can follow.  (References to kfifo are based on
> Keith's patch -
> http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html )
>
> 1. Should the Driver return both the Async Event Completion & the
> corresponding GetLogPage together to User Space?
>
> This can help to avoid the Async Event getting masked accidently. It's
> possible that any Application reads the Async Event from kfifo but fails to
> read the corresponding Log Page masking the event indefinitely.

I don't think so. The driver has no gaurantee there is even a user
space app consuming these events. If the driver is unmasking them,
we could continue getting events and overrun older ones.

> 2. How to have provision in driver to set the 'Temperature Threshold' &
> 'Asynchronous Event Configuration' before sending the AER command?
>
> This is needed as Spec says these should be set before the Async Event
> Request command is sent.

Does it? Please point me to the section in the spec, because I'm not
finding it in either 5.2 or 5.12.1.11.

Section 5.12.1.4 for Temperature Threshold feature says:

"The host should configure this feature prior to enabling asynchronous
event notification for the temperature exceeding the threshold".

I take that to mean the host should set the asynchronous event
configuration feature to enable temperature notification after you set
the temperature threshold. All this can be done from user space and async
event requests may be outstanding prior to this without affecting it.

> One way to achieve this is by making these module parameters and doing a
> SetFeature during Probe & Resume operations. If these module parameters are
> provided by user during insmod, driver should ensure that these are set
> before the kthread submits the AER.

What if I have different devices with vastly different temperature
thresholds in my machine?

> 3. Should the contents kfifo be preserved when the system goes into Suspend?

Maybe not. I don't think they would be relevant anymore after a resume.

> 4. Only one application will get any event (kfifo ensures this). Is this
> expected?

That's how it's designed, but I didn't get feedback if that's okay. It's
readable only by root, so the expectation is you know what you're
doing. Any other suggestions are appreciated, though.

> 5. How to have provision in the driver to cancel the current AER cmds and
> send new ones after doing some new Set Feature?

But the controller shouldn't require a new async event request in order
to respond to changed async configuration features.

> This will need the CmdIds for current AREs to be exported to User Space so
> that they can be aborted. Should the Driver support this?
>
> 6. Is the IOCtl interface enough? Or we want to keep the Read/Poll interface?

Is the IOCTL interface enough for what? It's certainly not usable for
async event notification as-is. You want to add a new IOCTL to consume
aysnc events instead of getting them from a read?

> 7. Is there any reference utility that can be looked at to see what such
> monitoring applications expect?

That would be something I'd like to see as well. I've no idea how these
sorts of events are typically consumed so I just made something up.

Questions on the correct Asynchronous Event Request Interface

[Indraneel Mukherjee]

> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On Behalf
> Of Keith Busch
> Sent: Thursday, June 12, 2014 9:37 PM
> To: INDRANEEL MUKHERJEE
> Cc: linux-nvme at lists.infradead.org
> Subject: Re: Questions on the correct Asynchronous Event Request Interface
> 
> On Wed, 11 Jun 2014, INDRANEEL MUKHERJEE wrote:
> > A couple of patches have already been posted on the Asynchronous Event
> > Request(AER) admin command.  But there is still no clarity on the what
> > is the correct interface to the application.
> >
> > Here'r some questions to help summarise the requirements & finalise an
> > interface so that a patch can follow.  (References to kfifo are based
> > on Keith's patch -
> > http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.htm
> > l )
> >
> > 1. Should the Driver return both the Async Event Completion & the
> > corresponding GetLogPage together to User Space?
> >
> > This can help to avoid the Async Event getting masked accidently. It's
> > possible that any Application reads the Async Event from kfifo but
> > fails to read the corresponding Log Page masking the event indefinitely.
> 
> I don't think so. The driver has no gaurantee there is even a user space
app
> consuming these events. If the driver is unmasking them, we could continue
> getting events and overrun older ones.
> 

What I meant was that when application issues a read for reading the kfifo
event, the driver can issue the corresponding get_log_page and return both
the kfifo event and the corresponding log page together.
Application should ensure it passes a large enough buffer for this. This
will ensure that event is not masked accidently.

> > 2. How to have provision in driver to set the 'Temperature Threshold'
> > & 'Asynchronous Event Configuration' before sending the AER command?
> >
> > This is needed as Spec says these should be set before the Async Event
> > Request command is sent.
> 
> Does it? Please point me to the section in the spec, because I'm not
finding it in
> either 5.2 or 5.12.1.11.
> 
> Section 5.12.1.4 for Temperature Threshold feature says:
> 
> "The host should configure this feature prior to enabling asynchronous
event
> notification for the temperature exceeding the threshold".
> 
> I take that to mean the host should set the asynchronous event
configuration
> feature to enable temperature notification after you set the temperature
> threshold. All this can be done from user space and async event requests
may be
> outstanding prior to this without affecting it.

I tested this on the device. Works as you say.

> 
> > One way to achieve this is by making these module parameters and doing
> > a SetFeature during Probe & Resume operations. If these module
> > parameters are provided by user during insmod, driver should ensure
> > that these are set before the kthread submits the AER.
> 
> What if I have different devices with vastly different temperature
thresholds in
> my machine?
> 
> > 3. Should the contents kfifo be preserved when the system goes into
Suspend?
> 
> Maybe not. I don't think they would be relevant anymore after a resume.
> 
> > 4. Only one application will get any event (kfifo ensures this). Is
> > this expected?
> 
> That's how it's designed, but I didn't get feedback if that's okay. It's
readable
> only by root, so the expectation is you know what you're doing. Any other
> suggestions are appreciated, though.


Noticed one thing in the kfifo patch - it uses the following sequence:
spin_lock(&dev->event_lock);
kfifo_to_user()       ----->  Can sleep as it is a copy_to_user() call
internally  <------
spin_unlock(&dev->event_lock)


> 
> > 5. How to have provision in the driver to cancel the current AER cmds
> > and send new ones after doing some new Set Feature?
> 
> But the controller shouldn't require a new async event request in order to
> respond to changed async configuration features.

Agreed. I had misinterpreted.

> 
> > This will need the CmdIds for current AREs to be exported to User
> > Space so that they can be aborted. Should the Driver support this?
> >
> > 6. Is the IOCtl interface enough? Or we want to keep the Read/Poll
interface?
> 
> Is the IOCTL interface enough for what? It's certainly not usable for
async event
> notification as-is. You want to add a new IOCTL to consume aysnc events
> instead of getting them from a read?
> 
> > 7. Is there any reference utility that can be looked at to see what
> > such monitoring applications expect?
> 
> That would be something I'd like to see as well. I've no idea how these
sorts of
> events are typically consumed so I just made something up.
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme