diff mbox

dax: use devm_add_action_or_reset()

Message ID 1467697807-27067-1-git-send-email-vikas.cha.sajjan@hpe.com (mailing list archive)
State Accepted
Commit d1c8e0c521c1
Headers show

Commit Message

Sajjan, Vikas C July 5, 2016, 5:50 a.m. UTC
If devm_add_action() fails, we are explicitly calling the cleanup to free
the resources allocated. Use the helper devm_add_action_or_reset()
and return directly in case of error, since the cleanup function
has been already called by the helper if there was any error.

Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Signed-off-by: Vikas C Sajjan <vikas.cha.sajjan@hpe.com>
---
 drivers/dax/dax.c  |  6 ++----
 drivers/dax/pmem.c | 14 ++++++--------
 2 files changed, 8 insertions(+), 12 deletions(-)

Comments

Johannes Thumshirn July 5, 2016, 6:59 a.m. UTC | #1
On Tue, Jul 05, 2016 at 11:20:07AM +0530, Vikas C Sajjan wrote:
> If devm_add_action() fails, we are explicitly calling the cleanup to free
> the resources allocated. Use the helper devm_add_action_or_reset()
> and return directly in case of error, since the cleanup function
> has been already called by the helper if there was any error.
> 
> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Signed-off-by: Vikas C Sajjan <vikas.cha.sajjan@hpe.com>

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Dan Williams July 6, 2016, 10:18 p.m. UTC | #2
Thanks, applied.

On Mon, Jul 4, 2016 at 11:59 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Tue, Jul 05, 2016 at 11:20:07AM +0530, Vikas C Sajjan wrote:
>> If devm_add_action() fails, we are explicitly calling the cleanup to free
>> the resources allocated. Use the helper devm_add_action_or_reset()
>> and return directly in case of error, since the cleanup function
>> has been already called by the helper if there was any error.
>>
>> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> Signed-off-by: Vikas C Sajjan <vikas.cha.sajjan@hpe.com>
>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
>
> --
> Johannes Thumshirn                                          Storage
> jthumshirn@suse.de                                +49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Sajjan, Vikas C July 7, 2016, 4:02 a.m. UTC | #3
-----Original Message-----
From: Dan Williams [mailto:dan.j.williams@intel.com] 

Sent: Thursday, July 07, 2016 3:49 AM
To: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Sajjan, Vikas C <vikas.cha.sajjan@hpe.com>; Ross Zwisler <ross.zwisler@linux.intel.com>; linux-nvdimm@lists.01.org; Linux ACPI <linux-acpi@vger.kernel.org>; Knippers, Linda <linda.knippers@hpe.com>
Subject: Re: [PATCH] dax: use devm_add_action_or_reset()

Thanks, applied.

Thank you, Dan.
 
On Mon, Jul 4, 2016 at 11:59 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> On Tue, Jul 05, 2016 at 11:20:07AM +0530, Vikas C Sajjan wrote:

>> If devm_add_action() fails, we are explicitly calling the cleanup to 

>> free the resources allocated. Use the helper 

>> devm_add_action_or_reset() and return directly in case of error, 

>> since the cleanup function has been already called by the helper if there was any error.

>>

>> Reported-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

>> Signed-off-by: Vikas C Sajjan <vikas.cha.sajjan@hpe.com>

>

> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

>

> --

> Johannes Thumshirn                                          Storage

> jthumshirn@suse.de                                +49 911 74053 689

> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg

> GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG 

> Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 

> 2D76 0850
diff mbox

Patch

diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c
index b891a12..803f395 100644
--- a/drivers/dax/dax.c
+++ b/drivers/dax/dax.c
@@ -211,11 +211,9 @@  int devm_create_dax_dev(struct dax_region *dax_region, struct resource *res,
 	}
 	dax_dev->dev = dev;
 
-	rc = devm_add_action(dax_region->dev, unregister_dax_dev, dev);
-	if (rc) {
-		unregister_dax_dev(dev);
+	rc = devm_add_action_or_reset(dax_region->dev, unregister_dax_dev, dev);
+	if (rc)
 		return rc;
-	}
 
 	return 0;
 
diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
index 55d510e..dfb1685 100644
--- a/drivers/dax/pmem.c
+++ b/drivers/dax/pmem.c
@@ -102,21 +102,19 @@  static int dax_pmem_probe(struct device *dev)
 	if (rc)
 		return rc;
 
-	rc = devm_add_action(dev, dax_pmem_percpu_exit, &dax_pmem->ref);
-	if (rc) {
-		dax_pmem_percpu_exit(&dax_pmem->ref);
+	rc = devm_add_action_or_reset(dev, dax_pmem_percpu_exit,
+							&dax_pmem->ref);
+	if (rc)
 		return rc;
-	}
 
 	addr = devm_memremap_pages(dev, &res, &dax_pmem->ref, altmap);
 	if (IS_ERR(addr))
 		return PTR_ERR(addr);
 
-	rc = devm_add_action(dev, dax_pmem_percpu_kill, &dax_pmem->ref);
-	if (rc) {
-		dax_pmem_percpu_kill(&dax_pmem->ref);
+	rc = devm_add_action_or_reset(dev, dax_pmem_percpu_kill,
+							&dax_pmem->ref);
+	if (rc)
 		return rc;
-	}
 
 	nd_region = to_nd_region(dev->parent);
 	dax_region = alloc_dax_region(dev, nd_region->id, &res,