Hey Viresh On Thursday 25 Jun 2020 at 16:24:16 (+0530), Viresh Kumar wrote: > The locking around governors handling isn't adequate currently. The list > of governors should never be traversed without locking in place. Also we > must make sure the governor isn't removed while it is still referenced > by code.
Thanks for having a look at this! This solves the issue for the reference to policy->last_governor, but given that your patch is based on top of [email protected], 'default_governor' needs a similar treatment I think. Perhaps something along the lines of the (completely untested) snippet below? diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index dad6b85f4c89..9d7cf2ce2768 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1062,6 +1062,17 @@ __weak struct cpufreq_governor *cpufreq_default_governor(void) return NULL; } +static bool get_default_governor(void) +{ + bool ret; + + mutex_lock(&cpufreq_governor_mutex); + ret = default_governor && !try_module_get(default_governor->owner); + mutex_unlock(&cpufreq_governor_mutex); + + return ret; +} + static int cpufreq_init_policy(struct cpufreq_policy *policy) { struct cpufreq_governor *gov = NULL; @@ -1073,20 +1084,21 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy) /* Update policy governor to the one used before hotplug. */ gov = get_governor(policy->last_governor); if (gov) { - put_governor = true; pr_debug("Restoring governor %s for cpu %d\n", policy->governor->name, policy->cpu); - } else if (default_governor) { + } else if (get_default_governor()) { gov = default_governor; } else { return -ENODATA; } + put_governor = true; } else { /* Use the default policy if there is no last_policy. */ if (policy->last_policy) { pol = policy->last_policy; - } else if (default_governor) { + } else if (get_default_governor()) { pol = cpufreq_parse_policy(default_governor->name); + module_put(default_governor->owner); /* * In case the default governor is neiter "performance" * nor "powersave", fall back to the initial policy
