[PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Vincent Guittot]

This follows the attempt to better track maximum lag of task in presence
of different slices duration:
[1]  https://lore.kernel.org/all/20250418151225.3006867-1-vincent.guittot@linaro.org/

Patch 1 is a simple cleanup to ease following changes.

Patch 2 uses Peter's proposal made in [1] to track the max slice of
enqueued tasks and use it to clamp the lag of dequeued task.

Patch 3 modify the protection of the slice of the current task to take
into account case when tasks with shorter slice wait to run on the cpu

Patch 4 extend to slice protection mecanism to the NO_RUN_TO_PARITY case
to ensure that a resched will be set regularly. Currently, the resched 
will be set only when curr will elapse its slice which is partly similar
to RUN_TO_PARITY. Now the running task has a minimum time quantum (0.7ms)
before eevdf looks for another task to run with the exception of
PREEMPT_SHORT case which remains valid.

Peter Zijlstra (1):
  sched/fair: Increase max lag clamping

Vincent Guittot (3):
  sched/fair: Use protect_slice() instead of direct comparison
  sched/fair: Limit run to parity to the min slice of enqueued entities
  sched/fair: Improve NO_RUN_TO_PARITY

 include/linux/sched.h |  1 +
 kernel/sched/fair.c   | 76 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 61 insertions(+), 16 deletions(-)

-- 
2.43.0

[PATCH 1/4] sched/fair: Use protect_slice() instead of direct comparison

[Vincent Guittot]

Replace the test by the relevant protect_slice() function.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 83157de5b808..44a09de38ddf 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1178,7 +1178,7 @@ static inline bool did_preempt_short(struct cfs_rq *cfs_rq, struct sched_entity
 	if (!sched_feat(PREEMPT_SHORT))
 		return false;
 
-	if (curr->vlag == curr->deadline)
+	if (protect_slice(curr))
 		return false;
 
 	return !entity_eligible(cfs_rq, curr);
-- 
2.43.0

Re: [PATCH 1/4] sched/fair: Use protect_slice() instead of direct comparison

[dhaval]

On Friday, June 13th, 2025 at 7:10 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:

> 
> 
> Replace the test by the relevant protect_slice() function.
> 
> Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> 

Reviewed-by: Dhaval Giani (AMD) <dhaval@gianis.ca>

> ---
> kernel/sched/fair.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 83157de5b808..44a09de38ddf 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1178,7 +1178,7 @@ static inline bool did_preempt_short(struct cfs_rq *cfs_rq, struct sched_entity
> if (!sched_feat(PREEMPT_SHORT))
> return false;
> 
> - if (curr->vlag == curr->deadline)
> 
> + if (protect_slice(curr))
> return false;
> 
> return !entity_eligible(cfs_rq, curr);
> --
> 2.43.0

[PATCH 2/4] sched/fair: Increase max lag clamping

[Vincent Guittot]

From: Peter Zijlstra <peterz@infradead.org>

sched_entity's lag is currently limited to the maximum between the tick
and twice its slice. This is too short compared to the maximum custom
slice that can be set and accumulated by other tasks.
A task can accumulate up to its slice of negative lag while running to
parity and the other runnable tasks can accumulate the same positive lag
while waiting to run. This positive lag could be lost during dequeue when
clamping it to twice task's slice if a task's slice is 100ms and others
use a smaller value like the default 2.8ms.
Clamp the lag of a task to the maximum slice of enqueued entities plus
a tick as the update can be delayed to the next tick.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[ Rebased and Fix max slice computation ]

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 include/linux/sched.h |  1 +
 kernel/sched/fair.c   | 41 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 4f78a64beb52..89855ab45c43 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -576,6 +576,7 @@ struct sched_entity {
 	u64				deadline;
 	u64				min_vruntime;
 	u64				min_slice;
+	u64				max_slice;
 
 	struct list_head		group_node;
 	unsigned char			on_rq;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 44a09de38ddf..479b38dc307a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -676,6 +676,8 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
 	return cfs_rq->min_vruntime + avg;
 }
 
+static inline u64 cfs_rq_max_slice(struct cfs_rq *cfs_rq);
+
 /*
  * lag_i = S - s_i = w_i * (V - v_i)
  *
@@ -689,17 +691,16 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
  * EEVDF gives the following limit for a steady state system:
  *
  *   -r_max < lag < max(r_max, q)
- *
- * XXX could add max_slice to the augmented data to track this.
  */
 static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
+	u64 max_slice = cfs_rq_max_slice(cfs_rq) + TICK_NSEC;
 	s64 vlag, limit;
 
 	WARN_ON_ONCE(!se->on_rq);
 
 	vlag = avg_vruntime(cfs_rq) - se->vruntime;
-	limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
+	limit = calc_delta_fair(max_slice, se);
 
 	se->vlag = clamp(vlag, -limit, limit);
 }
@@ -795,6 +796,21 @@ static inline u64 cfs_rq_min_slice(struct cfs_rq *cfs_rq)
 	return min_slice;
 }
 
+static inline u64 cfs_rq_max_slice(struct cfs_rq *cfs_rq)
+{
+	struct sched_entity *root = __pick_root_entity(cfs_rq);
+	struct sched_entity *curr = cfs_rq->curr;
+	u64 max_slice = 0ULL;
+
+	if (curr && curr->on_rq)
+		max_slice = curr->slice;
+
+	if (root)
+		max_slice = max(max_slice, root->max_slice);
+
+	return max_slice;
+}
+
 static inline bool __entity_less(struct rb_node *a, const struct rb_node *b)
 {
 	return entity_before(__node_2_se(a), __node_2_se(b));
@@ -820,6 +836,16 @@ static inline void __min_slice_update(struct sched_entity *se, struct rb_node *n
 	}
 }
 
+static inline void __max_slice_update(struct sched_entity *se, struct rb_node *node)
+{
+	if (node) {
+		struct sched_entity *rse = __node_2_se(node);
+
+		if (rse->max_slice > se->max_slice)
+			se->max_slice = rse->max_slice;
+	}
+}
+
 /*
  * se->min_vruntime = min(se->vruntime, {left,right}->min_vruntime)
  */
