| Message ID | 20230529142551.6726-1-wyes.karny@amd.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v4.1] cpufreq: Fail driver register if it has adjust_perf without fast_switch | expand |
On Mon, May 29, 2023 at 4:26 PM Wyes Karny <wyes.karny@amd.com> wrote: > > If fast_switch_possible flag is set by the scaling driver, the governor > is free to select fast_switch function even if adjust_perf is set. Some > scaling drivers which use adjust_perf don't set fast_switch thinking > that the governor would never fall back to fast_switch. But the governor > can fall back to fast_switch even in runtime if frequency invariance is > disabled due to some reason. This could crash the kernel if the driver > didn't set the fast_switch function pointer. > > Therefore, fail driver registration if it has adjust_perf without > fast_switch. > > Suggested-by: Rafael J. Wysocki <rafael@kernel.org> > Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > --- > drivers/cpufreq/cpufreq.c | 3 ++- > include/linux/cpufreq.h | 5 ++++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 6b52ebe5a890..50bbc969ffe5 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2828,7 +2828,8 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) > (driver_data->setpolicy && (driver_data->target_index || > driver_data->target)) || > (!driver_data->get_intermediate != !driver_data->target_intermediate) || > - (!driver_data->online != !driver_data->offline)) > + (!driver_data->online != !driver_data->offline) || > + (driver_data->adjust_perf && !driver_data->fast_switch)) > return -EINVAL; > > pr_debug("trying to register driver %s\n", driver_data->name); > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 26e2eb399484..172ff51c1b2a 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -340,7 +340,10 @@ struct cpufreq_driver { > /* > * ->fast_switch() replacement for drivers that use an internal > * representation of performance levels and can pass hints other than > - * the target performance level to the hardware. > + * the target performance level to the hardware. This can only be set > + * if ->fast_switch is set too, because in those cases (under specific > + * conditions) scale invariance can be disabled, which causes the > + * schedutil governor to fall back to the latter. > */ > void (*adjust_perf)(unsigned int cpu, > unsigned long min_perf, > -- Applied as 6.5 material, thanks!
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 6b52ebe5a890..50bbc969ffe5 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2828,7 +2828,8 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data) (driver_data->setpolicy && (driver_data->target_index || driver_data->target)) || (!driver_data->get_intermediate != !driver_data->target_intermediate) || - (!driver_data->online != !driver_data->offline)) + (!driver_data->online != !driver_data->offline) || + (driver_data->adjust_perf && !driver_data->fast_switch)) return -EINVAL; pr_debug("trying to register driver %s\n", driver_data->name); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 26e2eb399484..172ff51c1b2a 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -340,7 +340,10 @@ struct cpufreq_driver { /* * ->fast_switch() replacement for drivers that use an internal * representation of performance levels and can pass hints other than - * the target performance level to the hardware. + * the target performance level to the hardware. This can only be set + * if ->fast_switch is set too, because in those cases (under specific + * conditions) scale invariance can be disabled, which causes the + * schedutil governor to fall back to the latter. */ void (*adjust_perf)(unsigned int cpu, unsigned long min_perf,
If fast_switch_possible flag is set by the scaling driver, the governor is free to select fast_switch function even if adjust_perf is set. Some scaling drivers which use adjust_perf don't set fast_switch thinking that the governor would never fall back to fast_switch. But the governor can fall back to fast_switch even in runtime if frequency invariance is disabled due to some reason. This could crash the kernel if the driver didn't set the fast_switch function pointer. Therefore, fail driver registration if it has adjust_perf without fast_switch. Suggested-by: Rafael J. Wysocki <rafael@kernel.org> Suggested-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Wyes Karny <wyes.karny@amd.com> --- drivers/cpufreq/cpufreq.c | 3 ++- include/linux/cpufreq.h | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-)