Re: [RFC/RFT] [PATCH v2 4/6] cpufreq: intel_pstate: HWP boost performance on IO wakeup

[Pandruvada, Srinivas]

On Tue, 2018-05-29 at 09:44 +0200, Rafael J. Wysocki wrote:
> On Thu, May 24, 2018 at 3:47 AM, Srinivas Pandruvada
> <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:

[...]

> > 
> > +ÂÂÂÂÂÂÂcpu->sample.time = time;
> > +ÂÂÂÂÂÂÂio_flag = test_and_clear_bit(SCHED_CPUFREQ_IOWAIT, &cpu-
> > >sched_flags);
> 
> I don't think you need to use bit ops here.

Agree. This is not required here for just IO boost support.

> 
> _update_util() runs under rq->lock for the target CPU, so it will not
> run concurrently on two different CPUs for the same target anyway.
> 
> > +ÂÂÂÂÂÂÂif (io_flag) {
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂbool do_io = false;
> > +
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ/*
> > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ* 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);
> 
> But what happens if user space wants to update the limits while
> boosting is in effect?ÂÂShouldn't it take hwp_boost_min into account
> then?
User request has always higher priority. User min will be taken into
account as the boost min is updated under the the update util call
back, not just one time.

Thanks,
Srinivas

Attachment: smime.p7s
Description: S/MIME cryptographic signature