lkml.org 
[lkml]   [2018]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2018-07-23 18:40    [W:0.074 / U:7.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site