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@xxxxxxx> 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@xxxxxxx> > > --- > > 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 > >