@@ -827,6 +853,7 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
 {
 	u64 old_min_vruntime = se->min_vruntime;
 	u64 old_min_slice = se->min_slice;
+	u64 old_max_slice = se->max_slice;
 	struct rb_node *node = &se->run_node;
 
 	se->min_vruntime = se->vruntime;
@@ -837,8 +864,13 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
 	__min_slice_update(se, node->rb_right);
 	__min_slice_update(se, node->rb_left);
 
+	se->max_slice = se->slice;
+	__max_slice_update(se, node->rb_right);
+	__max_slice_update(se, node->rb_left);
+
 	return se->min_vruntime == old_min_vruntime &&
-	       se->min_slice == old_min_slice;
+	       se->min_slice == old_min_slice &&
+	       se->max_slice == old_max_slice;
 }
 
 RB_DECLARE_CALLBACKS(static, min_vruntime_cb, struct sched_entity,
@@ -852,6 +884,7 @@ static void __enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	avg_vruntime_add(cfs_rq, se);
 	se->min_vruntime = se->vruntime;
 	se->min_slice = se->slice;
+	se->max_slice = se->slice;
 	rb_add_augmented_cached(&se->run_node, &cfs_rq->tasks_timeline,
 				__entity_less, &min_vruntime_cb);
 }
-- 
2.43.0

Re: [PATCH 2/4] sched/fair: Increase max lag clamping

[dhaval]

On Friday, June 13th, 2025 at 7:14 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:

> 
> 
> From: Peter Zijlstra peterz@infradead.org
> 
> 
> sched_entity's lag is currently limited to the maximum between the tick
> and twice its slice. This is too short compared to the maximum custom
> slice that can be set and accumulated by other tasks.
> A task can accumulate up to its slice of negative lag while running to
> parity and the other runnable tasks can accumulate the same positive lag
> while waiting to run. This positive lag could be lost during dequeue when
> clamping it to twice task's slice if a task's slice is 100ms and others
> use a smaller value like the default 2.8ms.
> Clamp the lag of a task to the maximum slice of enqueued entities plus
> a tick as the update can be delayed to the next tick.
> 
> Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org
> 
> 
> [ Rebased and Fix max slice computation ]
> 
> Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> 
> ---
> include/linux/sched.h | 1 +
> kernel/sched/fair.c | 41 +++++++++++++++++++++++++++++++++++++----
> 2 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 4f78a64beb52..89855ab45c43 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -576,6 +576,7 @@ struct sched_entity {
> u64 deadline;
> u64 min_vruntime;
> u64 min_slice;
> + u64 max_slice;
> 

I am just wondering if it makes sense to maybe add a few comments here on what each of these fields are for. Maybe not this series, but if you are open to it, I will spin one up next week.

> struct list_head group_node;
> unsigned char on_rq;
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 44a09de38ddf..479b38dc307a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -676,6 +676,8 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
> return cfs_rq->min_vruntime + avg;
> 
> }
> 
> +static inline u64 cfs_rq_max_slice(struct cfs_rq cfs_rq);
> +
> /
> * lag_i = S - s_i = w_i * (V - v_i)
> *
> @@ -689,17 +691,16 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
> * EEVDF gives the following limit for a steady state system:
> *
> * -r_max < lag < max(r_max, q)
> - *
> - * XXX could add max_slice to the augmented data to track this.
> */
> static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> {
> + u64 max_slice = cfs_rq_max_slice(cfs_rq) + TICK_NSEC;
> s64 vlag, limit;
> 
> WARN_ON_ONCE(!se->on_rq);
> 
> 
> vlag = avg_vruntime(cfs_rq) - se->vruntime;
> 
> - limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> 
> + limit = calc_delta_fair(max_slice, se);
> 
> se->vlag = clamp(vlag, -limit, limit);
> 

As an aside, I have a test for Theorem 1 from the paper which shows we are clamping here even in conditions I do not expect. Almost always a cgroup seems to be involved. I was out dealing with sickness last couple of weeks, so I have not debugged further.

> }
> @@ -795,6 +796,21 @@ static inline u64 cfs_rq_min_slice(struct cfs_rq *cfs_rq)
> return min_slice;
> }
> 
> +static inline u64 cfs_rq_max_slice(struct cfs_rq *cfs_rq)
> +{
> + struct sched_entity *root = __pick_root_entity(cfs_rq);
> + struct sched_entity *curr = cfs_rq->curr;
> 
> + u64 max_slice = 0ULL;
> +
> + if (curr && curr->on_rq)
> 
> + max_slice = curr->slice;
> 
> +
> + if (root)
> + max_slice = max(max_slice, root->max_slice);
> 
> +
> + return max_slice;
> +}
> +
> static inline bool __entity_less(struct rb_node *a, const struct rb_node *b)
> {
> return entity_before(__node_2_se(a), __node_2_se(b));
> @@ -820,6 +836,16 @@ static inline void __min_slice_update(struct sched_entity *se, struct rb_node *n
> }
> }
> 
> +static inline void __max_slice_update(struct sched_entity *se, struct rb_node *node)
> +{
> + if (node) {
> + struct sched_entity *rse = __node_2_se(node);
> +
> + if (rse->max_slice > se->max_slice)
> 
> + se->max_slice = rse->max_slice;
> 
> + }
> +}
> +
> /*
> * se->min_vruntime = min(se->vruntime, {left,right}->min_vruntime)
> 
> */
> @@ -827,6 +853,7 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
> {
> u64 old_min_vruntime = se->min_vruntime;
> 
> u64 old_min_slice = se->min_slice;
> 
> + u64 old_max_slice = se->max_slice;
> 
> struct rb_node *node = &se->run_node;
> 
> 
> se->min_vruntime = se->vruntime;
> 
> @@ -837,8 +864,13 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
> __min_slice_update(se, node->rb_right);
> 
> __min_slice_update(se, node->rb_left);
> 
> 
> + se->max_slice = se->slice;
> 
> + __max_slice_update(se, node->rb_right);
> 
> + __max_slice_update(se, node->rb_left);
> 
> +
> return se->min_vruntime == old_min_vruntime &&
> 
> - se->min_slice == old_min_slice;
> 
> + se->min_slice == old_min_slice &&
> 
> + se->max_slice == old_max_slice;
> 
> }
> 
> RB_DECLARE_CALLBACKS(static, min_vruntime_cb, struct sched_entity,
> @@ -852,6 +884,7 @@ static void __enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
> avg_vruntime_add(cfs_rq, se);
> se->min_vruntime = se->vruntime;
> 
> se->min_slice = se->slice;
> 
> + se->max_slice = se->slice;
> 
> rb_add_augmented_cached(&se->run_node, &cfs_rq->tasks_timeline,
> 
> __entity_less, &min_vruntime_cb);
> }

otherwise

Reviewed-by: Dhaval Giani (AMD) <dhaval@gianis.ca>

> --
> 2.43.0

Re: [PATCH 2/4] sched/fair: Increase max lag clamping

[Vincent Guittot]

