nvdimm: cleanup resources when the initialization fails

Commit Message

Wu Bo April 10, 2020, 3:15 a.m. UTC

From: Wu Bo <[e-mail]>

When the initialization fails, add the cleanup resources
in pmem_attach_disk() function

Signed-off-by: Wu Bo <[e-mail]>
---
  drivers/nvdimm/pmem.c | 44 +++++++++++++++++++++++++++++++-------------
  1 file changed, 31 insertions(+), 13 deletions(-)

         /* we're attaching a block device, disable raw namespace access */
@@ -417,12 +417,13 @@ static int pmem_attach_disk(struct device *dev,
         if (!devm_request_mem_region(dev, res->start, resource_size(res),
                                 dev_name(&ndns->dev))) {
                 dev_warn(dev, "could not reserve region %pR\n", res);
-               return -EBUSY;
+               rc = -EBUSY;
+               goto out_free_pmem;
         }

         q = blk_alloc_queue(pmem_make_request, dev_to_node(dev));
         if (!q)
-               return -ENOMEM;
+               goto out_realease_mem_region;

         pmem->pfn_flags = PFN_DEV;
         pmem->pgmap.ref = &q->q_usage_counter;
@@ -447,14 +448,16 @@ static int pmem_attach_disk(struct device *dev,
         } else {
                 if (devm_add_action_or_reset(dev, pmem_release_queue,
                                         &pmem->pgmap))
-                       return -ENOMEM;
+                       goto out_cleanup_queue;
                 addr = devm_memremap(dev, pmem->phys_addr,
                                 pmem->size, ARCH_MEMREMAP_PMEM);
                 memcpy(&bb_res, &nsio->res, sizeof(bb_res));
         }

-       if (IS_ERR(addr))
-               return PTR_ERR(addr);
+       if (IS_ERR(addr)) {
+               rc = PTR_ERR(addr);
+               goto out_cleanup_queue;
+       }
         pmem->virt_addr = addr;

         blk_queue_write_cache(q, true, fua);
@@ -468,7 +471,7 @@ static int pmem_attach_disk(struct device *dev,

         disk = alloc_disk_node(0, nid);
         if (!disk)
-               return -ENOMEM;
+               goto out_cleanup_queue;
         pmem->disk = disk;

         disk->fops              = &pmem_fops;
@@ -479,7 +482,7 @@ static int pmem_attach_disk(struct device *dev,
         set_capacity(disk, (pmem->size - pmem->pfn_pad - pmem->data_offset)
                         / 512);
         if (devm_init_badblocks(dev, &pmem->bb))
-               return -ENOMEM;
+               goto out_put_disk;
         nvdimm_badblocks_populate(nd_region, &pmem->bb, &bb_res);
         disk->bb = &pmem->bb;

@@ -487,8 +490,8 @@ static int pmem_attach_disk(struct device *dev,
                 flags = DAXDEV_F_SYNC;
         dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags);
         if (IS_ERR(dax_dev)) {
-               put_disk(disk);
-               return PTR_ERR(dax_dev);
+               rc = PTR_ERR(dax_dev);
+               goto out_badblock_exit;
         }
         dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
         pmem->dax_dev = dax_dev;
@@ -497,7 +500,7 @@ static int pmem_attach_disk(struct device *dev,

         device_add_disk(dev, disk, NULL);
         if (devm_add_action_or_reset(dev, pmem_release_disk, pmem))
-               return -ENOMEM;
+               goto out_free_dax;

         revalidate_disk(disk);

@@ -507,6 +510,21 @@ static int pmem_attach_disk(struct device *dev,
                 dev_warn(dev, "'badblocks' notification disabled\n");

         return 0;
+
+out_free_dax:
+       del_gendisk(disk);
+       put_dax(dax_dev);
+out_badblock_exit:
+       badblocks_exit(&pmem->bb);
+out_put_disk:
+       put_disk(disk);
+out_cleanup_queue:
+       blk_cleanup_queue(q);
+out_realease_mem_region:
+       devm_release_mem_region(dev, res->start, resource_size(res));
+out_free_pmem:
+       devm_kfree(dev, pmem);
+       return rc;
  }

  static int nd_pmem_probe(struct device *dev)
--
1.8.3.1

Comments

Dan Williams April 10, 2020, 3:20 a.m. UTC | #1

On Thu, Apr 9, 2020 at 8:16 PM Wu Bo <[e-mail]> wrote:
>
> From: Wu Bo <[e-mail]>
>
> When the initialization fails, add the cleanup resources
> in pmem_attach_disk() function

Are you familiar with devm?

devm routines take care of this automatically on driver probe error conditions.

Wu Bo April 10, 2020, 4:23 a.m. UTC | #2

On 2020/4/10 11:20, Dan Williams wrote:
> On Thu, Apr 9, 2020 at 8:16 PM Wu Bo <[e-mail]> wrote:
>>
>> From: Wu Bo <[e-mail]>
>>
>> When the initialization fails, add the cleanup resources
>> in pmem_attach_disk() function
> 
> Are you familiar with devm?
> 
> devm routines take care of this automatically on driver probe error conditions.
> 

Sorry, i did not pay attention to devm_add_action_or_reset() will 
automatically call clean up action on driver probe error conditions. 
Please ignore this patch.

thanks.

Patch

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 2df6994..f235a59 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -383,7 +383,7 @@  static int pmem_attach_disk(struct device *dev,
         struct device *gendev;
         struct gendisk *disk;
         void *addr;
-       int rc;
+       int rc = -ENOMEM;
         unsigned long flags = 0UL;

         pmem = devm_kzalloc(dev, sizeof(*pmem), GFP_KERNEL);
@@ -392,14 +392,14 @@  static int pmem_attach_disk(struct device *dev,

         rc = devm_namespace_enable(dev, ndns, nd_info_block_reserve());
         if (rc)
-               return rc;
+               goto out_free_pmem;

         /* while nsio_rw_bytes is active, parse a pfn info block if 
present */
         if (is_nd_pfn(dev)) {
                 nd_pfn = to_nd_pfn(dev);
                 rc = nvdimm_setup_pfn(nd_pfn, &pmem->pgmap);
                 if (rc)
-                       return rc;
+                       goto out_free_pmem;
         }