Re: [PATCH v3] sched/eevdf: Force propagating min_slice of cfs_rq when {en,de}queue tasks

From: Tianchen Ding
Date: Sun Feb 16 2025 - 21:53:31 EST


Hi. Sorry for replying late due to weekend.

On 2/14/25 11:42 PM, K Prateek Nayak wrote:

[...]


Should we check if old slice matches with the new slice before
propagation to avoid any unnecessary propagate call? Something like:

        if (se->slice != slice) {
            se->slice = slice;
            if (se != cfs_rq->curr)
                min_vruntime_cb_propagate(&se->run_node, NULL);
        }

Thoughts?


This optimization makes sense to me. But the code would be a bit ugly :-/

Maybe we should wrap it in a helper. Something like:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1e78caa21436..ccceb67004a4 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -844,6 +844,16 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
RB_DECLARE_CALLBACKS(static, min_vruntime_cb, struct sched_entity,
run_node, min_vruntime, min_vruntime_update);
+static inline void propagate_slice(struct cfs_rq *cfs_rq, struct sched_entity *se, u64 slice)
+{
+ if (se->slice == slice)
+ return;
+
+ se->slice = slice;
+ if (se != cfs_rq->curr)
+ min_vruntime_cb_propagate(&se->run_node, NULL);
+}
+
/*
* Enqueue an entity into the rb-tree:
*/
@@ -6969,7 +6979,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
se_update_runnable(se);
update_cfs_group(se);
- se->slice = slice;
+ propagate_slice(cfs_rq, se, slice);
slice = cfs_rq_min_slice(cfs_rq);
cfs_rq->h_nr_runnable += h_nr_runnable;
@@ -7098,7 +7108,7 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
se_update_runnable(se);
update_cfs_group(se);
- se->slice = slice;
+ propagate_slice(cfs_rq, se, slice);
slice = cfs_rq_min_slice(cfs_rq);
cfs_rq->h_nr_runnable -= h_nr_runnable;
--

Since the patch has been accepted, I'm not sure whether I should send a
next version. The current version does introduce an extra function call
when se->slice == slice, but the loop will run only once and exit because
RBCOMPUTE() will return true. So maybe the cost is insignificant?