On Fri, 13 Jun 2025 at 23:00, <dhaval@gianis.ca> wrote:
>
>
>
>
>
>
> On Friday, June 13th, 2025 at 7:14 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:
>
> >
> >
> > From: Peter Zijlstra peterz@infradead.org
> >
> >
> > sched_entity's lag is currently limited to the maximum between the tick
> > and twice its slice. This is too short compared to the maximum custom
> > slice that can be set and accumulated by other tasks.
> > A task can accumulate up to its slice of negative lag while running to
> > parity and the other runnable tasks can accumulate the same positive lag
> > while waiting to run. This positive lag could be lost during dequeue when
> > clamping it to twice task's slice if a task's slice is 100ms and others
> > use a smaller value like the default 2.8ms.
> > Clamp the lag of a task to the maximum slice of enqueued entities plus
> > a tick as the update can be delayed to the next tick.
> >
> > Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org
> >
> >
> > [ Rebased and Fix max slice computation ]
> >
> > Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> >
> > ---
> > include/linux/sched.h | 1 +
> > kernel/sched/fair.c | 41 +++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 4f78a64beb52..89855ab45c43 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -576,6 +576,7 @@ struct sched_entity {
> > u64 deadline;
> > u64 min_vruntime;
> > u64 min_slice;
> > + u64 max_slice;
> >
>
> I am just wondering if it makes sense to maybe add a few comments here on what each of these fields are for. Maybe not this series, but if you are open to it, I will spin one up next week.

Yes, make sense


>
> > struct list_head group_node;
> > unsigned char on_rq;
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 44a09de38ddf..479b38dc307a 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -676,6 +676,8 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
> > return cfs_rq->min_vruntime + avg;
> >
> > }
> >
> > +static inline u64 cfs_rq_max_slice(struct cfs_rq cfs_rq);
> > +
> > /
> > * lag_i = S - s_i = w_i * (V - v_i)
> > *
> > @@ -689,17 +691,16 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
> > * EEVDF gives the following limit for a steady state system:
> > *
> > * -r_max < lag < max(r_max, q)
> > - *
> > - * XXX could add max_slice to the augmented data to track this.
> > */
> > static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> > {
> > + u64 max_slice = cfs_rq_max_slice(cfs_rq) + TICK_NSEC;
> > s64 vlag, limit;
> >
> > WARN_ON_ONCE(!se->on_rq);
> >
> >
> > vlag = avg_vruntime(cfs_rq) - se->vruntime;
> >
> > - limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> >
> > + limit = calc_delta_fair(max_slice, se);
> >
> > se->vlag = clamp(vlag, -limit, limit);
> >
>
> As an aside, I have a test for Theorem 1 from the paper which shows we are clamping here even in conditions I do not expect. Almost always a cgroup seems to be involved. I was out dealing with sickness last couple of weeks, so I have not debugged further.
>
> > }
> > @@ -795,6 +796,21 @@ static inline u64 cfs_rq_min_slice(struct cfs_rq *cfs_rq)
> > return min_slice;
> > }
> >
> > +static inline u64 cfs_rq_max_slice(struct cfs_rq *cfs_rq)
> > +{
> > + struct sched_entity *root = __pick_root_entity(cfs_rq);
> > + struct sched_entity *curr = cfs_rq->curr;
> >
> > + u64 max_slice = 0ULL;
> > +
> > + if (curr && curr->on_rq)
> >
> > + max_slice = curr->slice;
> >
> > +
> > + if (root)
> > + max_slice = max(max_slice, root->max_slice);
> >
> > +
> > + return max_slice;
> > +}
> > +
> > static inline bool __entity_less(struct rb_node *a, const struct rb_node *b)
> > {
> > return entity_before(__node_2_se(a), __node_2_se(b));
> > @@ -820,6 +836,16 @@ static inline void __min_slice_update(struct sched_entity *se, struct rb_node *n
> > }
> > }
> >
> > +static inline void __max_slice_update(struct sched_entity *se, struct rb_node *node)
> > +{
> > + if (node) {
> > + struct sched_entity *rse = __node_2_se(node);
> > +
> > + if (rse->max_slice > se->max_slice)
> >
> > + se->max_slice = rse->max_slice;
> >
> > + }
> > +}
> > +
> > /*
> > * se->min_vruntime = min(se->vruntime, {left,right}->min_vruntime)
> >
> > */
> > @@ -827,6 +853,7 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
> > {
> > u64 old_min_vruntime = se->min_vruntime;
> >
> > u64 old_min_slice = se->min_slice;
> >
> > + u64 old_max_slice = se->max_slice;
> >
> > struct rb_node *node = &se->run_node;
> >
> >
> > se->min_vruntime = se->vruntime;
> >
> > @@ -837,8 +864,13 @@ static inline bool min_vruntime_update(struct sched_entity *se, bool exit)
> > __min_slice_update(se, node->rb_right);
> >
> > __min_slice_update(se, node->rb_left);
> >
> >
> > + se->max_slice = se->slice;
> >
> > + __max_slice_update(se, node->rb_right);
> >
> > + __max_slice_update(se, node->rb_left);
> >
> > +
> > return se->min_vruntime == old_min_vruntime &&
> >
> > - se->min_slice == old_min_slice;
> >
> > + se->min_slice == old_min_slice &&
> >
> > + se->max_slice == old_max_slice;
> >
> > }
> >
> > RB_DECLARE_CALLBACKS(static, min_vruntime_cb, struct sched_entity,
> > @@ -852,6 +884,7 @@ static void __enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
> > avg_vruntime_add(cfs_rq, se);
> > se->min_vruntime = se->vruntime;
> >
> > se->min_slice = se->slice;
> >
> > + se->max_slice = se->slice;
> >
> > rb_add_augmented_cached(&se->run_node, &cfs_rq->tasks_timeline,
> >
> > __entity_less, &min_vruntime_cb);
> > }
>
> otherwise
>
> Reviewed-by: Dhaval Giani (AMD) <dhaval@gianis.ca>
>
> > --
> > 2.43.0
>
>

[PATCH 3/4] sched/fair: Limit run to parity to the min slice of enqueued entities

[Vincent Guittot]

Run to parity ensures that current will get a chance to run its full
slice in one go but this can create large latency for entity with shorter
slice that has alreasy exausted its slice and wait to run the next one.

