Re: [PATCH 4/4] dax: Cleanup extra dax_region references

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

On 6/2/23 23:14, Dan Williams wrote:
> Now that free_dev_dax_id() internally manages the references it needs
> the extra references taken by the dax_region drivers are not needed.
> 
> Reported-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/dax/bus.c       |    4 +---
>   drivers/dax/bus.h       |    1 -
>   drivers/dax/cxl.c       |    8 +-------
>   drivers/dax/hmem/hmem.c |    8 +-------
>   drivers/dax/pmem.c      |    7 +------
>   5 files changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index a4cc3eca774f..0ee96e6fc426 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -454,11 +454,10 @@ static void dax_region_free(struct kref *kref)
>   	kfree(dax_region);
>   }
>   
> -void dax_region_put(struct dax_region *dax_region)
> +static void dax_region_put(struct dax_region *dax_region)
>   {
>   	kref_put(&dax_region->kref, dax_region_free);
>   }
> -EXPORT_SYMBOL_GPL(dax_region_put);
>   
>   /* a return value >= 0 indicates this invocation invalidated the id */
>   static int __free_dev_dax_id(struct dev_dax *dev_dax)
> @@ -641,7 +640,6 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id,
>   		return NULL;
>   	}
>   
> -	kref_get(&dax_region->kref);
>   	if (devm_add_action_or_reset(parent, dax_region_unregister, dax_region))
>   		return NULL;
>   	return dax_region;
> diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
> index 8cd79ab34292..bdbf719df5c5 100644
> --- a/drivers/dax/bus.h
> +++ b/drivers/dax/bus.h
> @@ -9,7 +9,6 @@ struct dev_dax;
>   struct resource;
>   struct dax_device;
>   struct dax_region;
> -void dax_region_put(struct dax_region *dax_region);
>   
>   /* dax bus specific ioresource flags */
>   #define IORESOURCE_DAX_STATIC BIT(0)
> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> index ccdf8de85bd5..8bc9d04034d6 100644
> --- a/drivers/dax/cxl.c
> +++ b/drivers/dax/cxl.c
> @@ -13,7 +13,6 @@ static int cxl_dax_region_probe(struct device *dev)
>   	struct cxl_region *cxlr = cxlr_dax->cxlr;
>   	struct dax_region *dax_region;
>   	struct dev_dax_data data;
> -	struct dev_dax *dev_dax;
>   
>   	if (nid == NUMA_NO_NODE)
>   		nid = memory_add_physaddr_to_nid(cxlr_dax->hpa_range.start);
> @@ -28,13 +27,8 @@ static int cxl_dax_region_probe(struct device *dev)
>   		.id = -1,
>   		.size = range_len(&cxlr_dax->hpa_range),
>   	};
> -	dev_dax = devm_create_dev_dax(&data);
> -	if (IS_ERR(dev_dax))
> -		return PTR_ERR(dev_dax);
>   
> -	/* child dev_dax instances now own the lifetime of the dax_region */
> -	dax_region_put(dax_region);
> -	return 0;
> +	return PTR_ERR_OR_ZERO(devm_create_dev_dax(&data));
>   }
>   
>   static struct cxl_driver cxl_dax_region_driver = {
> diff --git a/drivers/dax/hmem/hmem.c b/drivers/dax/hmem/hmem.c
> index e5fe8b39fb94..5d2ddef0f8f5 100644
> --- a/drivers/dax/hmem/hmem.c
> +++ b/drivers/dax/hmem/hmem.c
> @@ -16,7 +16,6 @@ static int dax_hmem_probe(struct platform_device *pdev)
>   	struct dax_region *dax_region;
>   	struct memregion_info *mri;
>   	struct dev_dax_data data;
> -	struct dev_dax *dev_dax;
>   
>   	/*
>   	 * @region_idle == true indicates that an administrative agent
> @@ -38,13 +37,8 @@ static int dax_hmem_probe(struct platform_device *pdev)
>   		.id = -1,
>   		.size = region_idle ? 0 : range_len(&mri->range),
>   	};
> -	dev_dax = devm_create_dev_dax(&data);
> -	if (IS_ERR(dev_dax))
> -		return PTR_ERR(dev_dax);
>   
> -	/* child dev_dax instances now own the lifetime of the dax_region */
> -	dax_region_put(dax_region);
> -	return 0;
> +	return PTR_ERR_OR_ZERO(devm_create_dev_dax(&data));
>   }
>   
>   static struct platform_driver dax_hmem_driver = {
> diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
> index f050ea78bb83..ae0cb113a5d3 100644
> --- a/drivers/dax/pmem.c
> +++ b/drivers/dax/pmem.c
> @@ -13,7 +13,6 @@ static struct dev_dax *__dax_pmem_probe(struct device *dev)
>   	int rc, id, region_id;
>   	resource_size_t offset;
>   	struct nd_pfn_sb *pfn_sb;
> -	struct dev_dax *dev_dax;
>   	struct dev_dax_data data;
>   	struct nd_namespace_io *nsio;
>   	struct dax_region *dax_region;
> @@ -65,12 +64,8 @@ static struct dev_dax *__dax_pmem_probe(struct device *dev)
>   		.pgmap = &pgmap,
>   		.size = range_len(&range),
>   	};
> -	dev_dax = devm_create_dev_dax(&data);
>   
> -	/* child dev_dax instances now own the lifetime of the dax_region */
> -	dax_region_put(dax_region);
> -
> -	return dev_dax;
> +	return devm_create_dev_dax(&data);
>   }
>   
>   static int dax_pmem_probe(struct device *dev)
> 
>