[PATCH 1/1] device-dax: fix mismatches of request_mem_region()

[PATCH 1/1] device-dax: fix mismatches of request_mem_region()

[Zhen Lei]

The resources allocated by request_mem_region() is better to use
release_mem_region() to free. These two functions are paired.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/dax/kmem.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 275aa5f873991af..9e38f9c2b6d7f02 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -82,8 +82,7 @@ int dev_dax_kmem_probe(struct device *dev)
 	rc = add_memory_driver_managed(numa_node, new_res->start,
 				       resource_size(new_res), kmem_name);
 	if (rc) {
-		release_resource(new_res);
-		kfree(new_res);
+		release_mem_region(kmem_start, kmem_size);
 		kfree(new_res_name);
 		return rc;
 	}
@@ -118,8 +117,7 @@ static int dev_dax_kmem_remove(struct device *dev)
 	}
 
 	/* Release and free dax resources */
-	release_resource(res);
-	kfree(res);
+	release_mem_region(kmem_start, kmem_size);
 	kfree(res_name);
 	dev_dax->dax_kmem_res = NULL;
 
-- 
1.8.3

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 1/1] device-dax: fix mismatches of request_mem_region()

[Ira Weiny]

On Mon, Aug 17, 2020 at 02:59:26PM +0800, Zhen Lei wrote:
> The resources allocated by request_mem_region() is better to use
> release_mem_region() to free. These two functions are paired.

Does this fix a bug or some other issue?

It _looks_ ok but there is just enough complexity here that I wonder if it is
worth changing this without a good reason.

OTOH is there some way to make release_mem_region() more specific about which
resource is getting free'ed?

Ira

> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  drivers/dax/kmem.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 275aa5f873991af..9e38f9c2b6d7f02 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -82,8 +82,7 @@ int dev_dax_kmem_probe(struct device *dev)
>  	rc = add_memory_driver_managed(numa_node, new_res->start,
>  				       resource_size(new_res), kmem_name);
>  	if (rc) {
> -		release_resource(new_res);
> -		kfree(new_res);
> +		release_mem_region(kmem_start, kmem_size);
>  		kfree(new_res_name);
>  		return rc;
>  	}
> @@ -118,8 +117,7 @@ static int dev_dax_kmem_remove(struct device *dev)
>  	}
>  
>  	/* Release and free dax resources */
> -	release_resource(res);
> -	kfree(res);
> +	release_mem_region(kmem_start, kmem_size);
>  	kfree(res_name);
>  	dev_dax->dax_kmem_res = NULL;
>  
> -- 
> 1.8.3
> 
> _______________________________________________
> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 1/1] device-dax: fix mismatches of request_mem_region()

[Leizhen (ThunderTown)]

On 8/18/2020 2:57 AM, Ira Weiny wrote:
> On Mon, Aug 17, 2020 at 02:59:26PM +0800, Zhen Lei wrote:
>> The resources allocated by request_mem_region() is better to use
>> release_mem_region() to free. These two functions are paired.
> 
> Does this fix a bug or some other issue?
> 
> It _looks_ ok but there is just enough complexity here that I wonder if it is
> worth changing this without a good reason.
It's just that everyone uses it in this way. In fact, there's a similar patch:
b88aa8509828b56 mfd: sm501: Fix mismatches of request_mem_region

I used the subject directly. And we can found some examples in other places:
vi drivers/acpi/acpi_extlog.c +283 	in extlog_init()


> 
> OTOH is there some way to make release_mem_region() more specific about which
> resource is getting free'ed?

* __release_region - release a previously reserved resource region

As long as we pass the parameters correctly, it will work well.

> 
> Ira
> 
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  drivers/dax/kmem.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
>> index 275aa5f873991af..9e38f9c2b6d7f02 100644
>> --- a/drivers/dax/kmem.c
>> +++ b/drivers/dax/kmem.c
>> @@ -82,8 +82,7 @@ int dev_dax_kmem_probe(struct device *dev)
>>  	rc = add_memory_driver_managed(numa_node, new_res->start,
>>  				       resource_size(new_res), kmem_name);
>>  	if (rc) {
>> -		release_resource(new_res);
>> -		kfree(new_res);
>> +		release_mem_region(kmem_start, kmem_size);
>>  		kfree(new_res_name);
>>  		return rc;
>>  	}
>> @@ -118,8 +117,7 @@ static int dev_dax_kmem_remove(struct device *dev)
>>  	}
>>  
>>  	/* Release and free dax resources */
>> -	release_resource(res);
>> -	kfree(res);
>> +	release_mem_region(kmem_start, kmem_size);
>>  	kfree(res_name);
>>  	dev_dax->dax_kmem_res = NULL;
>>  
>> -- 
>> 1.8.3
>>
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
> 
> .
> 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org