linux-kernel - Re: [PATCH v2 1/2] cpufreq/schedutil: Remove fast_switch_possible flag if driver doesn't set fast

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 May 2023 11:12:46 +0530
From:   Wyes Karny <wyes.karny@....com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     ray.huang@....com, viresh.kumar@...aro.org,
        srinivas.pandruvada@...ux.intel.com, lenb@...nel.org,
        mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        joel@...lfernandes.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 1/2] cpufreq/schedutil: Remove fast_switch_possible
 flag if driver doesn't set fast_switch

Hi Rafael,

Thanks for reviewing the patch.

On 09 May 20:39, Rafael J. Wysocki wrote:
------------------------------------------>8--------------------------------------
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index 2548ec92faa2..007893514c87 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2698,8 +2698,6 @@ static int __intel_pstate_cpu_init(struct cpufreq_policy *policy)
> > >
> > >         intel_pstate_init_acpi_perf_limits(policy);
> > >
> > > -       policy->fast_switch_possible = true;
> > > -
> > >         return 0;
> > >  }
> > >
> > > @@ -2955,6 +2953,7 @@ static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
> > >         if (ret)
> > >                 return ret;
> > >
> > > +       policy->fast_switch_possible = true;
> 
> I'm not sure what this is about.  Is it a cleanup of intel_pstate?

This patch intends to remove fast_switch_possible flag dependency from 
drivers which only use adjust_perf as frequency/pref update callback. As
intel_pstate and amd_pstate driver has only adjust_perf and not
fast_switch, therefore I'm removing that flag from these drivers. But
intel_cpufreq has fast_switch therefore, only adding that flag for
intel_cpufreq driver.

Thanks & Regards,
Wyes

> 
> > >         policy->cpuinfo.transition_latency = INTEL_CPUFREQ_TRANSITION_LATENCY;
> > >         /* This reflects the intel_pstate_get_cpu_pstates() setting. */
> > >         policy->cur = policy->cpuinfo.min_freq;
> > > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > > index 26e2eb399484..7a32cfca26c9 100644
> > > --- a/include/linux/cpufreq.h
> > > +++ b/include/linux/cpufreq.h
> > > @@ -604,6 +604,7 @@ struct cpufreq_governor {
> > >  /* Pass a target to the cpufreq driver */
> > >  unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
> > >                                         unsigned int target_freq);
> > > +bool cpufreq_driver_has_fast_switch(void);
> > >  void cpufreq_driver_adjust_perf(unsigned int cpu,
> > >                                 unsigned long min_perf,
> > >                                 unsigned long target_perf,
> > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> > > index e3211455b203..f993ecf731a9 100644
> > > --- a/kernel/sched/cpufreq_schedutil.c
> > > +++ b/kernel/sched/cpufreq_schedutil.c
> > > @@ -776,7 +776,7 @@ static int sugov_start(struct cpufreq_policy *policy)
> > >
> > >         if (policy_is_shared(policy))
> > >                 uu = sugov_update_shared;
> > > -       else if (policy->fast_switch_enabled && cpufreq_driver_has_adjust_perf())
> > > +       else if (cpufreq_driver_has_adjust_perf())
> > >                 uu = sugov_update_single_perf;
> > >         else
> > >                 uu = sugov_update_single_freq;
> > > --