Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX

[Patrick Bellasi]

On 20-Jul 18:23, Suren Baghdasaryan wrote:
> Hi Patrick,

Hi Sure,
thank!

> On Mon, Jul 16, 2018 at 1:29 AM, Patrick Bellasi
> <patrick.bellasi@xxxxxxx> wrote:

[...]

> > @@ -977,13 +991,21 @@ static inline void uclamp_cpu_get_id(struct task_struct *p,
> >         uc_grp = &rq->uclamp.group[clamp_id][0];
> >         uc_grp[group_id].tasks += 1;
> >
> > +       /* Force clamp update on idle exit */
> > +       uc_cpu = &rq->uclamp;
> > +       clamp_value = p->uclamp[clamp_id].value;
> > +       if (unlikely(uc_cpu->flags & UCLAMP_FLAG_IDLE)) {
> 
> The condition below is not needed because UCLAMP_FLAG_IDLE is set only
> for UCLAMP_MAX clamp_id, therefore the above condition already covers
> the one below.

Not really, this function is called two times, the first time to
update UCLAMP_MIN and a second time to update UCLAMP_MAX.

For both clamp_id we want to force update uc_cpu->value[clamp_id],
thus the UCLAMP_FLAG_IDLE flag has to be cleared only the second time.

Maybe I can had the following comment to better explain the reason of
the check:

		/*
		 * This function is called for both UCLAMP_MIN (before) and
		 * UCLAMP_MAX (after). Let's reset the flag only the when
		 * we know that UCLAMP_MIN has been already updated.
		 */

> > +               if (clamp_id == UCLAMP_MAX)
> > +                       uc_cpu->flags &= ~UCLAMP_FLAG_IDLE;
> > +               uc_cpu->value[clamp_id] = clamp_value;
> > +               return;
> > +       }

[...]

-- 
#include <best/regards.h>

Patrick Bellasi