Re: [PATCH 2/2] nfit, tools/testing/nvdimm/: unify shutdown paths

[joeyli <jlee@suse.com>]

Hi Dan,

On Thu, Jul 21, 2016 at 08:25:50PM -0700, Dan Williams wrote:
> While testing the new on-demand ARS patches we discovered that
> differences between the nfit_test and normal nfit driver shutdown paths
> can leak resources.  Unify the shutdown paths to trigger via a devm_
> callback when the acpi_desc->dev is unbound from its driver.
> 
> Reported-by: Vishal Verma <vishal.l.verma@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Lee, Chun-Yi <jlee@suse.com>

Thanks a lot!
Joey Lee

> ---
>  drivers/acpi/nfit.c              |   36 ++++++++++++++++++++++--------------
>  tools/testing/nvdimm/test/nfit.c |   16 ++--------------
>  2 files changed, 24 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index e7eb3b6f1514..be7c2fde16e7 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -2291,6 +2291,16 @@ static int acpi_nfit_check_deletions(struct acpi_nfit_desc *acpi_desc,
>  	return 0;
>  }
>  
> +static void acpi_nfit_destruct(void *data)
> +{
> +	struct acpi_nfit_desc *acpi_desc = data;
> +
> +	acpi_desc->cancel = 1;
> +	flush_workqueue(nfit_wq);
> +	nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> +	acpi_desc->nvdimm_bus = NULL;
> +}
> +
>  int acpi_nfit_init(struct acpi_nfit_desc *acpi_desc, void *data, acpi_size sz)
>  {
>  	struct device *dev = acpi_desc->dev;
> @@ -2298,6 +2308,17 @@ int acpi_nfit_init(struct acpi_nfit_desc *acpi_desc, void *data, acpi_size sz)
>  	const void *end;
>  	int rc;
>  
> +	if (!acpi_desc->nvdimm_bus) {
> +		acpi_desc->nvdimm_bus = nvdimm_bus_register(dev,
> +				&acpi_desc->nd_desc);
> +		if (!acpi_desc->nvdimm_bus)
> +			return -ENOMEM;
> +		rc = devm_add_action_or_reset(dev, acpi_nfit_destruct,
> +				acpi_desc);
> +		if (rc)
> +			return rc;
> +	}
> +
>  	mutex_lock(&acpi_desc->init_mutex);
>  
>  	INIT_LIST_HEAD(&prev.spas);
> @@ -2456,9 +2477,6 @@ static int acpi_nfit_add(struct acpi_device *adev)
>  	if (!acpi_desc)
>  		return -ENOMEM;
>  	acpi_nfit_desc_init(acpi_desc, &adev->dev);
> -	acpi_desc->nvdimm_bus = nvdimm_bus_register(dev, &acpi_desc->nd_desc);
> -	if (!acpi_desc->nvdimm_bus)
> -		return -ENOMEM;
>  
>  	/* Save the acpi header for exporting the revision via sysfs */
>  	acpi_desc->acpi_header = *tbl;
> @@ -2480,19 +2498,12 @@ static int acpi_nfit_add(struct acpi_device *adev)
>  		rc = acpi_nfit_init(acpi_desc, (void *) tbl
>  				+ sizeof(struct acpi_table_nfit),
>  				sz - sizeof(struct acpi_table_nfit));
> -
> -	if (rc)
> -		nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
>  	return rc;
>  }
>  
>  static int acpi_nfit_remove(struct acpi_device *adev)
>  {
> -	struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(&adev->dev);
> -
> -	acpi_desc->cancel = 1;
> -	flush_workqueue(nfit_wq);
> -	nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> +	/* see acpi_nfit_destruct */
>  	return 0;
>  }
>  
> @@ -2519,9 +2530,6 @@ static void acpi_nfit_notify(struct acpi_device *adev, u32 event)
>  		if (!acpi_desc)
>  			goto out_unlock;
>  		acpi_nfit_desc_init(acpi_desc, &adev->dev);
> -		acpi_desc->nvdimm_bus = nvdimm_bus_register(dev, &acpi_desc->nd_desc);
> -		if (!acpi_desc->nvdimm_bus)
> -			goto out_unlock;
>  	} else {
>  		/*
>  		 * Finish previous registration before considering new
> diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
> index 642713f15723..5404efa578a3 100644
> --- a/tools/testing/nvdimm/test/nfit.c
> +++ b/tools/testing/nvdimm/test/nfit.c
> @@ -1465,16 +1465,11 @@ static int nfit_test_probe(struct platform_device *pdev)
>  	nd_desc->provider_name = NULL;
>  	nd_desc->module = THIS_MODULE;
>  	nd_desc->ndctl = nfit_test_ctl;
> -	acpi_desc->nvdimm_bus = nvdimm_bus_register(&pdev->dev, nd_desc);
> -	if (!acpi_desc->nvdimm_bus)
> -		return -ENXIO;
>  
>  	rc = acpi_nfit_init(acpi_desc, nfit_test->nfit_buf,
>  			nfit_test->nfit_size);
> -	if (rc) {
> -		nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> +	if (rc)
>  		return rc;
> -	}
>  
>  	if (nfit_test->setup != nfit_test0_setup)
>  		return 0;
> @@ -1484,21 +1479,14 @@ static int nfit_test_probe(struct platform_device *pdev)
>  
>  	rc = acpi_nfit_init(acpi_desc, nfit_test->nfit_buf,
>  			nfit_test->nfit_size);
> -	if (rc) {
> -		nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> +	if (rc)
>  		return rc;
> -	}
>  
>  	return 0;
>  }
>  
>  static int nfit_test_remove(struct platform_device *pdev)
>  {
> -	struct nfit_test *nfit_test = to_nfit_test(&pdev->dev);
> -	struct acpi_nfit_desc *acpi_desc = &nfit_test->acpi_desc;
> -
> -	nvdimm_bus_unregister(acpi_desc->nvdimm_bus);
> -
>  	return 0;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html