NVMe: Add support to receive NVMe asynchronous events
Subsystems (2)
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
As a NVMe mandatory admin command, this driver should be setup so that it can receive drive critical asynchronous notification for issue such as device reliability, temperature above threshold, or available spare space fallen below threshould. This patch enables very basic mechanism to log the asynchronous events in kernel log.
--- drivers/block/nvme-core.c | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
drivers/block/nvme-core.c +57 -0
@@ -198,6 +198,7 @@ static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx,#define CMD_CTX_COMPLETED (0x310 + CMD_CTX_BASE)#define CMD_CTX_INVALID (0x314 + CMD_CTX_BASE)#define CMD_CTX_ABORT (0x318 + CMD_CTX_BASE) +#define CMD_CTX_ASYNC_EVENT (0x319 + CMD_CTX_BASE)static void special_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe)
@@ -227,7 +228,27 @@ static void special_completion(struct nvme_queue *nvmeq, void *ctx,static void async_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe) { +int cmdid; +struct nvme_queue *adminq; +struct nvme_dev *dev = nvmeq->dev;struct async_cmd_info *cmdinfo = ctx; + +if (ctx ==CMD_CTX_ASYNC_EVENT ) { +dev_warn (nvmeq->q_dmadev,"An async event is detected (DW0:%X)\n" , + cqe->result); + + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry for next async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , + async_completion,0 ); +if (cmdid <0 ) +dev_warn (nvmeq->q_dmadev, +"Failure creating new entry for next async event\n" ); + +return ;/* Report asynchronous critical event, and exit */ + } + cmdinfo->result =le32_to_cpup (&cqe->result); cmdinfo->status =le16_to_cpup (&cqe->status) >>1 ;queue_kthread_work (cmdinfo->worker, &cmdinfo->work);
@@ -2381,6 +2402,37 @@ static int nvme_delete_cq(struct nvme_queue *nvmeq) nvme_del_cq_work_handler); } +static int nvme_enable_async_events (struct nvme_dev *dev) +{ +int status, cmdid; +u32 result, async_events; +struct nvme_queue *adminq; + + async_events =NVME_SMART_CRIT_SPARE | +NVME_SMART_CRIT_TEMPERATURE | +NVME_SMART_CRIT_RELIABILITY | +NVME_SMART_CRIT_MEDIA | +NVME_SMART_CRIT_VOLATILE_MEMORY ; + + status =nvme_set_features (dev,NVME_FEAT_ASYNC_EVENT , + async_events,0 , &result); + +if (status <0 ) +return status; + +if (status >0 ) { +dev_err (&dev->pci_dev->dev,"Could not enable async event (%d)\n" , + status); +return -EBUSY ; + } + + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry in preparation of next incoming async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , async_completion,0 ); +return cmdid; +} +static void nvme_del_sq_work_handler (struct kthread_work *work) { struct nvme_queue *nvmeq = container_of(work, struct nvme_queue,
@@ -2638,6 +2690,11 @@ static int nvme_dev_start(struct nvme_dev *dev) goto disable; } +/* Enable receive asynchronous event */ + result =nvme_enable_async_events (dev); +if (result <0 && result != -EBUSY ) + goto disable; + result =nvme_setup_io_queues (dev);if (result && result != -EBUSY ) goto disable;
Signature · 1 line
1.7.9.5
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Quoted 16 lines · Winson Yung (wyung)
As a NVMe mandatory admin command, this driver should be setup so that it can receive drive critical asynchronous notification for issue such as device reliability, temperature above threshold, or available spare space fallen below threshould. This patch enables very basic mechanism to log the asynchronous events in kernel log.
Signed-off-by Winson Yung <wyung at micron.com>---
drivers/block/nvme-core.c| 57 +++++++++++++++++++++++++++++++++++++++++++++1 file changed, 57 insertions(+)drivers/block/nvme-core.c +0
-0
drivers/block/nvme-core.c static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx, · line 201
Quoted 4 lines
drivers/block/nvme-core.c +1
-0@@ -198,6 +198,7 @@ static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx,#define CMD_CTX_COMPLETED (0x310 + CMD_CTX_BASE)#define CMD_CTX_INVALID (0x314 + CMD_CTX_BASE)#define CMD_CTX_ABORT (0x318 + CMD_CTX_BASE) +#define CMD_CTX_ASYNC_EVENT (0x319 + CMD_CTX_BASE)
Context needs to be dword aligned, so the next new context would be 0x31c.
Quoted 10 lines
drivers/block/nvme-core.c +3
-0static void special_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe)@@ -227,7 +228,27 @@ static void special_completion(struct nvme_queue *nvmeq, void *ctx,static void async_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe) { +int cmdid; +struct nvme_queue *adminq; +struct nvme_dev *dev = nvmeq->dev;struct async_cmd_info *cmdinfo = ctx;
drivers/block/nvme-core.c static void special_completion(struct nvme_queue *nvmeq, void *ctx, · line 244
Quoted 10 lines
drivers/block/nvme-core.c +10
-0+ +if (ctx ==CMD_CTX_ASYNC_EVENT ) { +dev_warn (nvmeq->q_dmadev,"An async event is detected (DW0:%X)\n" , + cqe->result); + + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry for next async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , + async_completion,0 );
You've allocated this new cmdid with no timeout, and then you do nothing with the command id. It's just going to get timed out.
Quoted 18 lines
drivers/block/nvme-core.c +11
-0+if (cmdid <0 ) +dev_warn (nvmeq->q_dmadev, +"Failure creating new entry for next async event\n" ); + +return ;/* Report asynchronous critical event, and exit */ + } + cmdinfo->result =le32_to_cpup (&cqe->result); cmdinfo->status =le16_to_cpup (&cqe->status) >>1 ;queue_kthread_work (cmdinfo->worker, &cmdinfo->work);@@ -2381,6 +2402,37 @@ static int nvme_delete_cq(struct nvme_queue *nvmeq) nvme_del_cq_work_handler); } +static int nvme_enable_async_events (struct nvme_dev *dev) +{ +int status, cmdid; +u32 result, async_events;
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2418
Quoted 10 lines
drivers/block/nvme-core.c +10
-0+struct nvme_queue *adminq; + + async_events =NVME_SMART_CRIT_SPARE | +NVME_SMART_CRIT_TEMPERATURE | +NVME_SMART_CRIT_RELIABILITY | +NVME_SMART_CRIT_MEDIA | +NVME_SMART_CRIT_VOLATILE_MEMORY ; + + status =nvme_set_features (dev,NVME_FEAT_ASYNC_EVENT , + async_events,0 , &result);
I think we'd rather let the user pick what events they want to see using the passthrough IOCTL rather than have the driver decide these things.
Quoted 4 lines
drivers/block/nvme-core.c +4
-0+ +if (status <0 ) +return status; +
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2432
Quoted 10 lines
drivers/block/nvme-core.c +10
-0+if (status >0 ) { +dev_err (&dev->pci_dev->dev,"Could not enable async event (%d)\n" , + status); +return -EBUSY ; + } + + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry in preparation of next incoming async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , async_completion,0 );
Same thing as in the completion, you're allocating a cmdid, but no command is associated with it.
I don't think you want to use the "async_completion" callback either (is it confusing to name that callback that way? :)) unless you have some deferred work that needs to be done upon the completion that can't be done in interrupt context.
Quoted 18 lines
drivers/block/nvme-core.c +8
-0+return cmdid; +} +static void nvme_del_sq_work_handler (struct kthread_work *work) { struct nvme_queue *nvmeq = container_of(work, struct nvme_queue,@@ -2638,6 +2690,11 @@ static int nvme_dev_start(struct nvme_dev *dev) goto disable; } +/* Enable receive asynchronous event */ + result =nvme_enable_async_events (dev); +if (result <0 && result != -EBUSY ) + goto disable; + result =nvme_setup_io_queues (dev);if (result && result != -EBUSY ) goto disable;
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Thanks Keith for the feedback. I wasn't sure how to use the current code to receive async event. I see the code create a cmdid in order to send an admin cmd. The operation is either synchronous or asynchronous, but it is always expecting the cmd completion response in the end within a predetermined period.
For receiving async events, I thought to just create a completion queue entry after 'set feature' enabling the async event, I was assuming anytime when there is an async event, nvme_process_cq() will pick up the completion queue entry I created to store the event detail, and there was no timeout involved regardless what I passed in for 'timeout' when calling alloc_cmdid.
See my additional comments inline below
Quoted 12 lines · Keith Busch
Quoted 11 lines · Winson Yung (wyung)
As a NVMe mandatory admin command, this driver should be setup so that it can receive drive critical asynchronous notification for issue such as device reliability, temperature above threshold, or available spare space fallen below threshould. This patch enables very basic mechanism to log the asynchronous events in kernel log.
Signed-off-by Winson Yung <wyung at micron.com>---
drivers/block/nvme-core.c| 57 +++++++++++++++++++++++++++++++++++++++++++++1 file changed, 57 insertions(+)
Quoted 12 lines
drivers/block/nvme-core.c static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx, · line 201
Quoted 9 lines
drivers/block/nvme-core.c +1
-0@@ -198,6 +198,7 @@ static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx,#define CMD_CTX_COMPLETED (0x310 + CMD_CTX_BASE)#define CMD_CTX_INVALID (0x314 + CMD_CTX_BASE)#define CMD_CTX_ABORT (0x318 + CMD_CTX_BASE) +#define CMD_CTX_ASYNC_EVENT (0x319 + CMD_CTX_BASE)Context needs to be dword aligned, so the next new context would be
0x31c.
thanks, I will fix the dword alignment definition problem
Quoted 15 lines
Quoted 15 lines
drivers/block/nvme-core.c +8
-0static void special_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe)@@ -227,7 +228,27 @@ static void special_completion(struct nvme_queue *nvmeq, void *ctx,static void async_completion (struct nvme_queue *nvmeq,void *ctx,struct nvme_completion *cqe) { +int cmdid; +struct nvme_queue *adminq; +struct nvme_dev *dev = nvmeq->dev;struct async_cmd_info *cmdinfo = ctx; + +if (ctx ==CMD_CTX_ASYNC_EVENT ) { +dev_warn (nvmeq->q_dmadev,"An async event is detected (DW0:%X)\n" , + cqe->result); +
Quoted 9 lines
drivers/block/nvme-core.c static void special_completion(struct nvme_queue *nvmeq, void *ctx, · line 244
Quoted 5 lines
drivers/block/nvme-core.c +5
-0+ adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry for next async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , + async_completion,0 );You've allocated this new cmdid with no timeout, and then you do nothing with the command id. It's just going to get timed out.
The reason I did this way is because I assume I need to allocate an empty completion queue entry in preparation to receive an incoming async event in the future. It doesn't need to submit anything with this cmdid. That's why it didn't do anything with it, and there is no timeout needed. Maybe my understanding was wrong.
Quoted 23 lines
Quoted 23 lines
drivers/block/nvme-core.c +16
-0+if (cmdid <0 ) +dev_warn (nvmeq->q_dmadev, +"Failure creating new entry for next async event\n" ); + +return ;/* Report asynchronous critical event, and exit */ + } + cmdinfo->result =le32_to_cpup (&cqe->result); cmdinfo->status =le16_to_cpup (&cqe->status) >>1 ;queue_kthread_work (cmdinfo->worker, &cmdinfo->work);@@ -2381,6 +2402,37 @@ static int nvme_delete_cq(struct nvme_queue *nvmeq) nvme_del_cq_work_handler); } +static int nvme_enable_async_events (struct nvme_dev *dev) +{ +int status, cmdid; +u32 result, async_events; +struct nvme_queue *adminq; + + async_events =NVME_SMART_CRIT_SPARE | +NVME_SMART_CRIT_TEMPERATURE | + NVME_SMART_CRIT_RELIABILITY |
Quoted 8 lines
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2418
Quoted 5 lines
drivers/block/nvme-core.c +5
-0+NVME_SMART_CRIT_MEDIA | +NVME_SMART_CRIT_VOLATILE_MEMORY ; + + status =nvme_set_features (dev,NVME_FEAT_ASYNC_EVENT , + async_events,0 , &result);I think we'd rather let the user pick what events they want to see using the passthrough IOCTL rather than have the driver decide these things.
Good idea, I think a sysfs might be better, do you have objection use that instead.
Quoted 10 lines
Quoted 10 lines
drivers/block/nvme-core.c +9
-0+ +if (status <0 ) +return status; + +if (status >0 ) { +dev_err (&dev->pci_dev->dev,"Could not enable async event (%d)\n" , + status); +return -EBUSY ; + }
Quoted 8 lines
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2432
Quoted 5 lines
drivers/block/nvme-core.c +5
-0+ + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry in preparation of next incoming async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , async_completion,0 );Same thing as in the completion, you're allocating a cmdid, but no command is associated with it.
Ditto here, I knew this will probably require some rework. The basic difference between an asynchronous event and regular admin cmd is that async event doesn't have a timeout, it can come any time after host issues enabling the async event feature, whereas a normal admin cmd will get some sort of completion response within certain timeout period.
Quoted 6 lines
I don't think you want to use the "
async_completion" callback either (is it confusing to name that callback that way? :)) unless you have some deferred work that needs to be done upon the completion that can't be done in interrupt context.
I can change to use a different callback name. But using alloc_cmdid with a zero timeout and callback may not be the right way for async event. Any suggestion is welcome.
Quoted 18 lines
Quoted 18 lines
drivers/block/nvme-core.c +8
-0+return cmdid; +} +static void nvme_del_sq_work_handler (struct kthread_work *work) { struct nvme_queue *nvmeq = container_of(work, struct nvme_queue,@@ -2638,6 +2690,11 @@ static int nvme_dev_start(struct nvme_dev *dev) goto disable; } +/* Enable receive asynchronous event */ + result =nvme_enable_async_events (dev); +if (result <0 && result != -EBUSY ) + goto disable; + result =nvme_setup_io_queues (dev);if (result && result != -EBUSY ) goto disable;
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
In reply to Winson Yung (wyung)Quoted 5 lines · Winson Yung (wyung)
As a NVMe mandatory admin command, this driver should be setup so that it can receive drive critical asynchronous notification for issue such as device reliability, temperature above threshold, or available spare space fallen below threshould. This patch enables very basic mechanism to log the asynchronous events in kernel log.
Umm ... did you test this patch?
Quoted 10 lines
Signed-off-by Winson Yung <wyung at micron.com>--- drivers/block/nvme-core.c | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)drivers/block/nvme-core.c +0
-0
drivers/block/nvme-core.c static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx, · line 201
Quoted 4 lines
drivers/block/nvme-core.c +1
-0@@ -198,6 +198,7 @@ static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx,#define CMD_CTX_COMPLETED (0x310 + CMD_CTX_BASE)#define CMD_CTX_INVALID (0x314 + CMD_CTX_BASE)#define CMD_CTX_ABORT (0x318 + CMD_CTX_BASE) +#define CMD_CTX_ASYNC_EVENT (0x319 + CMD_CTX_BASE)
Please only use multiples of four; that is, 0x31c, not 0x319. Also, these are for use by 'special_completion', not 'async_completion'.
Quoted 20 lines
drivers/block/nvme-core.c +20
-0+static int nvme_enable_async_events (struct nvme_dev *dev) +{ +int status, cmdid; +u32 result, async_events; +struct nvme_queue *adminq; + + async_events =NVME_SMART_CRIT_SPARE | +NVME_SMART_CRIT_TEMPERATURE | +NVME_SMART_CRIT_RELIABILITY | +NVME_SMART_CRIT_MEDIA | +NVME_SMART_CRIT_VOLATILE_MEMORY ; + + status =nvme_set_features (dev,NVME_FEAT_ASYNC_EVENT , + async_events,0 , &result); + +if (status <0 ) +return status; + +if (status >0 ) { + dev_err(&dev->pci_dev->dev, "Could not enable async event (%d)\n",
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2434
Quoted 10 lines
drivers/block/nvme-core.c +10
-0+ status); +return -EBUSY ; + } + + adminq =rcu_dereference (dev->queues[0 ]); + +/* Allocate a cmdid entry in preparation of next incoming async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , async_completion,0 ); +return cmdid; +}
How is this supposed to work? You've allocated a command ID, but you haven't submitted an asynchronous event request command. So how does the controller know to use this command ID to report asynchronous events?
You should probably take a look at Keith's work last year to allow userspace to send async event requests:
http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Quoted 4 lines · Matthew Wilcox
You should probably take a look at Keith's work last year to allow userspace to send async event requests:
http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html
Yeah, that proposal just lets userspace consume the results of an async request from a FIFO read through the character device. The driver still has to do the sending since there's no mechanism for requests from user space with infinite timeout.
I've no idea if the proposed method for consuming results of this kind of information was typical. I assumed it wasn't since it didn't get much traction. :)
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Keith, I do see a need to enable async event in the NVMe kernel driver. For example, when there is a temperature above threshold, driver can take an action (by telling firmware) to lower down operating frequency, or throttle IO request to protect drive from premature over heat damage.
/Winson
Quoted 15 lines · Keith Busch
Quoted 4 lines · Matthew Wilcox
You should probably take a look at Keith's work last year to allow userspace to send async event requests:
http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html
Yeah, that proposal just lets userspace consume the results of an async request from a FIFO read through the character device. The driver still has to do the sending since there's no mechanism for requests from user space with infinite timeout.
I've no idea if the proposed method for consuming results of this kind of information was typical. I assumed it wasn't since it didn't get much traction. :)
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Quoted 4 lines · Winson Yung (wyung)
Keith, I do see a need to enable async event in the NVMe kernel driver. For example, when there is a temperature above threshold, driver can take an action (by telling firmware) to lower down operating frequency, or throttle IO request to protect drive from premature over heat damage.
This is sounding specific to your environment. What's a good policy for yours might not be a good policy for mine. Why would I want the driver to throttle frequency when I have better environmental control options available? The right thing to do, IMHO, is make all information available to a user space app and provide a way for it to control the device according to a policy it knows.
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
This is exactly what the Windows OFA NVMe open source driver does as well. The driver does not, and should not, have knowledge as to what the "right temperature threshold" is or how many "I/O to throttle" in any kind of situation, good or bad. The NVMe device/controller parameters and policy should be set and handled (proactively and reactively) by the device vendor and any 3rd party user space application, either from the vendor or some other trusted OEM source.
-----Original Message-----
From: Linux-nvme [mailto:[email protected]] On Behalf Of Keith Busch
Sent: Monday, May 26, 2014 10:51 PM
To: Winson Yung (wyung)
Cc: Busch, Keith; Matthew Wilcox; linux-nvme at lists.infradead.org
Subject: Re: [PATCH] NVMe: Add support to receive NVMe asynchronous events
Quoted 4 lines · Winson Yung (wyung)
Keith, I do see a need to enable async event in the NVMe kernel
driver. For example, when there is a temperature above threshold,
driver can take an action (by telling firmware) to lower down
operating frequency, or throttle IO request to protect drive from premature over heat damage.
This is sounding specific to your environment. What's a good policy for yours might not be a good policy for mine. Why would I want the driver to throttle frequency when I have better environmental control options available? The right thing to do, IMHO, is make all information available to a user space app and provide a way for it to control the device according to a policy it knows.
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
In reply to Keith BuschI had a look of the patch, it looks it does what I was trying to enable, except it misses the portion that enables it by calling set feature cmd. Is it something we can consider now? Also, for these critical warnings that exposed by the async events, can we also put them in the kernel log as well when they enabled?
/Winson
Quoted 15 lines · Keith Busch
Quoted 4 lines · Matthew Wilcox
You should probably take a look at Keith's work last year to allow userspace to send async event requests:
http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html
Yeah, that proposal just lets userspace consume the results of an async request from a FIFO read through the character device. The driver still has to do the sending since there's no mechanism for requests from user space with infinite timeout.
I've no idea if the proposed method for consuming results of this kind of information was typical. I assumed it wasn't since it didn't get much traction. :)
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
Quoted 5 lines · Winson Yung (wyung)
I had a look of the patch, it looks it does what I was trying to enable, except it misses the portion that enables it by calling set feature cmd. Is it something we can consider now? Also, for these critical warnings that exposed by the async events, can we also put them in the kernel log as well when they enabled?
I thought we agreed it's best to let the user space do the set features so it gets to decide what events it wants to see.
I don't have a strong opinion on how to present events to the user. Maybe just print them to the kernel log and be done with it.
PATCH 1/1 NVMe: Add support to receive NVMe asynchronous events
In reply to Matthew WilcoxQuoted 8 lines · Matthew Wilcox
Quoted 5 lines · Winson Yung (wyung)
As a NVMe mandatory admin command, this driver should be setup so that it can receive drive critical asynchronous notification for issue such as device reliability, temperature above threshold, or available spare space fallen below threshould. This patch enables very basic mechanism to log the asynchronous events in kernel log.
Umm ... did you test this patch?
I didn't have the time to test the patch yet, perhaps I should mark this patch as RFC instead. But yes, I will try to verify when I have a system setup.
Quoted 6 lines
Quoted 6 lines
Signed-off-by Winson Yung <wyung at micron.com>--- drivers/block/nvme-core.c | 57 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
Quoted 13 lines
drivers/block/nvme-core.c static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx, · line 201
Quoted 9 lines
drivers/block/nvme-core.c +1
-0@@ -198,6 +198,7 @@ static int alloc_cmdid_killable(struct nvme_queue *nvmeq, void *ctx,#define CMD_CTX_COMPLETED (0x310 + CMD_CTX_BASE)#define CMD_CTX_INVALID (0x314 + CMD_CTX_BASE)#define CMD_CTX_ABORT (0x318 + CMD_CTX_BASE) +#define CMD_CTX_ASYNC_EVENT (0x319 + CMD_CTX_BASE)Please only use multiples of four; that is,
0x31c, not0x319. Also, these are for use by 'special_completion', not 'async_completion'.
Understood about the multiple of four thingy, I will fix it. I will change the name to avoid confusion if I can still use async_completion.
Quoted 26 lines
Quoted 26 lines
drivers/block/nvme-core.c +26
-0+static int nvme_enable_async_events (struct nvme_dev *dev) +{ +int status, cmdid; +u32 result, async_events; +struct nvme_queue *adminq; + + async_events =NVME_SMART_CRIT_SPARE | +NVME_SMART_CRIT_TEMPERATURE | +NVME_SMART_CRIT_RELIABILITY | +NVME_SMART_CRIT_MEDIA | +NVME_SMART_CRIT_VOLATILE_MEMORY ; + + status =nvme_set_features (dev,NVME_FEAT_ASYNC_EVENT , + async_events,0 , &result); + +if (status <0 ) +return status; + +if (status >0 ) { +dev_err (&dev->pci_dev->dev,"Could not enable async event (%d)\n" , + status); +return -EBUSY ; + } + + adminq =rcu_dereference (dev->queues[0 ]); +
Quoted 9 lines
drivers/block/nvme-core.c static int nvme_delete_cq(struct nvme_queue *nvmeq) · line 2434
Quoted 4 lines
drivers/block/nvme-core.c +4
-0+/* Allocate a cmdid entry in preparation of next incoming async event */ + cmdid =alloc_cmdid (adminq,CMD_CTX_ASYNC_EVENT , async_completion,0 ); +return cmdid; +}How is this supposed to work? You've allocated a command ID, but you haven't submitted an asynchronous event request command. So how does the controller know to use this command ID to report asynchronous events?
Please see my reply to Keith's email, perhaps my assumption was wrong.
Quoted 5 lines
You should probably take a look at Keith's work last year to allow userspace to send async event requests:
http://lists.infradead.org/pipermail/linux-nvme/2013-August/000350.html
Thanks for the link, I wasn't aware of that. I will take a look.