linux-kernel - Re: [PATCH v2.1 4/4] cpufreq: schedutil: Always call driver if need_freq

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:   Thu, 29 Oct 2020 11:42:56 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH v2.1 4/4] cpufreq: schedutil: Always call driver if
 need_freq_update is set

On Thu, Oct 29, 2020 at 12:10 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> On 27-10-20, 16:35, Rafael J. Wysocki wrote:
> > Index: linux-pm/kernel/sched/cpufreq_schedutil.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/cpufreq_schedutil.c
> > +++ linux-pm/kernel/sched/cpufreq_schedutil.c
> > @@ -102,11 +102,12 @@ static bool sugov_should_update_freq(str
> >  static bool sugov_update_next_freq(struct sugov_policy *sg_policy, u64 time,
> >                                  unsigned int next_freq)
> >  {
> > -     if (sg_policy->next_freq == next_freq)
> > +     if (sg_policy->next_freq == next_freq && !sg_policy->need_freq_update)
> >               return false;
> >
> >       sg_policy->next_freq = next_freq;
> >       sg_policy->last_freq_update_time = time;
> > +     sg_policy->need_freq_update = false;
> >
> >       return true;
> >  }
> > @@ -161,10 +162,12 @@ static unsigned int get_next_freq(struct
> >
> >       freq = map_util_freq(util, freq, max);
> >
> > -     if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
> > +     if (cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS))
> > +             sg_policy->need_freq_update = true;
> > +     else if (freq == sg_policy->cached_raw_freq &&
> > +              !sg_policy->need_freq_update)
> >               return sg_policy->next_freq;
> >
> > -     sg_policy->need_freq_update = false;
> >       sg_policy->cached_raw_freq = freq;
> >       return cpufreq_driver_resolve_freq(policy, freq);
> >  }
>
> What about just this instead ?
>
>   static bool sugov_update_next_freq(struct sugov_policy *sg_policy, u64 time,
>                                    unsigned int next_freq)
>   {
>  -      if (sg_policy->next_freq == next_freq)
>  +      if (sg_policy->next_freq == next_freq &&
>  +          !cpufreq_driver_test_flags(CPUFREQ_NEED_UPDATE_LIMITS))
>                 return false;
>
>         sg_policy->next_freq = next_freq;
>         sg_policy->last_freq_update_time = time;
>
>         return true;
>   }
>

Without any changes in get_next_freq() this is not sufficient, because
get_next_freq() may skip the update too.

If the intention is to always let the driver callback run when
CPUFREQ_NEED_UPDATE_LIMITS is set, then both get_next_freq() and
sugov_update_next_freq() need to be modified.