Re: [PATCH 4/6] cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup

[Dave Jiang <dave.jiang@intel.com>]

On 12/3/25 7:21 PM, Dan Williams wrote:
> In preparation for adding more setup steps, convert the current
> implementation to scope-based cleanup.
> 
> The cxl_memdev_shutdown() is only required after cdev_device_add(). With
> that moved to a helper function it precludes the need to add
> scope-based-handler for that cleanup if devm_add_action_or_reset() fails.
> 
> Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> Cc: Alejandro Lucero <alucerop@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/memdev.c | 58 +++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 7a4153e1c6a7..d0efc9ceda2a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -1050,6 +1050,33 @@ static const struct file_operations cxl_memdev_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> +/*
> + * Activate ioctl operations, no cxl_memdev_rwsem manipulation needed as this is
> + * ordered with cdev_add() publishing the device.
> + */
> +static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
> +{
> +	int rc;
> +
> +	cxlmd->cxlds = cxlds;
> +	cxlds->cxlmd = cxlmd;
> +
> +	rc = cdev_device_add(&cxlmd->cdev, &cxlmd->dev);
> +	if (rc) {
> +		/*
> +		 * The cdev was briefly live, shutdown any ioctl operations that
> +		 * saw that state.
> +		 */
> +		cxl_memdev_shutdown(&cxlmd->dev);
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
> +	    if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
> +
>  /*
>   * Core helper for devm_cxl_add_memdev() that wants to both create a device and
>   * assert to the caller that upon return cxl_mem::probe() has been invoked.
> @@ -1057,45 +1084,28 @@ static const struct file_operations cxl_memdev_fops = {
>  struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
>  					 struct cxl_dev_state *cxlds)
>  {
> -	struct cxl_memdev *cxlmd;
>  	struct device *dev;
> -	struct cdev *cdev;
>  	int rc;
>  
> -	cxlmd = cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
> +	struct cxl_memdev *cxlmd __free(put_cxlmd) =
> +		cxl_memdev_alloc(cxlds, &cxl_memdev_fops);
>  	if (IS_ERR(cxlmd))
>  		return cxlmd;
>  
>  	dev = &cxlmd->dev;
>  	rc = dev_set_name(dev, "mem%d", cxlmd->id);
>  	if (rc)
> -		goto err;
> -
> -	/*
> -	 * Activate ioctl operations, no cxl_memdev_rwsem manipulation
> -	 * needed as this is ordered with cdev_add() publishing the device.
> -	 */
> -	cxlmd->cxlds = cxlds;
> -	cxlds->cxlmd = cxlmd;
> +		return ERR_PTR(rc);
>  
> -	cdev = &cxlmd->cdev;
> -	rc = cdev_device_add(cdev, dev);
> +	rc = cxlmd_add(cxlmd, cxlds);
>  	if (rc)
> -		goto err;
> +		return ERR_PTR(rc);
>  
> -	rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
> +	rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
> +				      no_free_ptr(cxlmd));
>  	if (rc)
>  		return ERR_PTR(rc);
>  	return cxlmd;
> -
> -err:
> -	/*
> -	 * The cdev was briefly live, shutdown any ioctl operations that
> -	 * saw that state.
> -	 */
> -	cxl_memdev_shutdown(dev);
> -	put_device(dev);
> -	return ERR_PTR(rc);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
>