Re: [RFC/RFT] [PATCH 05/10] cpufreq: intel_pstate: HWP boost performance on IO Wake

Srinivas Pandruvada
2018-05-16T19:28:12Z

On Wed, 2018-05-16 at 11:45 +0200, Rafael J. Wysocki wrote:

[...]
>
> > + if (time_before64(time, cpu->last_io_update + 2 *
> > TICK_NSEC) &&
> > + intel_pstate_check_boost_threhold(cpu))
> > + cpu->iowait_boost = true;
> > +
> > + cpu->last_io_update = time;
> > + cpu->last_update = time;
>
> This is a shared data structure and it gets updated without
> synchronization, unless I'm missing something.
Good point.

>
> How much does the cross-CPU case matter?
I was under impression that IOWAIT flag is set on local CPU calls only,
but I see IOWAIT flags set for remote CPU all the time. So we will miss
if we don't take care of cross CPU calls.

But I can lump them as part of smp async call for all cross cpu updates
to avoid sync issue.

>
> > + }
> >
> > + /*
> > + * If the boost is active, we will remove it after timeout
> > on local
> > + * CPU only.
> > + */
> > + if (cpu->hwp_boost_active) {
> > + if (smp_processor_id() == cpu->cpu) {
> > + bool expired;
> > +
> > + expired = time_after64(time, cpu-
> > >last_update +
> > + (hwp_boost_hold_time
> > _ms * NSEC_PER_MSEC));
> > + if (expired) {
> > + intel_pstate_hwp_boost_down(cpu);
> > + cpu->hwp_boost_active = false;
> > + cpu->iowait_boost = false;
> > + }
> > + }
> > + return;
> > + }
> > +
> > + cpu->last_update = time;
> > +
> > + if (cpu->iowait_boost) {
> > + cpu->hwp_boost_active = true;
> > + if (smp_processor_id() == cpu->cpu)
> > + intel_pstate_hwp_boost_up(cpu);
> > + else
> > + smp_call_function_single_async(cpu->cpu,
> > &cpu->csd);
> > + }
> > }
> >
> > static inline void intel_pstate_calc_avg_perf(struct cpudata *cpu)
> > --
> > 2.9.5
> >