[PATCH 0/2] release /dev/nvmeX if nvme_probe failed

[PATCH 0/2] release /dev/nvmeX if nvme_probe failed

[Weiping Zhang]

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(-)

-- 
2.14.1

[PATCH 1/2] nvme-pci: remove whitespace

[Weiping Zhang]

Signed-off-by: Weiping Zhang <zhangweiping at didichuxing.com>
---
 drivers/nvme/host/pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ba943f211687..abf1885d6db6 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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;
-- 
2.14.1

[PATCH 1/2] nvme-pci: remove whitespace

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

[PATCH 1/2] nvme-pci: remove whitespace

[Christoph Hellwig]

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.

[PATCH 1/2] nvme-pci: remove whitespace

[Keith Busch]

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.

[PATCH 1/2] nvme-pci: remove whitespace

[Weiping Zhang]

Keith Busch <keith.busch at linux.intel.com> ?2018?7?2??? ??11:46???
>
> 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
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

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

[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(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index abf1885d6db6..d4f1f3b900e6 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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:
-- 
2.14.1

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

[Sagi Grimberg]

> +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?

>   release_pools:
>   	nvme_release_prp_pools(dev);
>   unmap:
> 

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

[Weiping Zhang]

Sagi Grimberg <sagi at grimberg.me> ?2018?7?2??? ??5:54???
>
>
> > +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.
> >   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