Clamp the run to parity duration to the shortest slice of all enqueued
entities.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 kernel/sched/fair.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 479b38dc307a..d8345219dfd4 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -917,23 +917,32 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
 }
 
 /*
- * HACK, stash a copy of deadline at the point of pick in vlag,
- * which isn't used until dequeue.
+ * HACK, Set the vruntime, up to which the entity can run before picking
+ * another one, in vlag, which isn't used until dequeue.
+ * In case of run to parity, we use the shortest slice of the enqueued
+ * entities.
  */
 static inline void set_protect_slice(struct sched_entity *se)
 {
-	se->vlag = se->deadline;
+	u64 min_slice;
+
+	min_slice = cfs_rq_min_slice(cfs_rq_of(se));
+
+	if (min_slice != se->slice)
+		se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
+	else
+		se->vlag = se->deadline;
 }
 
 static inline bool protect_slice(struct sched_entity *se)
 {
-	return se->vlag == se->deadline;
+	return ((s64)(se->vlag - se->vruntime) > 0);
 }
 
 static inline void cancel_protect_slice(struct sched_entity *se)
 {
 	if (protect_slice(se))
-		se->vlag = se->deadline + 1;
+		se->vlag = se->vruntime;
 }
 
 /*
-- 
2.43.0

Re: [PATCH 3/4] sched/fair: Limit run to parity to the min slice of enqueued entities

[dhaval]

On Friday, June 13th, 2025 at 7:16 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:

> 
> 
> Run to parity ensures that current will get a chance to run its full
> slice in one go but this can create large latency for entity with shorter
> slice that has alreasy exausted its slice and wait to run the next one.

"already exhausted"

> 
> Clamp the run to parity duration to the shortest slice of all enqueued
> entities.
> 
> Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> 
> ---
> kernel/sched/fair.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 479b38dc307a..d8345219dfd4 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -917,23 +917,32 @@ struct sched_entity *__pick_first_entity(struct cfs_rq cfs_rq)
> }
> 
> /
> - * HACK, stash a copy of deadline at the point of pick in vlag,
> - * which isn't used until dequeue.
> + * HACK, Set the vruntime, up to which the entity can run before picking
> + * another one, in vlag, which isn't used until dequeue.
> + * In case of run to parity, we use the shortest slice of the enqueued
> + * entities.
> */

I am going to admit - I don't have a good intuitive sense on how this will affect the functionality. Maybe you can help me think of a test case to explicitly write out this assumption in behavior?

Dhaval

> static inline void set_protect_slice(struct sched_entity *se)
> {
> - se->vlag = se->deadline;
> 
> + u64 min_slice;
> +
> + min_slice = cfs_rq_min_slice(cfs_rq_of(se));
> +
> + if (min_slice != se->slice)
> 
> + se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
> 
> + else
> + se->vlag = se->deadline;
> 
> }
> 
> static inline bool protect_slice(struct sched_entity *se)
> {
> - return se->vlag == se->deadline;
> 
> + return ((s64)(se->vlag - se->vruntime) > 0);
> 
> }
> 
> static inline void cancel_protect_slice(struct sched_entity *se)
> {
> if (protect_slice(se))
> - se->vlag = se->deadline + 1;
> 
> + se->vlag = se->vruntime;
> 
> }
> 
> /*
> --
> 2.43.0

Re: [PATCH 3/4] sched/fair: Limit run to parity to the min slice of enqueued entities

[Vincent Guittot]

On Sat, 14 Jun 2025 at 00:53, <dhaval@gianis.ca> wrote:
>
>
>
>
>
>
> On Friday, June 13th, 2025 at 7:16 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:
>
> >
> >
> > Run to parity ensures that current will get a chance to run its full
> > slice in one go but this can create large latency for entity with shorter
> > slice that has alreasy exausted its slice and wait to run the next one.
>
> "already exhausted"
>
> >
> > Clamp the run to parity duration to the shortest slice of all enqueued
> > entities.
> >
> > Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> >
> > ---
> > kernel/sched/fair.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 479b38dc307a..d8345219dfd4 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -917,23 +917,32 @@ struct sched_entity *__pick_first_entity(struct cfs_rq cfs_rq)
> > }
> >
> > /
> > - * HACK, stash a copy of deadline at the point of pick in vlag,
> > - * which isn't used until dequeue.
> > + * HACK, Set the vruntime, up to which the entity can run before picking
> > + * another one, in vlag, which isn't used until dequeue.
> > + * In case of run to parity, we use the shortest slice of the enqueued
> > + * entities.
> > */
>
> I am going to admit - I don't have a good intuitive sense on how this will affect the functionality. Maybe you can help me think of a test case to explicitly write out this assumption in behavior?

Run to parity minimizes the number of context switches to improve
throughput by letting an entity run its full slice before picking
another entity. When all entities have the same and default
sysctl_sched_base_slice, the latter can be assumed to also be the
quantum q (although this is not really true as the entity can be
preempted during its quantum in our case). In such case, we still
comply with the theorem:
        -rmax < lagk (d) < max(rmax ; q); rmax being the max slice
request of the task k

When entities have different slices duration, we will break this rule
which becomes
       -rmax < lagk (d) < max(max of r ; q); 'max of r' being the
maximum slice of all entities

In order to come back to the 1st version, we can't wait for the end of
the slice of the current task but align with shorter slice

When run to parity is disabled, we can face a similar problem because
we don't enforce a resched periodically. In this case (patch 5), we
use the 0.7ms value as the quantum q.

So I would say that checking -rmax < lagk (d) < max(rmax ; q); when a
task is dequeued should be a good test. We might need to use -(rmax +
tick period) < lagk (d) < max(rmax ; q) + tick period; because of the
way we trigger  resched


>
> Dhaval
>
> > static inline void set_protect_slice(struct sched_entity *se)
> > {
> > - se->vlag = se->deadline;
> >
> > + u64 min_slice;
> > +
> > + min_slice = cfs_rq_min_slice(cfs_rq_of(se));
> > +
> > + if (min_slice != se->slice)
> >
> > + se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
> >
> > + else
> > + se->vlag = se->deadline;
> >
> > }
> >
> > static inline bool protect_slice(struct sched_entity *se)
> > {
> > - return se->vlag == se->deadline;
> >
> > + return ((s64)(se->vlag - se->vruntime) > 0);
> >
> > }
> >
> > static inline void cancel_protect_slice(struct sched_entity *se)
> > {
> > if (protect_slice(se))
> > - se->vlag = se->deadline + 1;
> >
> > + se->vlag = se->vruntime;
> >
> > }
> >
> > /*
> > --
> > 2.43.0
>
>

[PATCH 4/4] sched/fair: Improve NO_RUN_TO_PARITY

[Vincent Guittot]

The slice protection of RUN_TO_PARITY should also be applied with a
minimal quantum of time for NO_RUN_TO_PARITY in order to ensure a minimum
runtime for each task with same slice duration but also to ensure
periodic switch between threads.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 kernel/sched/fair.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d8345219dfd4..73bde511c53b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -921,15 +921,20 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
  * one in vlag, which isn't used until dequeue.
  * In case of run to parity, we use the shortest slice of the enqueued
  * entities.
+ * When run to parity is disable we give a minimum quantum to the
+ * running entity to ensure progress.
  */
 static inline void set_protect_slice(struct sched_entity *se)
 {
-	u64 min_slice;
+	u64 quantum;
 
-	min_slice = cfs_rq_min_slice(cfs_rq_of(se));
+	if (sched_feat(RUN_TO_PARITY))
+		quantum = cfs_rq_min_slice(cfs_rq_of(se));
+	else
+		quantum = min(se->slice, normalized_sysctl_sched_base_slice);
 
-	if (min_slice != se->slice)
-		se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
+	if (quantum != se->slice)
+		se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(quantum, se));
 	else
 		se->vlag = se->deadline;
 }
