The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices
Date: Tue, 17 Jun 2025 11:22:08 +0200	[thread overview]
Message-ID: <20250617092208.GQ1613376@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20250613140514.2781138-1-vincent.guittot@linaro.org>

[-- 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);

  parent reply	other threads:[~2025-06-17  9:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-13 14:05 [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices Vincent Guittot
2025-06-13 14:05 ` [PATCH 1/4] sched/fair: Use protect_slice() instead of direct comparison Vincent Guittot
2025-06-13 17:39   ` dhaval
2025-06-13 14:05 ` [PATCH 2/4] sched/fair: Increase max lag clamping Vincent Guittot
2025-06-13 21:00   ` dhaval
2025-06-16 14:51     ` Vincent Guittot
2025-06-13 14:05 ` [PATCH 3/4] sched/fair: Limit run to parity to the min slice of enqueued entities Vincent Guittot
2025-06-13 22:53   ` dhaval
2025-06-16 12:37     ` Vincent Guittot
2025-06-13 14:05 ` [PATCH 4/4] sched/fair: Improve NO_RUN_TO_PARITY Vincent Guittot
2025-06-13 22:55   ` dhaval
2025-06-19 12:31   ` Vincent Guittot
2025-06-17  9:22 ` Peter Zijlstra [this message]
2025-06-18  7:03   ` [PATCH 0/4] sched/fair: Manage lag and run to parity with different slices Vincent Guittot
2025-06-19 12:27     ` Vincent Guittot
2025-06-20  8:42       ` Peter Zijlstra
2025-06-20 10:29         ` Vincent Guittot
2025-06-23 11:16           ` Peter Zijlstra
2025-06-23 16:27             ` Vincent Guittot
2025-06-25 19:45   ` Dhaval Giani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250617092208.GQ1613376@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox