On Wed, Mar 13, 2019 at 9:16 AM Patrick Bellasi <patrick.bellasi@xxxxxxx> wrote: > > On 13-Mar 15:12, Peter Zijlstra wrote: > > On Fri, Feb 08, 2019 at 10:05:41AM +0000, Patrick Bellasi wrote: > > > +static inline void uclamp_idle_reset(struct rq *rq, unsigned int clamp_id, > > > + unsigned int clamp_value) > > > +{ > > > + /* Reset max-clamp retention only on idle exit */ > > > + if (!(rq->uclamp_flags & UCLAMP_FLAG_IDLE)) > > > + return; > > > + > > > + WRITE_ONCE(rq->uclamp[clamp_id].value, clamp_value); > > > + > > > + /* > > > + * This function is called for both UCLAMP_MIN (before) and UCLAMP_MAX > > > + * (after). The idle flag is reset only the second time, when we know > > > + * that UCLAMP_MIN has been already updated. > > > > Why do we care? That is, what is this comment trying to tell us. > > Right, the code is clear enough, I'll remove this comment. It would be probably even clearer if rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE is done from inside uclamp_rq_inc after uclamp_rq_inc_id for both clamps is called. > > > > > + */ > > > + if (clamp_id == UCLAMP_MAX) > > > + rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE; > > > +} > > -- > #include <best/regards.h> > > Patrick Bellasi