Skip to contentSkip to conversation

release /dev/nvmeX if nvme_probe failed

PATCH Superseded Weiping Zhang 5 participants 9 messages Jul 1, 2018 – Jul 2, 2018 NVMe

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover release /dev/nvmeX if nvme_probe failed

First patch only remove whitespace, coding style fixup. Second patch, remove nvme_ctrl device (/dev/nvmeX) if failed to alloc memory for iod_mempool in nvme_probe.

Weiping Zhang (2):
nvme-pci: remove whitespace
nvme-pci: release nvme_ctrl if failed to alloc memory in nvme_probe

 drivers/nvme/host/pci.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/2 nvme-pci: remove whitespace

Signed-off-by Weiping Zhang <zhangweiping at didichuxing.com>
---
 drivers/nvme/host/pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/nvme/host/pci.c +4 -4
index ba943f211687..abf1885d6db6 100644
@@ -2585,13 +2585,13 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	return 0;
 
- release_pools:
+release_pools:
 	nvme_release_prp_pools(dev);
- unmap:
+unmap:
 	nvme_dev_unmap(dev);
- put_pci:
+put_pci:
 	put_device(dev->dev);
- free:
+free:
 	kfree(dev->queues);
 	kfree(dev);
 	return result;
Signature · 1 line
2.14.1
maintainer
#2 lore raw

PATCH 1/2 nvme-pci: remove whitespace

Reviewed-by Sagi Grimberg <sagi at grimberg.me>
#3 lore raw

PATCH 1/2 nvme-pci: remove whitespace

In reply to Weiping Zhang

Please, no whitespace cleanups before critical patches.

Also both styles in this case are acceptable in the kernel. I personally prefer to not have space, but we really shouldn't spuriously change it.

#4 lore raw

PATCH 1/2 nvme-pci: remove whitespace

In reply to Weiping Zhang

These style changes aren't the sort of cleanups we need. For one, there's nothing wrong with how it already exists, and it has been this particular style since the driver's inclusion; there are lots of other places in the driver using this pattern. More importantly, it creates inconvenient stable porting issues for developers doing real work in these areas.

#5 lore raw

PATCH 1/2 nvme-pci: remove whitespace

Keith Busch <keith.busch at linux.intel.com> ?2018?7?2??? ??11:46???

Quoted 7 lines

These style changes aren't the sort of cleanups we need. For one, there's nothing wrong with how it already exists, and it has been this particular style since the driver's inclusion; there are lots of other places in the driver using this pattern. More importantly, it creates inconvenient stable porting issues for developers doing real work in these areas.

OK, I drop this one

Quoted 4 lines

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

#6 lore raw

PATCH 2/2 nvme-pci: release nvme_ctrl if failed to alloc memory in nvme_probe

In reply to Weiping Zhang

release nvme_ctrl if failed to alloc memory for iod_mempool, otherwise /dev/nvmeX cann't be removed for this nvme controller, any ioctl to this device doesn't work.

Signed-off-by Weiping Zhang <zhangweiping at didichuxing.com>
---
 drivers/nvme/host/pci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/nvme/host/pci.c +4 -1
index abf1885d6db6..d4f1f3b900e6 100644
@@ -2575,7 +2575,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 						GFP_KERNEL, node);
 	if (!dev->iod_mempool) {
 		result = -ENOMEM;
-		goto release_pools;
+		goto release_ctrl;
 	}
 
 	dev_info(dev->ctrl.device, "pci function %s\n", dev_name(&pdev->dev));
@@ -2585,6 +2585,9 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	return 0;
 
+release_ctrl:
+	nvme_uninit_ctrl(&dev->ctrl);
+	nvme_put_ctrl(&dev->ctrl);
 release_pools:
 	nvme_release_prp_pools(dev);
 unmap:
Signature · 1 line
2.14.1
maintainer
#7 lore raw

PATCH 2/2 nvme-pci: release nvme_ctrl if failed to alloc memory in nvme_probe

drivers/nvme/host/pci.c static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) · line 2590

Quoted 4 lines
drivers/nvme/host/pci.c +3 -0
+release_ctrl:
+	nvme_uninit_ctrl(&dev->ctrl);
+	nvme_put_ctrl(&dev->ctrl);

Hmm, put_ctrl would trigger nvme_pci_free_ctrl which will do some of what you fallthru down here, won't that cause double free/put?

Quoted 4 lines
  release_pools:
  	nvme_release_prp_pools(dev);
  unmap:
#8 lore raw

PATCH 2/2 nvme-pci: release nvme_ctrl if failed to alloc memory in nvme_probe

Sagi Grimberg <sagi at grimberg.me> ?2018?7?2??? ??5:54???

Quoted 10 lines

drivers/nvme/host/pci.c static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) · line 2590

Quoted 5 lines
drivers/nvme/host/pci.c +3 -0
+release_ctrl:
+     nvme_uninit_ctrl(&dev->ctrl);
+     nvme_put_ctrl(&dev->ctrl);

Hmm, put_ctrl would trigger nvme_pci_free_ctrl which will do some of what you fallthru down here, won't that cause double free/put?

You are right, I plan to move all this cleanup to nvme_pci_ctrl_free, If failed before call nvme_init_ctrl, do cleanup by these goto. if failed after call nvme_init_ctrl, direct use nvme_put_ctrl to release all these resource.

I'll send V2.

Quoted 9 lines
Quoted 4 lines
  release_pools:
      nvme_release_prp_pools(dev);
  unmap:

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