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);
next prev 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