| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0jB2kn3CfX_-hdH0cVcEo5pwrMAEhvPmpmHgYkYx9GW1g@mail.gmail.com>
Date: Fri, 6 Oct 2023 22:07:41 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>,
Pierre Gondois <pierre.gondois@....com>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Daniel Bristot de Oliveira <bristot@...hat.com>,
Valentin Schneider <vschneid@...hat.com>,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v2] cpufreq: Rebuild sched-domains when removing cpufreq driver
On Fri, Oct 6, 2023 at 9:07 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> On 05-10-23, 15:41, Pierre Gondois wrote:
> > The Energy Aware Scheduler (EAS) relies on the schedutil governor.
> > When moving to/from the schedutil governor, sched domains must be
> > rebuilt to allow re-evaluating the enablement conditions of EAS.
> > This is done through sched_cpufreq_governor_change().
> >
> > Having a cpufreq governor assumes a cpufreq driver is running.
> > Inserting/removing a cpufreq driver should trigger a re-evaluation
> > of EAS enablement conditions, avoiding to see EAS enabled when
> > removing a running cpufreq driver.
> >
> > Rebuild the sched domains in schedutil's sugov_init()/sugov_exit(),
> > allowing to check EAS's enablement condition whenever schedutil
> > governor is initialized/exited from.
> > Move relevant code up in schedutil.c to avoid a split and conditional
> > function declaration.
> > Rename sched_cpufreq_governor_change() to sugov_eas_rebuild_sd().
> >
> > Signed-off-by: Pierre Gondois <pierre.gondois@....com>
> > ---
> > drivers/cpufreq/cpufreq.c | 3 +-
> > include/linux/cpufreq.h | 8 -----
> > kernel/sched/cpufreq_schedutil.c | 55 +++++++++++++++++---------------
> > 3 files changed, 30 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 60ed89000e82..4bc15634d49c 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1544,7 +1544,7 @@ static int cpufreq_online(unsigned int cpu)
> >
> > /*
> > * Register with the energy model before
> > - * sched_cpufreq_governor_change() is called, which will result
> > + * sugov_eas_rebuild_sd() is called, which will result
> > * in rebuilding of the sched domains, which should only be done
> > * once the energy model is properly initialized for the policy
> > * first.
> > @@ -2652,7 +2652,6 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> > ret = cpufreq_start_governor(policy);
> > if (!ret) {
> > pr_debug("governor change\n");
> > - sched_cpufreq_governor_change(policy, old_gov);
> > return 0;
> > }
> > cpufreq_exit_governor(policy);
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 71d186d6933a..1c5ca92a0555 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -1193,14 +1193,6 @@ static inline int of_perf_domain_get_sharing_cpumask(int pcpu, const char *list_
> > }
> > #endif
> >
> > -#if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)
> > -void sched_cpufreq_governor_change(struct cpufreq_policy *policy,
> > - struct cpufreq_governor *old_gov);
> > -#else
> > -static inline void sched_cpufreq_governor_change(struct cpufreq_policy *policy,
> > - struct cpufreq_governor *old_gov) { }
> > -#endif
> > -
> > extern unsigned int arch_freq_get_on_cpu(int cpu);
> >
> > #ifndef arch_set_freq_scale
> > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> > index 4492608b7d7f..901cada51ba7 100644
> > --- a/kernel/sched/cpufreq_schedutil.c
> > +++ b/kernel/sched/cpufreq_schedutil.c
> > @@ -555,6 +555,31 @@ static const struct kobj_type sugov_tunables_ktype = {
> >
> > /********************** cpufreq governor interface *********************/
> >
> > +#ifdef CONFIG_ENERGY_MODEL
> > +static void rebuild_sd_workfn(struct work_struct *work)
> > +{
> > + rebuild_sched_domains_energy();
> > +}
> > +
> > +static DECLARE_WORK(rebuild_sd_work, rebuild_sd_workfn);
> > +
> > +/*
> > + * EAS shouldn't be attempted without sugov, so rebuild the sched_domains
> > + * on governor changes to make sure the scheduler knows about it.
> > + */
> > +static void sugov_eas_rebuild_sd(void)
> > +{
> > + /*
> > + * When called from the cpufreq_register_driver() path, the
> > + * cpu_hotplug_lock is already held, so use a work item to
> > + * avoid nested locking in rebuild_sched_domains().
> > + */
> > + schedule_work(&rebuild_sd_work);
> > +}
> > +#else
> > +static inline void sugov_eas_rebuild_sd(void) { };
> > +#endif
> > +
> > struct cpufreq_governor schedutil_gov;
> >
> > static struct sugov_policy *sugov_policy_alloc(struct cpufreq_policy *policy)
> > @@ -709,6 +734,8 @@ static int sugov_init(struct cpufreq_policy *policy)
> > if (ret)
> > goto fail;
> >
> > + sugov_eas_rebuild_sd();
> > +
> > out:
> > mutex_unlock(&global_tunables_lock);
> > return 0;
> > @@ -750,6 +777,8 @@ static void sugov_exit(struct cpufreq_policy *policy)
> > sugov_kthread_stop(sg_policy);
> > sugov_policy_free(sg_policy);
> > cpufreq_disable_fast_switch(policy);
> > +
> > + sugov_eas_rebuild_sd();
> > }
> >
> > static int sugov_start(struct cpufreq_policy *policy)
> > @@ -838,29 +867,3 @@ struct cpufreq_governor *cpufreq_default_governor(void)
> > #endif
> >
> > cpufreq_governor_init(schedutil_gov);
> > -
> > -#ifdef CONFIG_ENERGY_MODEL
> > -static void rebuild_sd_workfn(struct work_struct *work)
> > -{
> > - rebuild_sched_domains_energy();
> > -}
> > -static DECLARE_WORK(rebuild_sd_work, rebuild_sd_workfn);
> > -
> > -/*
> > - * EAS shouldn't be attempted without sugov, so rebuild the sched_domains
> > - * on governor changes to make sure the scheduler knows about it.
> > - */
> > -void sched_cpufreq_governor_change(struct cpufreq_policy *policy,
> > - struct cpufreq_governor *old_gov)
> > -{
> > - if (old_gov == &schedutil_gov || policy->governor == &schedutil_gov) {
> > - /*
> > - * When called from the cpufreq_register_driver() path, the
> > - * cpu_hotplug_lock is already held, so use a work item to
> > - * avoid nested locking in rebuild_sched_domains().
> > - */
> > - schedule_work(&rebuild_sd_work);
> > - }
> > -
> > -}
> > -#endif
>
> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
Applied as 6.7 material, thanks!
Powered by blists - more mailing lists