@@ -981,7 +986,7 @@ static struct sched_entity *pick_eevdf(struct cfs_rq *cfs_rq)
 	if (curr && (!curr->on_rq || !entity_eligible(cfs_rq, curr)))
 		curr = NULL;
 
-	if (sched_feat(RUN_TO_PARITY) && curr && protect_slice(curr))
+	if (curr && protect_slice(curr))
 		return curr;
 
 	/* Pick the leftmost entity if it's eligible */
@@ -1215,11 +1220,8 @@ static inline void update_curr_task(struct task_struct *p, s64 delta_exec)
 	cgroup_account_cputime(p, delta_exec);
 }
 
-static inline bool did_preempt_short(struct cfs_rq *cfs_rq, struct sched_entity *curr)
+static inline bool resched_next_quantum(struct cfs_rq *cfs_rq, struct sched_entity *curr)
 {
-	if (!sched_feat(PREEMPT_SHORT))
-		return false;
-
 	if (protect_slice(curr))
 		return false;
 
@@ -1307,7 +1309,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
 	if (cfs_rq->nr_queued == 1)
 		return;
 
-	if (resched || did_preempt_short(cfs_rq, curr)) {
+	if (resched || resched_next_quantum(cfs_rq, curr)) {
 		resched_curr_lazy(rq);
 		clear_buddies(cfs_rq, curr);
 	}
-- 
2.43.0

Re: [PATCH 4/4] sched/fair: Improve NO_RUN_TO_PARITY

[dhaval]

On Friday, June 13th, 2025 at 7:15 AM, Vincent Guittot <vincent.guittot@linaro.org> wrote:

> 
> 
> The slice protection of RUN_TO_PARITY should also be applied with a
> minimal quantum of time for NO_RUN_TO_PARITY in order to ensure a minimum
> runtime for each task with same slice duration but also to ensure
> periodic switch between threads.
> 
> Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
> 
> ---
> kernel/sched/fair.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d8345219dfd4..73bde511c53b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -921,15 +921,20 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
> * one in vlag, which isn't used until dequeue.
> * In case of run to parity, we use the shortest slice of the enqueued
> * entities.
> + * When run to parity is disable we give a minimum quantum to the

"disabled"

> + * running entity to ensure progress.
> */
> static inline void set_protect_slice(struct sched_entity *se)
> {
> - u64 min_slice;
> + u64 quantum;
> 
> - min_slice = cfs_rq_min_slice(cfs_rq_of(se));
> + if (sched_feat(RUN_TO_PARITY))
> + quantum = cfs_rq_min_slice(cfs_rq_of(se));
> + else
> + quantum = min(se->slice, normalized_sysctl_sched_base_slice);
> 
> 
> - if (min_slice != se->slice)
> 
> - se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
> 
> + if (quantum != se->slice)
> 
> + se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(quantum, se));
> 
> else
> se->vlag = se->deadline;
> 
> }
> @@ -981,7 +986,7 @@ static struct sched_entity *pick_eevdf(struct cfs_rq *cfs_rq)
> if (curr && (!curr->on_rq || !entity_eligible(cfs_rq, curr)))
> 
> curr = NULL;
> 
> - if (sched_feat(RUN_TO_PARITY) && curr && protect_slice(curr))
> + if (curr && protect_slice(curr))
> return curr;
> 
> /* Pick the leftmost entity if it's eligible */
> @@ -1215,11 +1220,8 @@ static inline void update_curr_task(struct task_struct *p, s64 delta_exec)
> cgroup_account_cputime(p, delta_exec);
> }
> 
> -static inline bool did_preempt_short(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> +static inline bool resched_next_quantum(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> {
> - if (!sched_feat(PREEMPT_SHORT))
> - return false;
> -
> if (protect_slice(curr))
> return false;
> 
> @@ -1307,7 +1309,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
> if (cfs_rq->nr_queued == 1)
> 
> return;
> 
> - if (resched || did_preempt_short(cfs_rq, curr)) {
> + if (resched || resched_next_quantum(cfs_rq, curr)) {
> resched_curr_lazy(rq);
> clear_buddies(cfs_rq, curr);
> }
> --
> 2.43.0

Re: [PATCH 4/4] sched/fair: Improve NO_RUN_TO_PARITY

[Vincent Guittot]

On Fri, 13 Jun 2025 at 16:05, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> The slice protection of RUN_TO_PARITY should also be applied with a
> minimal quantum of time for NO_RUN_TO_PARITY in order to ensure a minimum
> runtime for each task with same slice duration but also to ensure
> periodic switch between threads.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>  kernel/sched/fair.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index d8345219dfd4..73bde511c53b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -921,15 +921,20 @@ struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
>   * one in vlag, which isn't used until dequeue.
>   * In case of run to parity, we use the shortest slice of the enqueued
>   * entities.
> + * When run to parity is disable we give a minimum quantum to the
> + * running entity to ensure progress.
>   */
>  static inline void set_protect_slice(struct sched_entity *se)
>  {
> -       u64 min_slice;
> +       u64 quantum;
>
> -       min_slice = cfs_rq_min_slice(cfs_rq_of(se));
> +       if (sched_feat(RUN_TO_PARITY))
> +               quantum = cfs_rq_min_slice(cfs_rq_of(se));
> +       else
> +               quantum = min(se->slice, normalized_sysctl_sched_base_slice);
The above is not correct and needs the fix below on top

-               quantum = min(se->slice, normalized_sysctl_sched_base_slice);
+       quantum = normalized_sysctl_sched_base_slice;

+       quantum = min(se->slice, quantum);

>
> -       if (min_slice != se->slice)
> -               se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(min_slice, se));
> +       if (quantum != se->slice)
> +               se->vlag = min(se->deadline, se->vruntime + calc_delta_fair(quantum, se));
>         else
>                 se->vlag = se->deadline;
>  }
> @@ -981,7 +986,7 @@ static struct sched_entity *pick_eevdf(struct cfs_rq *cfs_rq)
>         if (curr && (!curr->on_rq || !entity_eligible(cfs_rq, curr)))
>                 curr = NULL;
>
> -       if (sched_feat(RUN_TO_PARITY) && curr && protect_slice(curr))
> +       if (curr && protect_slice(curr))
>                 return curr;
>
>         /* Pick the leftmost entity if it's eligible */
> @@ -1215,11 +1220,8 @@ static inline void update_curr_task(struct task_struct *p, s64 delta_exec)
>         cgroup_account_cputime(p, delta_exec);
>  }
>
> -static inline bool did_preempt_short(struct cfs_rq *cfs_rq, struct sched_entity *curr)
> +static inline bool resched_next_quantum(struct cfs_rq *cfs_rq, struct sched_entity *curr)
>  {
> -       if (!sched_feat(PREEMPT_SHORT))
> -               return false;
> -
>         if (protect_slice(curr))
>                 return false;
>
> @@ -1307,7 +1309,7 @@ static void update_curr(struct cfs_rq *cfs_rq)
>         if (cfs_rq->nr_queued == 1)
>                 return;
>
> -       if (resched || did_preempt_short(cfs_rq, curr)) {
> +       if (resched || resched_next_quantum(cfs_rq, curr)) {
>                 resched_curr_lazy(rq);
>                 clear_buddies(cfs_rq, curr);
>         }
> --
> 2.43.0
>

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Peter Zijlstra]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> Vincent Guittot (3):
>   sched/fair: Use protect_slice() instead of direct comparison
>   sched/fair: Limit run to parity to the min slice of enqueued entities
>   sched/fair: Improve NO_RUN_TO_PARITY

Ah. I wrote these here patches and then totally forgot about them :/.
They take a different approach.

The approach I took was to move decision to stick with curr after pick,
instead of before it. That way we can evaluate the tree at the time of
preemption.



[-- Attachment #2: peterz-sched-fair-fix-delayed-requeue.patch --]
[-- Type: text/x-diff, Size: 818 bytes --]

Subject: sched/fair: Fix requeue_delayed_entity()
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri Apr  4 11:23:00 CEST 2025

Since enqueue_task_fair() doesn't call update_curr() before calling
requeue_delayed_entity(), which means that update_entity_lag() uses a
slightly out-of-date avg_vruntime() -- which includes current.

Fixes: 54a58a787791 ("sched/fair: Implement DELAY_ZERO")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6893,6 +6893,8 @@ requeue_delayed_entity(struct sched_enti
 {
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
+	update_curr(cfs_rq);
+
 	/*
 	 * se->sched_delayed should imply: se->on_rq == 1.
 	 * Because a delayed entity is one that is still on

[-- Attachment #3: peterz-sched-fair-fix-preempt-short.patch --]
[-- Type: text/x-diff, Size: 2298 bytes --]

Subject: sched/eevdf: Re-arrange current protection in pick_eevdf()
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri Apr 4 10:15:15 CEST 2025

The way pick_eevdf() limits preemption is by explicitly picking
current if it is still eligible. It does this without consideration of
the best in-tree task.

Move current protection to after the tree selection such that a follow
up patch can change the conditions.

Should be an semantics no-op at this point.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -903,6 +903,26 @@ static inline void cancel_protect_slice(
 		se->vlag = se->deadline + 1;
 }
 
+static inline bool __pick_eevdf_curr(struct cfs_rq *cfs_rq, struct sched_entity *best)
+{
+	struct sched_entity *curr = cfs_rq->curr;
+
+	/* only called when this is already checked */
+	WARN_ON_ONCE(!curr || !curr->on_rq);
+
+	/*
+	 * Strictly speaking we should allow current to run until its
+	 * deadline. However allow (wakeup) preemption once it is no longer
+	 * eligible.
+	 */
+	if (sched_feat(RUN_TO_PARITY) &&
+	    protect_slice(curr) &&
+	    entity_eligible(cfs_rq, curr))
+		return true;
+
+	return entity_before(curr, best);
+}
+
 /*
  * Earliest Eligible Virtual Deadline First
  *
@@ -929,18 +949,15 @@ static struct sched_entity *pick_eevdf(s
 	struct sched_entity *curr = cfs_rq->curr;
 	struct sched_entity *best = NULL;
 
+	if (curr && !curr->on_rq)
+		curr = NULL;
+
 	/*
 	 * We can safely skip eligibility check if there is only one entity
 	 * in this cfs_rq, saving some cycles.
 	 */
 	if (cfs_rq->nr_queued == 1)
-		return curr && curr->on_rq ? curr : se;
-
-	if (curr && (!curr->on_rq || !entity_eligible(cfs_rq, curr)))
-		curr = NULL;
-
-	if (sched_feat(RUN_TO_PARITY) && curr && protect_slice(curr))
-		return curr;
+		return curr ?: se;
 
 	/* Pick the leftmost entity if it's eligible */
 	if (se && entity_eligible(cfs_rq, se)) {
@@ -977,7 +994,7 @@ static struct sched_entity *pick_eevdf(s
 		node = node->rb_right;
 	}
 found:
-	if (!best || (curr && entity_before(curr, best)))
+	if (!best || (curr && __pick_eevdf_curr(cfs_rq, best)))
 		best = curr;
 
 	return best;

[-- Attachment #4: peterz-sched-fair-fix-preempt-short-2.patch --]
[-- Type: text/x-diff, Size: 1019 bytes --]

Subject: sched/eevdf: Fix RUN_TO_PARITY vs PREEMPT_SHORT
From: Peter Zijlstra <peterz@infradead.org>
Date: Fri Apr 4 10:25:03 CEST 2025

Vincent noted that RUN_TO_PARITY can prevent preemption by a shorter
slice under some conditions.

Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -918,7 +918,7 @@ static inline bool __pick_eevdf_curr(str
 	if (sched_feat(RUN_TO_PARITY) &&
 	    protect_slice(curr) &&
 	    entity_eligible(cfs_rq, curr))
-		return true;
+		return !sched_feat(PREEMPT_SHORT) || curr->slice < best->slice;
 
 	return entity_before(curr, best);
 }
@@ -1195,7 +1195,7 @@ static inline bool did_preempt_short(str
 	if (!sched_feat(PREEMPT_SHORT))
 		return false;
 
-	if (curr->vlag == curr->deadline)
+	if (protect_slice(curr))
 		return false;
 
 	return !entity_eligible(cfs_rq, curr);

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Vincent Guittot]

On Tue, 17 Jun 2025 at 11:22, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> > Vincent Guittot (3):
> >   sched/fair: Use protect_slice() instead of direct comparison
> >   sched/fair: Limit run to parity to the min slice of enqueued entities
> >   sched/fair: Improve NO_RUN_TO_PARITY
>
> Ah. I wrote these here patches and then totally forgot about them :/.
> They take a different approach.
>
> The approach I took was to move decision to stick with curr after pick,
> instead of before it. That way we can evaluate the tree at the time of
> preemption.

Let me have a look at your patches

>
>

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Vincent Guittot]

On Wed, 18 Jun 2025 at 09:03, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Tue, 17 Jun 2025 at 11:22, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> > > Vincent Guittot (3):
> > >   sched/fair: Use protect_slice() instead of direct comparison
> > >   sched/fair: Limit run to parity to the min slice of enqueued entities
> > >   sched/fair: Improve NO_RUN_TO_PARITY
> >
> > Ah. I wrote these here patches and then totally forgot about them :/.
> > They take a different approach.
> >
> > The approach I took was to move decision to stick with curr after pick,
> > instead of before it. That way we can evaluate the tree at the time of
> > preemption.
>
> Let me have a look at your patches

I have looked and tested your patches but they don't solve the lag and
run to parity issues not sur what he's going wrong. Also, my patchset
take into account the NO_RUN_TO_PARITY case by adding a notion of
quantum execution time which was missing until now
Regarding the "fix delayed requeue", I already get an update of
current before requeueing a delayed task. Do you have a use case in
mind ?

>
> >
> >

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Peter Zijlstra]

On Thu, Jun 19, 2025 at 02:27:43PM +0200, Vincent Guittot wrote:
> On Wed, 18 Jun 2025 at 09:03, Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> > On Tue, 17 Jun 2025 at 11:22, Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> > > > Vincent Guittot (3):
> > > >   sched/fair: Use protect_slice() instead of direct comparison
> > > >   sched/fair: Limit run to parity to the min slice of enqueued entities
> > > >   sched/fair: Improve NO_RUN_TO_PARITY
> > >
> > > Ah. I wrote these here patches and then totally forgot about them :/.
> > > They take a different approach.
> > >
> > > The approach I took was to move decision to stick with curr after pick,
> > > instead of before it. That way we can evaluate the tree at the time of
> > > preemption.
> >
> > Let me have a look at your patches
> 
> I have looked and tested your patches but they don't solve the lag and
> run to parity issues not sur what he's going wrong.

Humm.. So what you do in patch 3, setting the protection to min_slice
instead of the deadline, that only takes into account the tasks present
at the point we schedule.

Which is why I approached it by moving the protection to after pick;
because then we can directly compare the task we're running to the
best pick -- which includes the tasks that got woken. This gives
check_preempt_wakeup_fair() better chances.

To be fair, I did not get around to testing the patches much beyond
booting them, so quite possibly they're buggered :-/

> Also, my patchset take into account the NO_RUN_TO_PARITY case by
> adding a notion of quantum execution time which was missing until now

Right; not ideal, but I suppose for the people that disable
RUN_TO_PARITY it might make sense. But perhaps there should be a little
more justification for why we bother tweaking a non-default option.

The problem with usage of normalized_sysctl_ values is that you then get
behavioural differences between 1 and 8 CPUs or so. Also, perhaps its
time to just nuke that whole scaling thing (I'm sure someone mentioned
that a short while ago).

> Regarding the "fix delayed requeue", I already get an update of
> current before requeueing a delayed task. Do you have a use case in
> mind ?

Ah, it was just from reading code, clearly I missed something. Happy to
forget about that patch :-)

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Vincent Guittot]

On Fri, 20 Jun 2025 at 10:42, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 19, 2025 at 02:27:43PM +0200, Vincent Guittot wrote:
> > On Wed, 18 Jun 2025 at 09:03, Vincent Guittot
> > <vincent.guittot@linaro.org> wrote:
> > >
> > > On Tue, 17 Jun 2025 at 11:22, Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> > > > > Vincent Guittot (3):
> > > > >   sched/fair: Use protect_slice() instead of direct comparison
> > > > >   sched/fair: Limit run to parity to the min slice of enqueued entities
> > > > >   sched/fair: Improve NO_RUN_TO_PARITY
> > > >
> > > > Ah. I wrote these here patches and then totally forgot about them :/.
> > > > They take a different approach.
> > > >
> > > > The approach I took was to move decision to stick with curr after pick,
> > > > instead of before it. That way we can evaluate the tree at the time of
> > > > preemption.
> > >
> > > Let me have a look at your patches
> >
> > I have looked and tested your patches but they don't solve the lag and
> > run to parity issues not sur what he's going wrong.
>
> Humm.. So what you do in patch 3, setting the protection to min_slice
> instead of the deadline, that only takes into account the tasks present
> at the point we schedule.

yes but at this point any waking up task is either the next running
task or enqueued in the rb tree

>
> Which is why I approached it by moving the protection to after pick;
> because then we can directly compare the task we're running to the
> best pick -- which includes the tasks that got woken. This gives
> check_preempt_wakeup_fair() better chances.

we don't always want to break the run to parity but only when a task
wakes up and should preempt current or decrease the run to parity
period. Otherwise, the protection applies for a duration that is short
enough to stay fair for others

I will see if check_preempt_wakeup_fair can be smarter when deciding
to cancel the protection

>
> To be fair, I did not get around to testing the patches much beyond
> booting them, so quite possibly they're buggered :-/
>
> > Also, my patchset take into account the NO_RUN_TO_PARITY case by
> > adding a notion of quantum execution time which was missing until now
>
> Right; not ideal, but I suppose for the people that disable
> RUN_TO_PARITY it might make sense. But perhaps there should be a little
> more justification for why we bother tweaking a non-default option.

Otherwise disabling RUN_TO_PARITY to check if it's the root cause of a
regression or a problem becomes pointless because the behavior without
the feature is wrong.
And some might not want to run to parity but behave closer to the
white paper with a pick after each quantum with quantum being
something in the range [0.7ms:2*tick)

>
> The problem with usage of normalized_sysctl_ values is that you then get
> behavioural differences between 1 and 8 CPUs or so. Also, perhaps its

normalized_sysctl_ values don't scale with the number of CPUs. In this
case, it's always 0.7ms which is short enough compare to 1ms tick
period to prevent default irq accounting to keep current for another
tick

> time to just nuke that whole scaling thing (I'm sure someone mentioned
> that a short while ago).
>
> > Regarding the "fix delayed requeue", I already get an update of
> > current before requeueing a delayed task. Do you have a use case in
> > mind ?
>
> Ah, it was just from reading code, clearly I missed something. Happy to
> forget about that patch :-)

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Peter Zijlstra]

On Fri, Jun 20, 2025 at 12:29:27PM +0200, Vincent Guittot wrote:

> yes but at this point any waking up task is either the next running
> task or enqueued in the rb tree

The scenario I was thinking of was something like:

A (long slice)
B (short slice)
C (short slice)

  A wakes up and goes running

Since A is the only task around, it gets normal protection
 
  B wakes up and doesn't win

So now we have A running with long protection and short task on-rq

  C wakes up ...

Whereas what we would've wanted to end up with for C is A running with
short protection.

> > Which is why I approached it by moving the protection to after pick;
> > because then we can directly compare the task we're running to the
> > best pick -- which includes the tasks that got woken. This gives
> > check_preempt_wakeup_fair() better chances.
> 
> we don't always want to break the run to parity but only when a task
> wakes up and should preempt current or decrease the run to parity
> period. Otherwise, the protection applies for a duration that is short
> enough to stay fair for others
> 
> I will see if check_preempt_wakeup_fair can be smarter when deciding
> to cancel the protection

Thanks. In the above scenario B getting selected when C wakes up would
be a clue I suppose :-)

