[PATCH] NVMe: Fix error clean-up on nvme_alloc_queue

[PATCH] NVMe: Fix error clean-up on nvme_alloc_queue

[Keith Busch]

The nvme_queue's depth is not set if we fail to allocate submission queue
entries, which was being used to determine how much coherent memory to
free on error. Use the depth variable instead.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 40908a0..e78faf1 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -956,7 +956,7 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, int qid,
 	return nvmeq;
 
  free_cqdma:
-	dma_free_coherent(dmadev, CQ_SIZE(nvmeq->q_depth), (void *)nvmeq->cqes,
+	dma_free_coherent(dmadev, CQ_SIZE(depth), (void *)nvmeq->cqes,
 							nvmeq->cq_dma_addr);
  free_nvmeq:
 	kfree(nvmeq);
-- 
1.7.0.4

[PATCH] NVMe: Check for NULL memory in nvme_dev_add

[Keith Busch]

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index e78faf1..e7d68e3 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1523,6 +1523,8 @@ static int nvme_dev_add(struct nvme_dev *dev)
 
 	mem = dma_alloc_coherent(&dev->pci_dev->dev, 8192, &dma_addr,
 								GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
 
 	res = nvme_identify(dev, 0, 1, dma_addr);
 	if (res) {
-- 
1.7.0.4

[PATCH] NVMe: Remove dead code in nvme_dev_add

[Keith Busch]

There is no situation that could occur where we could error out of this
function and require cleaning up allocated namespaces.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index e7d68e3..82a5f81 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1511,7 +1511,7 @@ static void nvme_free_queues(struct nvme_dev *dev)
 static int nvme_dev_add(struct nvme_dev *dev)
 {
 	int res, nn, i;
-	struct nvme_ns *ns, *next;
+	struct nvme_ns *ns;
 	struct nvme_id_ctrl *ctrl;
 	struct nvme_id_ns *id_ns;
 	void *mem;
@@ -1564,15 +1564,8 @@ static int nvme_dev_add(struct nvme_dev *dev)
 	list_for_each_entry(ns, &dev->namespaces, list)
 		add_disk(ns->disk);
 	res = 0;
-	goto out;
 
  out_free:
-	list_for_each_entry_safe(ns, next, &dev->namespaces, list) {
-		list_del(&ns->list);
-		nvme_ns_free(ns);
-	}
-
- out:
 	dma_free_coherent(&dev->pci_dev->dev, 8192, mem, dma_addr);
 	return res;
 }
-- 
1.7.0.4

[PATCH] NVMe: Remove dead code in nvme_dev_add

[Matthew Wilcox]

On Wed, May 01, 2013@01:07:49PM -0600, Keith Busch wrote:
> There is no situation that could occur where we could error out of this
> function and require cleaning up allocated namespaces.

I'd rather the exit label were called 'out' than 'out_free' at the end
of this patch.

> Signed-off-by: Keith Busch <keith.busch at intel.com>
> ---
>  drivers/block/nvme-core.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index e7d68e3..82a5f81 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -1511,7 +1511,7 @@ static void nvme_free_queues(struct nvme_dev *dev)
>  static int nvme_dev_add(struct nvme_dev *dev)
>  {
>  	int res, nn, i;
> -	struct nvme_ns *ns, *next;
> +	struct nvme_ns *ns;
>  	struct nvme_id_ctrl *ctrl;
>  	struct nvme_id_ns *id_ns;
>  	void *mem;
> @@ -1564,15 +1564,8 @@ static int nvme_dev_add(struct nvme_dev *dev)
>  	list_for_each_entry(ns, &dev->namespaces, list)
>  		add_disk(ns->disk);
>  	res = 0;
> -	goto out;
>  
>   out_free:
> -	list_for_each_entry_safe(ns, next, &dev->namespaces, list) {
> -		list_del(&ns->list);
> -		nvme_ns_free(ns);
> -	}
> -
> - out:
>  	dma_free_coherent(&dev->pci_dev->dev, 8192, mem, dma_addr);
>  	return res;
>  }
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

[PATCH] NVMe: Free allocated memory in probe failure

[Keith Busch]

When initializing in probe fails, free only the allocated memory.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 82a5f81..2801875 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1703,11 +1703,11 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	dev->entry = kcalloc(num_possible_cpus(), sizeof(*dev->entry),
 								GFP_KERNEL);
 	if (!dev->entry)
-		goto free;
+		goto free_dev;
 	dev->queues = kcalloc(num_possible_cpus() + 1, sizeof(void *),
 								GFP_KERNEL);
 	if (!dev->queues)
-		goto free;
+		goto free_entry;
 
 	if (pci_enable_device_mem(pdev))
 		goto free;
@@ -1781,7 +1781,9 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_release_regions(pdev);
  free:
 	kfree(dev->queues);
+ free_entry:
 	kfree(dev->entry);
+ free_dev:
 	kfree(dev);
 	return result;
 }
-- 
1.7.0.4

[PATCH] NVMe: Free allocated memory in probe failure

[Matthew Wilcox]

On Wed, May 01, 2013@01:07:50PM -0600, Keith Busch wrote:
> When initializing in probe fails, free only the allocated memory.

'dev'is allocated using kzalloc, and kfree(NULL) is defined to be a no-op.
So having one 'free' label that frees all the memory is fine.

>  	dev->entry = kcalloc(num_possible_cpus(), sizeof(*dev->entry),
>  								GFP_KERNEL);
>  	if (!dev->entry)
> -		goto free;
> +		goto free_dev;
>  	dev->queues = kcalloc(num_possible_cpus() + 1, sizeof(void *),
>  								GFP_KERNEL);
>  	if (!dev->queues)
> -		goto free;
> +		goto free_entry;
>  
>  	if (pci_enable_device_mem(pdev))
>  		goto free;
> @@ -1781,7 +1781,9 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	pci_release_regions(pdev);
>   free:
>  	kfree(dev->queues);
> + free_entry:
>  	kfree(dev->entry);
> + free_dev:
>  	kfree(dev);

[PATCH] NVMe: Free admin queue on request_irq error

[Keith Busch]

Fixes a potential memory leak if requesting the admin queue irq fails.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme-core.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 2801875..c13ca5b 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -1054,14 +1054,19 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 		}
 	}
 
-	if (result) {
-		nvme_free_queue_mem(nvmeq);
-		return result;
-	}
+	if (result)
+		goto free_q;
 
 	result = queue_request_irq(dev, nvmeq, "nvme admin");
+	if (result)
+		goto free_q;
+
 	dev->queues[0] = nvmeq;
 	return result;
+
+ free_q:
+	nvme_free_queue_mem(nvmeq);
+	return result;
 }
 
 struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
-- 
1.7.0.4