Re: [PATCH v2 1/2] soc: fsl: dpio: Use scope-based resource management in dpaa2_io_store_create()

[Dan Carpenter]

On Mon, Jul 06, 2026 at 12:38:08PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Mon, 6 Jul 2026 10:34:43 +0200
> 
> Scope-based resource management became supported for some
> programming interfaces by contributions of Peter Zijlstra on 2023-05-26.
> See also the commit 54da6a0924311c7cf5015533991e44fb8eb12773 ("locking:
> Introduce __cleanup() based infrastructure").
> 
> * Thus use the attribute “__free(kfree)”.
> 
> * Reduce the scope for the local variable “ret”.
> 
> * Omit two kfree() calls accordingly.
> 
> * Omit the local variable “size” (for another memory allocation).

Why?  This seems unrelated...

> 
> * Use the macro call “return_ptr(ret)” at the end.
> 
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/soc/fsl/dpio/dpio-service.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
> index 317ca50b0c2b..b252c3c7fa65 100644
> --- a/drivers/soc/fsl/dpio/dpio-service.c
> +++ b/drivers/soc/fsl/dpio/dpio-service.c
> @@ -652,23 +652,17 @@ EXPORT_SYMBOL_GPL(dpaa2_io_service_acquire);

Missing #include <linux/cleanup.h>

>  struct dpaa2_io_store *dpaa2_io_store_create(unsigned int max_frames,
>  					     struct device *dev)
>  {
> -	struct dpaa2_io_store *ret;
> -	size_t size;
> -
>  	if (!max_frames || (max_frames > 32))
>  		return NULL;
>  
> -	ret = kmalloc_obj(*ret);
> +	struct dpaa2_io_store *ret __free(kfree) = kmalloc_obj(*ret);
>  	if (!ret)
>  		return NULL;
>  
>  	ret->max = max_frames;
> -	size = max_frames * sizeof(struct dpaa2_dq) + 64;
> -	ret->alloced_addr = kzalloc(size, GFP_KERNEL);
> -	if (!ret->alloced_addr) {
> -		kfree(ret);
> +	ret->alloced_addr = kzalloc(max_frames * sizeof(struct dpaa2_dq) + 64, GFP_KERNEL);
> +	if (!ret->alloced_addr)
>  		return NULL;

Why convert the ret allocation but not the ret->alloced_addr allocation?

regards,
dan carpenter

> -	}
>  
>  	ret->vaddr = PTR_ALIGN(ret->alloced_addr, 64);
>  	ret->paddr = dma_map_single(dev, ret->vaddr,
> @@ -676,14 +670,13 @@ struct dpaa2_io_store *dpaa2_io_store_create(unsigned int max_frames,
>  				    DMA_FROM_DEVICE);
>  	if (dma_mapping_error(dev, ret->paddr)) {
>  		kfree(ret->alloced_addr);
> -		kfree(ret);
>  		return NULL;
>  	}
>  
>  	ret->idx = 0;
>  	ret->dev = dev;
>  
> -	return ret;
> +	return_ptr(ret);
>  }
>  EXPORT_SYMBOL_GPL(dpaa2_io_store_create);
>  
> -- 
> 2.54.0
>