Re: [PATCH v2 1/6] devfreq: Use mutex guard in governor_store()

[Yaxiong Tian <tianyaxiong@kylinos.cn>]

在 2026/5/13 17:38, Jie Zhan 写道:
> Use mutex guard in governor_store() so as to simplify the locking logic.
>
> No functional impact intended.
>
> Signed-off-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c | 38 ++++++++++++++++----------------------
>   1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index f08fc6966eae..7a70dd051644 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -1394,23 +1394,20 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	if (ret != 1)
>   		return -EINVAL;
>   
> -	mutex_lock(&devfreq_list_lock);
> +	guard(mutex)(&devfreq_list_lock);
>   	governor = try_then_request_governor(str_governor);
> -	if (IS_ERR(governor)) {
> -		ret = PTR_ERR(governor);
> -		goto out;
> -	}
> +	if (IS_ERR(governor))
> +		return PTR_ERR(governor);
> +
>   	if (!df->governor)
>   		goto start_new_governor;
>   
> -	if (df->governor == governor) {
> -		ret = 0;
> -		goto out;
> -	} else if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE)
> -		|| IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> +	if (df->governor == governor)
> +		return count;
> +
> +	if (IS_SUPPORTED_FLAG(df->governor->flags, IMMUTABLE) ||
> +	    IS_SUPPORTED_FLAG(governor->flags, IMMUTABLE))
> +		return -EINVAL;
>   
>   	/*
>   	 * Stop the current governor and remove the specific sysfs files
> @@ -1420,7 +1417,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	if (ret) {
>   		dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
>   			 __func__, df->governor->name, ret);
> -		goto out;
> +		return ret;
>   	}
>   
>   start_new_governor:
> @@ -1438,7 +1435,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   		/* Restore previous governor */
>   		df->governor = prev_governor;
>   		if (!df->governor)
> -			goto out;
> +			return ret;
>   
>   		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   		if (ret) {
> @@ -1446,7 +1443,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   				"%s: reverting to Governor %s failed (%d)\n",
>   				__func__, prev_governor->name, ret);
>   			df->governor = NULL;
> -			goto out;
> +			return ret;
>   		}
>   	}
>   
> @@ -1455,13 +1452,10 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>   	 * the new governor, restore the sysfs files of previous governor.
>   	 */
>   	ret = sysfs_update_group(&df->dev.kobj, &gov_attr_group);
> +	if (ret)
> +		return ret;
>   
> -out:
> -	mutex_unlock(&devfreq_list_lock);
> -
> -	if (!ret)
> -		ret = count;
> -	return ret;
> +	return count;
>   }
>   static DEVICE_ATTR_RW(governor);
>   
Reviewed-by: Yaxiong Tian <tianyaxiong@kylinos.cn>