[PATCH v3 4/4] devfreq: Refcount governor modules while in use

[Jie Zhan]

On 5/21/2026 4:13 PM, zhenglifeng (A) wrote:
> On 5/19/2026 7:32 PM, Jie Zhan wrote:
>> A governor module can be inserted or removed dynamically when built as a
>> kernel module.  'devfreq->governor' would become NULL if the governor
>> module is removed when it's in use.
>>
>> Add a refcount mechanism for governor modules to prevent the governor
>> module from being removed (except for force unload):
>> 1. Add an optional 'owner' member to struct devfreq_governor so the devfreq
>>    core can identify the module that holds the governor code.
>> 2. Get and put a refcount of the governor module when starting and stopping
>>    the governor.
>>
>> The new 'owner' field is optional:
>> - Common governor modules (performance, powersave, simple_ondemand,
>>   userspace, passive) set 'owner' to THIS_MODULE.
>> - Governors that are bundled into a device driver module must leave 'owner'
>>   NULL.  The device's lifetime already pins that module, and setting
>>   'owner' would create a self-reference that blocks the driver from being
>>   unloaded.
>>
>> As a result, a non-forced rmmod of an in-use stand-alone governor now
>> fails with -EBUSY, e.g.:
>>
>>   $ cat governor
>>   performance
>>   $ rmmod governor_performance
>>   rmmod: ERROR: Module governor_performance is in use
>>
>> Force unloads (rmmod -f, if configured) can't be blocked.
>>
>> Signed-off-by: Jie Zhan <zhanjie9 at hisilicon.com>
>> ---
>>  drivers/devfreq/devfreq.c                 | 17 ++++++++++++++++-
>>  drivers/devfreq/governor_passive.c        |  1 +
>>  drivers/devfreq/governor_performance.c    |  1 +
>>  drivers/devfreq/governor_powersave.c      |  1 +
>>  drivers/devfreq/governor_simpleondemand.c |  1 +
>>  drivers/devfreq/governor_userspace.c      |  1 +
>>  include/linux/devfreq-governor.h          | 11 +++++++++++
>>  7 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 9e3e6a7348f8..1cee43636ded 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;
> 
> Shouldn't the old governor be restored here?
> 
Yeah.  Perhaps we just move try_module_get() before stopping the old
governor so as to avoid another hunk of restoring governor.
>> +		return -EINVAL;
>> +	}
>> +
>>  	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;
>>  		}
[ ... ]