[PATCH v2 5/6] devfreq: Get and put module refcount when switching governor

[Yaxiong Tian]

在 2026/5/13 17:38, Jie Zhan 写道:
> A governor module can be dynamically inserted or removed if compiled as a
> kernel module.  'devfreq->governor' would become NULL if the governor
> module is removed when it's in use.
>
> To prevent the governor module from being removed (except for force
> unload) when it's in use, get and put a refcount of the governor module
> when starting and stopping the governor.
>
> As a result, unloading a governor module in use returns an error, e.g.:
>    $ cat governor
>    performance
>    $ rmmod governor_performance
>    rmmod: ERROR: Module governor_performance is in use
>
> Signed-off-by: Jie Zhan <zhanjie9 at hisilicon.com>
> ---
>   drivers/devfreq/devfreq.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index e1363ab69173..2ea42325d030 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -345,24 +345,37 @@ static int devfreq_set_governor(struct devfreq *df,
>   				 __func__, df->governor->name, ret);
>   			return ret;
>   		}
> +		module_put(old_gov->owner);
>   	}
>   
>   	/* Start the new governor */
> +	if (!try_module_get(new_gov->owner)) {
> +		df->governor = NULL;
> +		return -EINVAL;
> +	}
> +

Here, new_gov has already been checked in try_then_request_governor() 
for whether the module can be obtained.

I think it might be more reasonable to merge try_then_request_governor() 
and devfreq_set_governor(), so that when new_gov cannot be obtained, the 
operation of the old governor is not affected.

>   	df->governor = new_gov;
>   	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   	if (ret) {
>   		dev_warn(dev, "%s: Governor %s not started(%d)\n",
>   			 __func__, df->governor->name, ret);
> +		module_put(new_gov->owner);
>   
>   		/* Restore previous governor */
>   		df->governor = old_gov;
>   		if (!df->governor)
>   			return ret;
>   
> +		if (!try_module_get(old_gov->owner)) {
> +			df->governor = NULL;
> +			return -EINVAL;
> +		}
> +
>   		ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>   		if (ret) {
>   			dev_err(dev, "%s: restore Governor %s failed (%d)\n",
>   				__func__, df->governor->name, ret);
> +			module_put(old_gov->owner);
>   			df->governor = NULL;
>   			return ret;
>   		}
> @@ -1040,9 +1053,11 @@ int devfreq_remove_device(struct devfreq *devfreq)
>   
>   	devfreq_cooling_unregister(devfreq->cdev);
>   
> -	if (devfreq->governor)
> +	if (devfreq->governor) {
>   		devfreq->governor->event_handler(devfreq,
>   						 DEVFREQ_GOV_STOP, NULL);
> +		module_put(devfreq->governor->owner);
> +	}
>   	device_unregister(&devfreq->dev);
>   
>   	return 0;