nvme-pci: factor out a nvme_pci_alloc_dev helper
authorChristoph Hellwig <hch@lst.de>
Tue, 8 Nov 2022 08:44:00 +0000 (09:44 +0100)
committerChristoph Hellwig <hch@lst.de>
Tue, 15 Nov 2022 09:56:09 +0000 (10:56 +0100)
Add a helper that allocates the nvme_dev structure up to the point where
we can call nvme_init_ctrl.  This pairs with the free_ctrl method and can
thus be used to cleanup the teardown path and make it more symmetric.

Note that this now calls nvme_init_ctrl a lot earlier during probing,
which also means the per-controller character device shows up earlier.
Due to the controller state no commnds can be send on it, but it might
make sense to delay the cdev registration until nvme_init_ctrl_finish.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Tested-by Gerd Bayer <gbayer@linxu.ibm.com>

drivers/nvme/host/pci.c

index f7dab65..03c83cd 100644 (file)
@@ -2777,6 +2777,7 @@ static void nvme_free_tagset(struct nvme_dev *dev)
        dev->ctrl.tagset = NULL;
 }
 
+/* pairs with nvme_pci_alloc_dev */
 static void nvme_pci_free_ctrl(struct nvme_ctrl *ctrl)
 {
        struct nvme_dev *dev = to_nvme_dev(ctrl);
@@ -3090,19 +3091,23 @@ static void nvme_async_probe(void *data, async_cookie_t cookie)
        nvme_put_ctrl(&dev->ctrl);
 }
 
-static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static struct nvme_dev *nvme_pci_alloc_dev(struct pci_dev *pdev,
+               const struct pci_device_id *id)
 {
-       int node, result = -ENOMEM;
-       struct nvme_dev *dev;
        unsigned long quirks = id->driver_data;
+       int node = dev_to_node(&pdev->dev);
+       struct nvme_dev *dev;
+       int ret = -ENOMEM;
 
-       node = dev_to_node(&pdev->dev);
        if (node == NUMA_NO_NODE)
                set_dev_node(&pdev->dev, first_memory_node);
 
        dev = kzalloc_node(sizeof(*dev), GFP_KERNEL, node);
        if (!dev)
-               return -ENOMEM;
+               return NULL;
+       INIT_WORK(&dev->ctrl.reset_work, nvme_reset_work);
+       INIT_WORK(&dev->remove_work, nvme_remove_dead_ctrl_work);
+       mutex_init(&dev->shutdown_lock);
 
        dev->nr_write_queues = write_queues;
        dev->nr_poll_queues = poll_queues;
@@ -3110,25 +3115,11 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
        dev->queues = kcalloc_node(dev->nr_allocated_queues,
                        sizeof(struct nvme_queue), GFP_KERNEL, node);
        if (!dev->queues)
-               goto free;
+               goto out_free_dev;
 
        dev->dev = get_device(&pdev->dev);
-       pci_set_drvdata(pdev, dev);
-
-       result = nvme_dev_map(dev);
-       if (result)
-               goto put_pci;
-
-       INIT_WORK(&dev->ctrl.reset_work, nvme_reset_work);
-       INIT_WORK(&dev->remove_work, nvme_remove_dead_ctrl_work);
-       mutex_init(&dev->shutdown_lock);
-
-       result = nvme_setup_prp_pools(dev);
-       if (result)
-               goto unmap;
 
        quirks |= check_vendor_combination_bug(pdev);
-
        if (!noacpi && acpi_storage_d3(&pdev->dev)) {
                /*
                 * Some systems use a bios work around to ask for D3 on
@@ -3138,34 +3129,54 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
                         "platform quirk: setting simple suspend\n");
                quirks |= NVME_QUIRK_SIMPLE_SUSPEND;
        }
+       ret = nvme_init_ctrl(&dev->ctrl, &pdev->dev, &nvme_pci_ctrl_ops,
+                            quirks);
+       if (ret)
+               goto out_put_device;
+       return dev;
 
-       result = nvme_pci_alloc_iod_mempool(dev);
+out_put_device:
+       put_device(dev->dev);
+       kfree(dev->queues);
+out_free_dev:
+       kfree(dev);
+       return ERR_PTR(ret);
+}
+
+static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+       struct nvme_dev *dev;
+       int result = -ENOMEM;
+
+       dev = nvme_pci_alloc_dev(pdev, id);
+       if (!dev)
+               return -ENOMEM;
+
+       result = nvme_dev_map(dev);
        if (result)
-               goto release_pools;
+               goto out_uninit_ctrl;
 
-       result = nvme_init_ctrl(&dev->ctrl, &pdev->dev, &nvme_pci_ctrl_ops,
-                       quirks);
+       result = nvme_setup_prp_pools(dev);
+       if (result)
+               goto out_dev_unmap;
+
+       result = nvme_pci_alloc_iod_mempool(dev);
        if (result)
-               goto release_mempool;
+               goto out_release_prp_pools;
 
        dev_info(dev->ctrl.device, "pci function %s\n", dev_name(&pdev->dev));
+       pci_set_drvdata(pdev, dev);
 
        nvme_reset_ctrl(&dev->ctrl);
        async_schedule(nvme_async_probe, dev);
-
        return 0;
 
- release_mempool:
-       mempool_destroy(dev->iod_mempool);
- release_pools:
+out_release_prp_pools:
        nvme_release_prp_pools(dev);
- unmap:
+out_dev_unmap:
        nvme_dev_unmap(dev);
- put_pci:
-       put_device(dev->dev);
- free:
-       kfree(dev->queues);
-       kfree(dev);
+out_uninit_ctrl:
+       nvme_uninit_ctrl(&dev->ctrl);
        return result;
 }