Messages in this thread | | | From | Suren Baghdasaryan <> | Date | Mon, 23 Jul 2018 09:40:08 -0700 | Subject | Re: [PATCH v2 07/12] sched/core: uclamp: enforce last task UCLAMP_MAX |
| |
On Mon, Jul 23, 2018 at 8:02 AM, Patrick Bellasi <patrick.bellasi@arm.com> wrote: > 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@arm.com> 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. > */ >
Ah, my bad. I missed the fact that uc_cpu->flags is shared for both UCLAMP_MIN and UCLAMP_MAX. It's fine the way it originally was. Thanks for explanation!
>> > + 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
| |