Re: [PATCH] dax/hmem: Fix lockdep warning for hmem_register_resource()

[Jonathan Cameron <jonathan.cameron@huawei.com>]

> The lock ordering can cause potential deadlock. There are instances
> where hmem_resource_lock is taken after (node_chain).rwsem, and vice
> versa. Narrow the scope of hmem_resource_lock in hmem_register_resource()
> to avoid the circular locking dependency. The locking is only needed when
> hmem_active needs to be protected.
> 
> Fixes: 7dab174e2e27 ("dax/hmem: Move hmem device registration to dax_hmem.ko")
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

One trivial thing...

> ---
>  drivers/dax/hmem/device.c | 42 +++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/dax/hmem/device.c b/drivers/dax/hmem/device.c
> index f9e1a76a04a9..ab5977d75d1f 100644
> --- a/drivers/dax/hmem/device.c
> +++ b/drivers/dax/hmem/device.c
> @@ -33,21 +33,37 @@ int walk_hmem_resources(struct device *host, walk_hmem_fn fn)
>  }
>  EXPORT_SYMBOL_GPL(walk_hmem_resources);
>  
> -static void __hmem_register_resource(int target_nid, struct resource *res)
> +static struct resource *hmem_request_resource(int target_nid,
> +					      struct resource *res)
>  {
> -	struct platform_device *pdev;
>  	struct resource *new;
> -	int rc;
>  
> -	new = __request_region(&hmem_active, res->start, resource_size(res), "",
> -			       0);
> +	guard(mutex)(&hmem_resource_lock);
> +	new = __request_region(&hmem_active, res->start,
> +			       resource_size(res), "", 0);

Why the rewrap?  Arguably it is slightly prettier but original was 80 chars
(I think) and making this change adds noise to the real code in this patch.

>  	if (!new) {
>  		pr_debug("hmem range %pr already active\n", res);
> -		return;
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
>  	new->desc = target_nid;
>  
> +	return new;
> +}