> > To be fair, I did not get around to testing the patches much beyond
> > booting them, so quite possibly they're buggered :-/
> >
> > > Also, my patchset take into account the NO_RUN_TO_PARITY case by
> > > adding a notion of quantum execution time which was missing until now
> >
> > Right; not ideal, but I suppose for the people that disable
> > RUN_TO_PARITY it might make sense. But perhaps there should be a little
> > more justification for why we bother tweaking a non-default option.
> 
> Otherwise disabling RUN_TO_PARITY to check if it's the root cause of a
> regression or a problem becomes pointless because the behavior without
> the feature is wrong.

Fair enough.

> And some might not want to run to parity but behave closer to the
> white paper with a pick after each quantum with quantum being
> something in the range [0.7ms:2*tick)
> 
> >
> > The problem with usage of normalized_sysctl_ values is that you then get
> > behavioural differences between 1 and 8 CPUs or so. Also, perhaps its
> 
> normalized_sysctl_ values don't scale with the number of CPUs. In this
> case, it's always 0.7ms which is short enough compare to 1ms tick
> period to prevent default irq accounting to keep current for another
> tick

Right; but it not scaling means it is the full slice on UP, half the
slice on SMP-4 and a third for SMP-8 and up or somesuch.

It probably doesn't matter much, but its weird.

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Vincent Guittot]

