Messages in this thread | | | Date | Fri, 3 Nov 2023 22:01:55 +0800 | Subject | Re: [PATCH v2 1/1] sched/fair: Track current se's EEVDF parameters | From | Yiwei Lin <> |
| |
On 11/3/23 14:45, Abel Wu wrote: > On 11/2/23 10:20 PM, Yiwei Lin Wrote: >> After dequeuing the current-picked scheduling entity with >> `__dequeue_entity`, its contribution to the EEVDF parameters >> cfs_rq->avg_vruntime and cfs_rq->avg_load are also removed. >> Because these should in fact be considered for the EEVDF algorithm, >> we took curr as the special case and inserted back the contributions >> when requests for cfs_rq->avg_vruntime and cfs_rq->avg_load. >> >> Functions like `entity_eligible` which is called insied a loop may >> therefore recalculate these statistics repeatly and require more effort. >> Instead, we could just avoid to remove these statistics from >> cfs_rq->avg_vruntime and cfs_rq->avg_load directly. >> >> Signed-off-by: Yiwei Lin <s921975628@gmail.com> >> --- >> kernel/sched/fair.c | 39 +++++++++++---------------------------- >> 1 file changed, 11 insertions(+), 28 deletions(-) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 876798824..a10a73603 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -655,17 +655,9 @@ void avg_vruntime_update(struct cfs_rq *cfs_rq, >> s64 delta) >> */ >> u64 avg_vruntime(struct cfs_rq *cfs_rq) >> { >> - struct sched_entity *curr = cfs_rq->curr; >> s64 avg = cfs_rq->avg_vruntime; >> long load = cfs_rq->avg_load; >> - if (curr && curr->on_rq) { >> - unsigned long weight = scale_load_down(curr->load.weight); >> - >> - avg += entity_key(cfs_rq, curr) * weight; >> - load += weight; >> - } >> - >> if (load) { >> /* sign flips effective floor / ceil */ >> if (avg < 0) >> @@ -722,17 +714,9 @@ static void update_entity_lag(struct cfs_rq >> *cfs_rq, struct sched_entity *se) >> */ >> int entity_eligible(struct cfs_rq *cfs_rq, struct sched_entity *se) >> { >> - struct sched_entity *curr = cfs_rq->curr; >> s64 avg = cfs_rq->avg_vruntime; >> long load = cfs_rq->avg_load; >> - if (curr && curr->on_rq) { >> - unsigned long weight = scale_load_down(curr->load.weight); >> - >> - avg += entity_key(cfs_rq, curr) * weight; >> - load += weight; >> - } >> - >> return avg >= entity_key(cfs_rq, se) * load; >> } >> @@ -821,11 +805,12 @@ static void __enqueue_entity(struct cfs_rq >> *cfs_rq, struct sched_entity *se) >> __entity_less, &min_deadline_cb); >> } >> -static void __dequeue_entity(struct cfs_rq *cfs_rq, struct >> sched_entity *se) >> +static void __dequeue_entity(struct cfs_rq *cfs_rq, struct >> sched_entity *se, bool on_rq) > > Please don't add such complexity. Since avg_vruntime includes the whole > cfs_rq after this change, you can simply avg_vruntime_add() when doing > enqueue_entity() and avg_vruntime_sub() in dequeue_entity(). > Right. I should consider that we'll also use __enqueue_entity to put back the 'curr', and avg_vruntime_add() when doing enqueue_entity() and avg_vruntime_sub() in dequeue_entity() as you mentioned could not only simplify the implementation but also address to this issue. I'll fix it.
>> { >> rb_erase_augmented_cached(&se->run_node, &cfs_rq->tasks_timeline, >> &min_deadline_cb); >> - avg_vruntime_sub(cfs_rq, se); >> + if (!on_rq) >> + avg_vruntime_sub(cfs_rq, se); >> } >> struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq) >> @@ -1137,6 +1122,7 @@ static void update_curr(struct cfs_rq *cfs_rq) >> struct sched_entity *curr = cfs_rq->curr; >> u64 now = rq_clock_task(rq_of(cfs_rq)); >> u64 delta_exec; >> + u64 delta_fair; >> if (unlikely(!curr)) >> return; >> @@ -1158,7 +1144,9 @@ static void update_curr(struct cfs_rq *cfs_rq) >> curr->sum_exec_runtime += delta_exec; >> schedstat_add(cfs_rq->exec_clock, delta_exec); >> - curr->vruntime += calc_delta_fair(delta_exec, curr); >> + delta_fair = calc_delta_fair(delta_exec, curr); >> + curr->vruntime += delta_fair; >> + cfs_rq->avg_vruntime += delta_fair * >> scale_load_down(curr->load.weight); > > What if curr->load.weight changes in this time slice?
The change on reweight_entity now don't separate curr and other entities. If curr->load.weight is changed, reweight_entity will do: 1. update_curr() to account the latest contribution of vruntime with old weight. 2. avg_vruntime_sub() to remove the curr vruntime with old weight 3. avg_vruntime_add() to insert the curr vruntime with new weight Although step1 and step2 could be merged together when re-weighting the curr entity, but it seems just find to general implement reweight_entity.
| |