lkml.org 
[lkml]   [2022]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [External] Re: [PATCH v3 2/2] sched/fair: optimize and simplify rq leaf_cfs_rq_list
From
Hi,

On 2022/5/30 15:52, Vincent Guittot wrote:
> On Thu, 26 May 2022 at 12:40, Chengming Zhou
> <zhouchengming@bytedance.com> wrote:
>>
[...]
>> @@ -11257,9 +11218,10 @@ static inline bool vruntime_normalized(struct task_struct *p)
>> */
>> static void propagate_entity_cfs_rq(struct sched_entity *se)
>> {
>> - struct cfs_rq *cfs_rq;
>> + struct cfs_rq *cfs_rq = cfs_rq_of(se);
>>
>> - list_add_leaf_cfs_rq(cfs_rq_of(se));
>> + if (!throttled_hierarchy(cfs_rq))
>> + list_add_leaf_cfs_rq(cfs_rq);
>>
>> /* Start to propagate at parent */
>> se = se->parent;
>> @@ -11268,7 +11230,8 @@ static void propagate_entity_cfs_rq(struct sched_entity *se)
>> cfs_rq = cfs_rq_of(se);
>>
>
> you can break if the cfs is throttled because it's sched_entity has
> been dequeued. In this case we check if the cfs is throttled not if
> the hierarchy is throttled
>
> + if (cfs_rq_throttled(cfs_rq))
> + break;
>

This propagate part still make me confused. :-)

I wonder if you think we should change like this:

static void propagate_entity_cfs_rq(struct sched_entity *se)
{
struct cfs_rq *cfs_rq = cfs_rq_of(se);

if (cfs_rq_throttled(cfs_rq)) --> break if cfs is throttled
return;

if (!throttled_hierarchy(cfs_rq))
list_add_leaf_cfs_rq(cfs_rq);

/* Start to propagate at parent */
se = se->parent;

for_each_sched_entity(se) {
cfs_rq = cfs_rq_of(se);

if (cfs_rq_throttled(cfs_rq)) --> break if cfs is throttled
break;

update_load_avg(cfs_rq, se, UPDATE_TG); --> throttled cfs_rq->prop not updated
if (!throttled_hierarchy(cfs_rq))
list_add_leaf_cfs_rq(cfs_rq);
}
}



If I understand right, we should update_load_avg() until cfs_rq_throttled(),
including that throttled cfs_rq? So we can go on propagating when unthrottle.

Maybe like this?

static void propagate_entity_cfs_rq(struct sched_entity *se)
{
struct cfs_rq *cfs_rq = cfs_rq_of(se);

if (cfs_rq_throttled(cfs_rq)) --> break if cfs is throttled
return;

if (!throttled_hierarchy(cfs_rq))
list_add_leaf_cfs_rq(cfs_rq);

/* Start to propagate at parent */
se = se->parent;

for_each_sched_entity(se) {
cfs_rq = cfs_rq_of(se);

update_load_avg(cfs_rq, se, UPDATE_TG); --> update throttled cfs_rq->prop

if (cfs_rq_throttled(cfs_rq)) --> break if cfs is throttled
break;

if (!throttled_hierarchy(cfs_rq))
list_add_leaf_cfs_rq(cfs_rq);
}
}


Thanks!


>> update_load_avg(cfs_rq, se, UPDATE_TG);
>> - list_add_leaf_cfs_rq(cfs_rq);
>> + if (!throttled_hierarchy(cfs_rq))
>> + list_add_leaf_cfs_rq(cfs_rq);
>> }
>> }
>> #else
>> --
>> 2.36.1
>>

\
 
 \ /
  Last update: 2022-05-31 15:56    [W:0.075 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site