NVMe: Automatic namespace rescan
Subsystems (2)
PATCH 1/1 NVMe: Automatic namespace rescan
This rescans device namespaces everytime the controller is reset, or if the controller posts an asynchronous event indicating namespace attributes have changed. The namespace scanning work will revalidate existing namespaces for any attribute changes, such as block format or capacity, as well as look for new namespaces that may be allocated or attached to this controller.
If namespaces are deleted, this does not go so far as to delete gendisks; instead, its capacity will be set to 0.
--- drivers/block/nvme-core.c | 66 +++++++++++++++++++++++++++++++++++++++------ include/linux/nvme.h | 1 + include/uapi/linux/nvme.h | 5 ++++ 3 files changed, 64 insertions(+), 8 deletions(-)
drivers/block/nvme-core.c +58 -8
@@ -307,9 +307,16 @@ static void async_req_completion(struct nvme_queue *nvmeq, void *ctx,if (status ==NVME_SC_SUCCESS || status ==NVME_SC_ABORT_REQ ) ++nvmeq->dev->event_limit; -if (status ==NVME_SC_SUCCESS ) -dev_warn (nvmeq->q_dmadev, -"async event result %08x\n" , result); +if (status ==NVME_SC_SUCCESS ) { +dev_warn (nvmeq->q_dmadev,"async event result %08x\n" , result); +if ((result &0x3 ) ==NVME_AER_TYPE_NOTICE && + ((result >>8 ) &0xff ) ==NVME_AER_INFO_NS_CHANGE ) { +struct nvme_dev *dev = nvmeq->dev; +dev_info (nvmeq->q_dmadev, +"namespace change event, rescan\n" ); +schedule_work (&dev->scan_work); + } + } } static void abort_completion(struct nvme_queue *nvmeq, void *ctx,
@@ -2162,6 +2169,50 @@ static void nvme_alloc_ns(struct nvme_dev *dev, unsigned nsid)kfree (ns); } +static struct nvme_ns *nvme_find_ns (struct nvme_dev *dev,unsigned nsid) +{ +struct nvme_ns *ns; + +list_for_each_entry (ns, &dev->namespaces, list) +if (ns->ns_id == nsid) +return ns; +return NULL ; +} + +static void nvme_scan_namespaces (struct nvme_dev *dev,unsigned nn) +{ +struct nvme_ns *ns; +unsigned i; + +for (i =1 ; i <= nn; i++) { + ns =nvme_find_ns (dev, i); +if (ns) +revalidate_disk (ns->disk); +else +nvme_alloc_ns (dev, i); + } +} + +static void nvme_dev_scan (struct work_struct *work) +{ + struct nvme_dev *dev = container_of(work, struct nvme_dev, scan_work); +unsigned long nn; +void *mem; +struct nvme_id_ctrl *ctrl; +dma_addr_t dma_addr; + + mem =dma_alloc_coherent (&dev->pci_dev->dev,4096 , &dma_addr,GFP_KERNEL ); +if (!mem) +return ; +if (nvme_identify (dev,0 ,1 , dma_addr)) + goto free; + ctrl = mem; + nn =le32_to_cpup (&ctrl->nn); +nvme_scan_namespaces (dev, nn); + free: +dma_free_coherent (&dev->pci_dev->dev,4096 , mem, dma_addr); +} +static void nvme_create_io_queues (struct nvme_dev *dev) {unsigned i;
@@ -2283,7 +2334,6 @@ static int nvme_dev_add(struct nvme_dev *dev) {struct pci_dev *pdev = dev->pci_dev;int res; -unsigned nn, i;struct nvme_id_ctrl *ctrl;void *mem;dma_addr_t dma_addr;
@@ -2301,7 +2351,6 @@ static int nvme_dev_add(struct nvme_dev *dev) } ctrl = mem; - nn =le32_to_cpup (&ctrl->nn); dev->oncs =le16_to_cpup (&ctrl->oncs); dev->abort_limit = ctrl->acl +1 ; dev->vwc = ctrl->vwc;
@@ -2337,9 +2386,7 @@ static int nvme_dev_add(struct nvme_dev *dev)if (blk_mq_alloc_tag_set (&dev->tagset))return 0 ; -for (i =1 ; i <= nn; i++) -nvme_alloc_ns (dev, i); - +schedule_work (&dev->scan_work);return 0 ; }
@@ -2911,6 +2958,7 @@ static int nvme_dev_resume(struct nvme_dev *dev)spin_unlock (&dev_list_lock); }else {nvme_unfreeze_queues (dev); +schedule_work (&dev->scan_work);nvme_set_irq_hints (dev); }return 0 ;
@@ -2989,6 +3037,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)get_device (dev->device);INIT_LIST_HEAD (&dev->node); +INIT_WORK (&dev->scan_work, nvme_dev_scan);INIT_WORK (&dev->probe_work, nvme_async_probe);schedule_work (&dev->probe_work);return 0 ;
@@ -3054,6 +3103,7 @@ static void nvme_remove(struct pci_dev *pdev)spin_unlock (&dev_list_lock);pci_set_drvdata (pdev,NULL ); +flush_work (&dev->scan_work);flush_work (&dev->probe_work);flush_work (&dev->reset_work);nvme_dev_remove (dev);
include/linux/nvme.h +1 -0
@@ -92,6 +92,7 @@ struct nvme_dev {work_func_t reset_workfn;struct work_struct reset_work;struct work_struct probe_work; +struct work_struct scan_work;char name[12 ];char serial[20 ];char model[40 ];
include/uapi/linux/nvme.h +5 -0
@@ -179,6 +179,11 @@ enum { NVME_SMART_CRIT_VOLATILE_MEMORY = 1 << 4, }; +enum { +NVME_AER_TYPE_NOTICE =2 , +NVME_AER_INFO_NS_CHANGE =0 , +}; +struct nvme_lba_range_type {__u8 type;__u8 attributes;
Signature · 1 line
1.7.10.4
PATCH 1/1 NVMe: Automatic namespace rescan
Quoted 10 lines · Keith Busch
drivers/block/nvme-core.c +3
-3@@ -307,9 +307,16 @@ static void async_req_completion(struct nvme_queue *nvmeq, void *ctx,if (status ==NVME_SC_SUCCESS || status ==NVME_SC_ABORT_REQ ) ++nvmeq->dev->event_limit; -if (status ==NVME_SC_SUCCESS ) -dev_warn (nvmeq->q_dmadev, -"async event result %08x\n" , result); +if (status ==NVME_SC_SUCCESS ) { +dev_warn (nvmeq->q_dmadev,"async event result %08x\n" , result); + if ((result & 0x3) == NVME_AER_TYPE_NOTICE &&
drivers/block/nvme-core.c static void async_req_completion(struct nvme_queue *nvmeq, void *ctx, · line 322
Quoted 10 lines
drivers/block/nvme-core.c +7
-0+ ((result >> 8) & 0xff) == NVME_AER_INFO_NS_CHANGE) { +struct nvme_dev *dev = nvmeq->dev; +dev_info (nvmeq->q_dmadev, +"namespace change event, rescan\n" ); +schedule_work (&dev->scan_work); + } + } } static void abort_completion(struct nvme_queue *nvmeq, void *ctx,
I don't think we want the dev_warn() and the dev_info() for the same event. How about ...
if (status != NVME_SC_SUCCESS)
return;
switch (result & 0xff07) {
case NVME_AER_NOTICE_NS_CHANGED:
dev_info(nvmeq->q_dmadev, "rescanning\n");
schedule_work(&nvmeq->dev->scan_work);
default:
dev_warn(nvmeq->q_dmadev, "async event result %08x\n", result);
}with NVME_AER_NOTICE_NS_CHANGED being an enum with value 0x0002
drivers/block/nvme-core.c static void nvme_alloc_ns(struct nvme_dev *dev, unsigned nsid) · line 2180
Quoted 9 lines
drivers/block/nvme-core.c +9
-0+static struct nvme_ns *nvme_find_ns (struct nvme_dev *dev,unsigned nsid) +{ +struct nvme_ns *ns; + +list_for_each_entry (ns, &dev->namespaces, list) +if (ns->ns_id == nsid) +return ns; +return NULL ; +}
Pondering if it's worth keeping the list sorted so we can break out early if the namespace isn't in the list?
PATCH 1/1 NVMe: Automatic namespace rescan
Quoted 17 lines · Matthew Wilcox
Review static void async_req_completion(struct nvme_queue *nvmeq, void *ctx,
Quoted 1 line · Keith Busch
(quoted diff) +0
-0@@ -307,9 +307,16 @@ static void async_req_completion(struct nvme_queue *nvmeq, void *ctx,I don't think we want the
dev_warn()and thedev_info()for the same event. How about ...if (status != NVME_SC_SUCCESS) return; switch (result & 0xff07) { case NVME_AER_NOTICE_NS_CHANGED: dev_info(nvmeq->q_dmadev, "rescanning\n"); schedule_work(&nvmeq->dev->scan_work); default: dev_warn(nvmeq->q_dmadev, "async event result %08x\n", result); }
Quoted 1 line
with
NVME_AER_NOTICE_NS_CHANGEDbeing an enum with value0x0002
Brilliant!
Quoted 4 lines
Quoted 4 lines
drivers/block/nvme-core.c +4
-0+static struct nvme_ns *nvme_find_ns (struct nvme_dev *dev,unsigned nsid) +{ +struct nvme_ns *ns; +
Quoted 8 lines
drivers/block/nvme-core.c static void nvme_alloc_ns(struct nvme_dev *dev, unsigned nsid) · line 2180
Quoted 5 lines
drivers/block/nvme-core.c +5
-0+list_for_each_entry (ns, &dev->namespaces, list) +if (ns->ns_id == nsid) +return ns; +return NULL ; +}Pondering if it's worth keeping the list sorted so we can break out early if the namespace isn't in the list?
The list is actually already sorted since this doesn't allow gaps in NSIDs. If a namespace is not attached, the driver creates a 0 capacity block device for it and appends it to the list, so it's always in ascending order.
Is this a bad idea? Let's say a controller supports 128 namespaces, but only NSID 128 is attached, we'd see 127 zero capacity /dev/nvme#n# block devs.
PATCH 1/1 NVMe: Automatic namespace rescan
Quoted 4 lines · Keith Busch
Quoted 2 lines · Matthew Wilcox
Pondering if it's worth keeping the list sorted so we can break out early if the namespace isn't in the list?
Quoted 8 lines
The list is actually already sorted since this doesn't allow gaps in NSIDs. If a namespace is not attached, the driver creates a 0 capacity block device for it and appends it to the list, so it's always in ascending order.
Is this a bad idea? Let's say a controller supports 128 namespaces, but only NSID 128 is attached, we'd see 127 zero capacity /dev/nvme#n# block devs.
Decided we don't want a bunch of zero capacity drives if there are gaps in attached NSIDs. There could be millions after all. But it's not so simple since the nvmeq's hctx points to hctx from a single namespace's request_queue, and that's not right. I'll take a stab at decoupling that next week. In the mean time, this patch is dead.
PATCH 1/1 NVMe: Automatic namespace rescan
Agree with your decision that we probably don?t want a bunch of zero capacity drives if there are gaps in the namespace IDs. I?d like to see a version of your original patch get in - we have been carrying a patch against nvme-legacy internally that does a similar rescan-after-reset but haven?t had the time to prepare a patch against the current tree yet.
If someone deletes a namespace (vs. detaching it) why wouldn?t you want to remove the gendisk as well? This question is based on your comment in the original patch: "If namespaces are deleted, this does not go so far as to delete gendisks; instead, its capacity will be set to 0." I believe users would expect to see the device representing the namespace go away if they deleted the namespace.
Brandon
Quoted 25 lines · Keith Busch
Quoted 5 lines · Keith Busch
Quoted 3 lines · Matthew Wilcox
Pondering if it's worth keeping the list sorted so we can break out early
if the namespace isn't in the list?Quoted 8 lines
The list is actually already sorted since this doesn't allow gaps in NSIDs. If a namespace is not attached, the driver creates a 0 capacity block device for it and appends it to the list, so it's always in ascending order.
Is this a bad idea? Let's say a controller supports 128 namespaces, but only NSID 128 is attached, we'd see 127 zero capacity /dev/nvme#n# block devs.
Decided we don't want a bunch of zero capacity drives if there are gaps in attached NSIDs. There could be millions after all. But it's not so simple since the nvmeq's hctx points to hctx from a single namespace's
request_queue, and that's not right. I'll take a stab at decoupling that next week. In the mean time, this patch is dead._______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme