Messages in this thread | | | Date | Tue, 31 May 2022 21:55:23 +0800 | Subject | Re: [External] Re: [PATCH v3 2/2] sched/fair: optimize and simplify rq leaf_cfs_rq_list | From | Chengming Zhou <> |
| |
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 >>
| |