Re: [PATCH 3/9] platform/x86: lenovo-wmi-capdata: Defer mutex initialization

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 14 Sep 2026, Rong Zhang wrote:

> In the following changes, priv->list may be freed if the first call to
> lwmi_cd_cache() fails due to WMI/ACPI errors, so the list_mutex must be
> initialized after it in order not to break lockdep, as there is no
> devm_mutex_destroy(). Considering that the first call to lwmi_cd_cache()
> doesn't need serialization as there is no other reader or writer this
> early, the initialization of list_mutex can be deferred.
> 
> Therefore, initialize list_mutex only after the first call to
> lwmi_cd_cache() succeeds, otherwise it remains uninitialized and can be
> devm_kfree()-ed.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.c | 80 +++++++++++++++++++++++--------
>  1 file changed, 60 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
> index 880ac444c206..0123ec8f7b53 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> @@ -91,6 +91,7 @@ struct lwmi_cd_priv {
>  	struct wmi_device *wdev;
>  	struct cd_list *list;
>  	struct dentry *debugfs_dir;
> +	bool initialized;
>  
>  	/*
>  	 * A capdata device may be a component master of another capdata device.
> @@ -588,14 +589,14 @@ static void lwmi_cd_debugfs_remove(struct lwmi_cd_priv *priv)
>  /* ======== WMI interface ======== */
>  
>  /**
> - * lwmi_cd_cache() - Cache all WMI data block information
> + * __lwmi_cd_cache() - Cache all WMI data block information locklessly
>   * @priv: lenovo-wmi-capdata driver data.
>   *
> - * Loop through each WMI data block and cache the data.
> + * Loop through each WMI data block and cache the data locklessly.
>   *
>   * Return: 0 on success, or an error.
>   */
> -static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> +static int __lwmi_cd_cache(struct lwmi_cd_priv *priv)
>  {
>  	size_t size;
>  	int idx;
> @@ -617,7 +618,6 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>  		return -EINVAL;
>  	}
>  
> -	guard(mutex)(&priv->list->list_mutex);
>  	for (idx = 0; idx < priv->list->count; idx++, p += size) {
>  		union acpi_object *ret_obj __free(kfree) = NULL;
>  
> @@ -635,14 +635,37 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>  	return 0;
>  }
>  
> +/**
> + * lwmi_cd_cache() - Cache all WMI data block information
> + * @priv: lenovo-wmi-capdata driver data.
> + *
> + * Loop through each WMI data block and cache the data.
> + *
> + * Return: 0 on success, or an error.
> + */
> +static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
> +{
> +	if (!priv->initialized)
> +		return __lwmi_cd_cache(priv);
> +
> +	switch (priv->info->type) {
> +	case LENOVO_CAPABILITY_DATA_01:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	guard(mutex)(&priv->list->list_mutex);
> +	return __lwmi_cd_cache(priv);
> +}
> +
>  /**
>   * lwmi_cd_fan_list_alloc_cache() - Alloc and cache Fan Test Data list
>   * @priv: lenovo-wmi-capdata driver data.
> - * @listptr: Pointer to returned cd_list pointer.
>   *
>   * Return: count of fans found, or an error.
>   */
> -static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_list **listptr)
> +static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
>  {
>  	struct cd_list *list;
>  	size_t size;
> @@ -688,6 +711,9 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_lis
>  	if (!list)
>  		return -ENOMEM;
>  
> +	list->count = count;
> +	priv->list = list;
> +
>  	for (idx = 0; idx < count; idx++) {
>  		/* Do not calculate array index using count, as it may be truncated. */
>  		list->cd_fan[idx] = (struct capdata_fan) {
> @@ -697,8 +723,7 @@ static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_lis
>  		};
>  	}
>  
> -	*listptr = list;
> -	return count;
> +	return 0;
>  }
>  
>  /**
> @@ -714,7 +739,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>  {
>  	struct cd_list *list;
>  	size_t list_size;
> -	int count, ret;
> +	int count;
>  
>  	count = wmidev_instance_count(priv->wdev);
>  
> @@ -726,11 +751,7 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>  		list_size = struct_size(list, cd01, count);
>  		break;
>  	case LENOVO_FAN_TEST_DATA:
> -		count = lwmi_cd_fan_list_alloc_cache(priv, &list);
> -		if (count < 0)
> -			return count;
> -
> -		goto got_list;
> +		return lwmi_cd_fan_list_alloc_cache(priv);
>  	default:
>  		return -EINVAL;
>  	}
> @@ -739,17 +760,32 @@ static int lwmi_cd_alloc(struct lwmi_cd_priv *priv)
>  	if (!list)
>  		return -ENOMEM;
>  
> -got_list:
> -	ret = devm_mutex_init(&priv->wdev->dev, &list->list_mutex);
> -	if (ret)
> -		return ret;
> -
>  	list->count = count;
>  	priv->list = list;
>  
>  	return 0;
>  }
>  
> +/**
> + * lwmi_cd_finalize() - Finalize the capability data initialization
> + * @priv: lenovo-wmi-capdata driver data.
> + *
> + * Return: 0 on success, or an error code.
> + */
> +static int lwmi_cd_finalize(struct lwmi_cd_priv *priv)
> +{
> +	int ret;
> +
> +	if (priv->list) {
> +		ret = devm_mutex_init(&priv->wdev->dev, &priv->list->list_mutex);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	priv->initialized = 1;

Don't you need a barrier to ensure these are really done in the correct 
order?

> +	return 0;
> +}
> +
>  /**
>   * lwmi_cd_setup() - Cache all WMI data block information
>   * @priv: lenovo-wmi-capdata driver data.
> @@ -768,7 +804,11 @@ static int lwmi_cd_setup(struct lwmi_cd_priv *priv)
>  	if (ret)
>  		return ret;
>  
> -	return lwmi_cd_cache(priv);
> +	ret = lwmi_cd_cache(priv);
> +	if (ret)
> +		return ret;
> +
> +	return lwmi_cd_finalize(priv);
>  }
>  
>  /**
> 
> 

-- 
 i.