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] [day] [month] [year] [list]
Message-ID: <0a0186cad5a9254027d0ac6a7f39e39f5473665c.camel@linux.intel.com>
Date: Mon, 30 Sep 2024 13:35:25 -0700
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Christian Loehle
	 <christian.loehle@....com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
 peterz@...radead.org, juri.lelli@...hat.com, mingo@...hat.com, 
 dietmar.eggemann@....com, vschneid@...hat.com, vincent.guittot@...aro.org, 
 Johannes.Thumshirn@....com, adrian.hunter@...el.com,
 ulf.hansson@...aro.org,  bvanassche@....org, andres@...razel.de,
 asml.silence@...il.com,  linux-block@...r.kernel.org,
 io-uring@...r.kernel.org, qyousef@...alina.io,  dsmythies@...us.net,
 axboe@...nel.dk
Subject: Re: [RFC PATCH 6/8] cpufreq: intel_pstate: Remove iowait boost

On Mon, 2024-09-30 at 20:03 +0200, Rafael J. Wysocki wrote:
> +Srinivas who can say more about the reasons why iowait boosting
> makes
> a difference for intel_pstate than I do.
> 
It makes difference on Xeons and also GFX performance.
The actual gains will be model specific as it will be dependent on
hardware algorithms and EPP.

It was introduced to solve regression in Skylake xeons. But even in the
recent servers there are gains.
Refer to
https://lkml.iu.edu/hypermail/linux/kernel/1806.0/03574.html

Thanks,
Srinivas


> On Thu, Sep 5, 2024 at 11:27 AM Christian Loehle
> <christian.loehle@....com> wrote:
> > 
> > Analogous to schedutil, remove iowait boost for the same reasons.
> 
> Well, first of all, iowait boosting was added to intel_pstate to help
> some workloads that otherwise were underperforming.  I'm not sure if
> you can simply remove it without introducing performance regressions
> in those workloads.
> 
> While you can argue that it is not useful in schedutil any more due
> to
> the improved scheduler input for it, you can hardly extend that
> argument to intel_pstate because it doesn't use all of the scheduler
> input used by schedutil.
> 
> Also, the EAS and UCLAMP_MAX arguments are not applicable to
> intel_pstate because it doesn't support any of them.
> 
> This applies to the ondemand cpufreq governor either.
> 
> 
> > Signed-off-by: Christian Loehle <christian.loehle@....com>
> > ---
> >  drivers/cpufreq/intel_pstate.c | 50 ++----------------------------
> > ----
> >  1 file changed, 3 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index c0278d023cfc..7f30b2569bb3 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -191,7 +191,6 @@ struct global_params {
> >   * @policy:            CPUFreq policy value
> >   * @update_util:       CPUFreq utility callback information
> >   * @update_util_set:   CPUFreq utility callback is set
> > - * @iowait_boost:      iowait-related boost fraction
> >   * @last_update:       Time of the last update.
> >   * @pstate:            Stores P state limits for this CPU
> >   * @vid:               Stores VID limits for this CPU
> > @@ -245,7 +244,6 @@ struct cpudata {
> >         struct acpi_processor_performance acpi_perf_data;
> >         bool valid_pss_table;
> >  #endif
> > -       unsigned int iowait_boost;
> >         s16 epp_powersave;
> >         s16 epp_policy;
> >         s16 epp_default;
> > @@ -2136,28 +2134,7 @@ static inline void
> > intel_pstate_update_util_hwp_local(struct cpudata *cpu,
> >  {
> >         cpu->sample.time = time;
> > 
> > -       if (cpu->sched_flags & SCHED_CPUFREQ_IOWAIT) {
> > -               bool do_io = false;
> > -
> > -               cpu->sched_flags = 0;
> > -               /*
> > -                * Set iowait_boost flag and update time. Since IO
> > WAIT flag
> > -                * is set all the time, we can't just conclude that
> > there is
> > -                * some IO bound activity is scheduled on this CPU
> > with just
> > -                * one occurrence. If we receive at least two in
> > two
> > -                * consecutive ticks, then we treat as boost
> > candidate.
> > -                */
> > -               if (time_before64(time, cpu->last_io_update + 2 *
> > TICK_NSEC))
> > -                       do_io = true;
> > -
> > -               cpu->last_io_update = time;
> > -
> > -               if (do_io)
> > -                       intel_pstate_hwp_boost_up(cpu);
> > -
> > -       } else {
> > -               intel_pstate_hwp_boost_down(cpu);
> > -       }
> > +       intel_pstate_hwp_boost_down(cpu);
> >  }
> > 
> >  static inline void intel_pstate_update_util_hwp(struct
> > update_util_data *data,
> > @@ -2240,9 +2217,6 @@ static inline int32_t
> > get_target_pstate(struct cpudata *cpu)
> >         busy_frac = div_fp(sample->mperf << cpu->aperf_mperf_shift,
> >                            sample->tsc);
> > 
> > -       if (busy_frac < cpu->iowait_boost)
> > -               busy_frac = cpu->iowait_boost;
> > -
> >         sample->busy_scaled = busy_frac * 100;
> > 
> >         target = READ_ONCE(global.no_turbo) ?
> > @@ -2303,7 +2277,7 @@ static void intel_pstate_adjust_pstate(struct
> > cpudata *cpu)
> >                 sample->aperf,
> >                 sample->tsc,
> >                 get_avg_frequency(cpu),
> > -               fp_toint(cpu->iowait_boost * 100));
> > +               0);
> >  }
> > 
> >  static void intel_pstate_update_util(struct update_util_data
> > *data, u64 time,
> > @@ -2317,24 +2291,6 @@ static void intel_pstate_update_util(struct
> > update_util_data *data, u64 time,
> >                 return;
> > 
> >         delta_ns = time - cpu->last_update;
> > -       if (flags & SCHED_CPUFREQ_IOWAIT) {
> > -               /* Start over if the CPU may have been idle. */
> > -               if (delta_ns > TICK_NSEC) {
> > -                       cpu->iowait_boost = ONE_EIGHTH_FP;
> > -               } else if (cpu->iowait_boost >= ONE_EIGHTH_FP) {
> > -                       cpu->iowait_boost <<= 1;
> > -                       if (cpu->iowait_boost > int_tofp(1))
> > -                               cpu->iowait_boost = int_tofp(1);
> > -               } else {
> > -                       cpu->iowait_boost = ONE_EIGHTH_FP;
> > -               }
> > -       } else if (cpu->iowait_boost) {
> > -               /* Clear iowait_boost if the CPU may have been
> > idle. */
> > -               if (delta_ns > TICK_NSEC)
> > -                       cpu->iowait_boost = 0;
> > -               else
> > -                       cpu->iowait_boost >>= 1;
> > -       }
> >         cpu->last_update = time;
> >         delta_ns = time - cpu->sample.time;
> >         if ((s64)delta_ns < INTEL_PSTATE_SAMPLING_INTERVAL)
> > @@ -2832,7 +2788,7 @@ static void intel_cpufreq_trace(struct
> > cpudata *cpu, unsigned int trace_type, in
> >                 sample->aperf,
> >                 sample->tsc,
> >                 get_avg_frequency(cpu),
> > -               fp_toint(cpu->iowait_boost * 100));
> > +               0);
> >  }
> > 
> >  static void intel_cpufreq_hwp_update(struct cpudata *cpu, u32 min,
> > u32 max,
> > --
> > 2.34.1
> > 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