On Mon, 23 Jun 2025 at 13:16, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Fri, Jun 20, 2025 at 12:29:27PM +0200, Vincent Guittot wrote:
>
> > yes but at this point any waking up task is either the next running
> > task or enqueued in the rb tree
>
> The scenario I was thinking of was something like:
>
> A (long slice)
> B (short slice)
> C (short slice)
>
>   A wakes up and goes running
>
> Since A is the only task around, it gets normal protection
>
>   B wakes up and doesn't win
>
> So now we have A running with long protection and short task on-rq
>
>   C wakes up ...
>
> Whereas what we would've wanted to end up with for C is A running with
> short protection.

I will look at this case more deeply. We might want to update the
slice protection with the new min slice even if B doesn't preempt A.
That's part of a smarter check_preempt_wakeup_fair that I mentioned
below.
In case of B deadline not being before A, we don't need to update the
protection as the remaining protect duration is already shorter than
the new slice
In case of B not eligible and already on the cpu, B is already
enqueued (delayed dequeue) so its short slice is already accounted in
set protection. I still have to look at B being migrated from another
CPU with negative lag

>
> > > Which is why I approached it by moving the protection to after pick;
> > > because then we can directly compare the task we're running to the
> > > best pick -- which includes the tasks that got woken. This gives
> > > check_preempt_wakeup_fair() better chances.
> >
> > we don't always want to break the run to parity but only when a task
> > wakes up and should preempt current or decrease the run to parity
> > period. Otherwise, the protection applies for a duration that is short
> > enough to stay fair for others
> >
> > I will see if check_preempt_wakeup_fair can be smarter when deciding
> > to cancel the protection
>
> Thanks. In the above scenario B getting selected when C wakes up would
> be a clue I suppose :-)

yes, fixing the comment :
* Note that even if @p does not turn out to be the most eligible
* task at this moment, current's slice protection will be lost.

>
> > > To be fair, I did not get around to testing the patches much beyond
> > > booting them, so quite possibly they're buggered :-/
> > >
> > > > Also, my patchset take into account the NO_RUN_TO_PARITY case by
> > > > adding a notion of quantum execution time which was missing until now
> > >
> > > Right; not ideal, but I suppose for the people that disable
> > > RUN_TO_PARITY it might make sense. But perhaps there should be a little
> > > more justification for why we bother tweaking a non-default option.
> >
> > Otherwise disabling RUN_TO_PARITY to check if it's the root cause of a
> > regression or a problem becomes pointless because the behavior without
> > the feature is wrong.
>
> Fair enough.
>
> > And some might not want to run to parity but behave closer to the
> > white paper with a pick after each quantum with quantum being
> > something in the range [0.7ms:2*tick)
> >
> > >
> > > The problem with usage of normalized_sysctl_ values is that you then get
> > > behavioural differences between 1 and 8 CPUs or so. Also, perhaps its
> >
> > normalized_sysctl_ values don't scale with the number of CPUs. In this
> > case, it's always 0.7ms which is short enough compare to 1ms tick
> > period to prevent default irq accounting to keep current for another
> > tick
>
> Right; but it not scaling means it is the full slice on UP, half the
> slice on SMP-4 and a third for SMP-8 and up or somesuch.

Yes, the goal is to implement a kind of quantum of time which doesn't
scale with number CPUs unlike the default slice duration

>
> It probably doesn't matter much, but its weird.

Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices

[Dhaval Giani]

On Tue, Jun 17, 2025 at 11:22:08AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 13, 2025 at 04:05:10PM +0200, Vincent Guittot wrote:
> > Vincent Guittot (3):
> >   sched/fair: Use protect_slice() instead of direct comparison
> >   sched/fair: Limit run to parity to the min slice of enqueued entities
> >   sched/fair: Improve NO_RUN_TO_PARITY
> 
> Ah. I wrote these here patches and then totally forgot about them :/.
> They take a different approach.
> 

Mind sticking them on a git tree somewhere please?

Dhaval