From: Tobias Huschle <huschle@linux.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
bristot@redhat.com, vschneid@redhat.com,
linux-kernel@vger.kernel.org, kprateek.nayak@amd.com,
wuyun.abel@bytedance.com, tglx@linutronix.de, efault@gmx.de
Subject: Re: [RFC][PATCH 08/10] sched/fair: Implement delayed dequeue
Date: Mon, 22 Apr 2024 15:13:23 +0200 [thread overview]
Message-ID: <ZiZic8t72/tG+z0T@DESKTOP-2CCOB1S.> (raw)
In-Reply-To: <20240405110010.631664251@infradead.org>
On Fri, Apr 05, 2024 at 12:28:02PM +0200, Peter Zijlstra wrote:
> Extend / fix 86bfbb7ce4f6 ("sched/fair: Add lag based placement") by
> noting that lag is fundamentally a temporal measure. It should not be
> carried around indefinitely.
>
> OTOH it should also not be instantly discarded, doing so will allow a
> task to game the system by purposefully (micro) sleeping at the end of
> its time quantum.
>
> Since lag is intimately tied to the virtual time base, a wall-time
> based decay is also insufficient, notably competition is required for
> any of this to make sense.
>
> Instead, delay the dequeue and keep the 'tasks' on the runqueue,
> competing until they are eligible.
>
> Strictly speaking, we only care about keeping them until the 0-lag
> point, but that is a difficult proposition, instead carry them around
> until they get picked again, and dequeue them at that point.
>
> Since we should have dequeued them at the 0-lag point, truncate lag
> (eg. don't let them earn positive lag).
>
I stumbled upon a subtle change between CFS and EEVDF which kinda relates
to the question how long lag should be carried around, but on the other
side of the 0-lag value.
Assume that we have two tasks taking turns on a single CPU.
Task 1 does something and wakes up Task 2.
Task 2 does something and goes to sleep.
And we're just repeating that.
Task 1 and task 2 only run for very short amounts of time, i.e. much
shorter than a regular time slice.
Let's further assume, that task 1 runs longer than task 2.
In CFS, this means, that vruntime of task 1 starts to outrun the vruntime
of task 2. This means that vruntime(task2) < vruntime(task1). Hence, task 2
always gets picked on wake up because it has the smaller vruntime.
In EEVDF, this would translate to a permanent positive lag, which also
causes task 2 to get consistently scheduled on wake up.
Let's now assume, that ocassionally, task 2 runs a little bit longer than
task 1. In CFS, this means, that task 2 can close the vruntime gap by a
bit, but, it can easily remain below the value of task 1. Task 2 would
still get picked on wake up.
With EEVDF, in its current form, task 2 will now get a negative lag, which
in turn, will cause it not being picked on the next wake up.
So, it seems we have a change in the level of how far the both variants look
into the past. CFS being willing to take more history into account, whereas
EEVDF does not (with update_entity_lag setting the lag value from scratch,
and place_entity not taking the original vruntime into account).
All of this can be seen as correct by design, a task consumes more time
than the others, so it has to give way to others. The big difference
is now, that CFS allowed a task to collect some bonus by constantly using
less CPU time than others and trading that time against ocassionally taking
more CPU time. EEVDF could do the same thing, by allowing the accumulation
of (positive) lag, which can then be traded against the one time the task
would get negative lag.
This might of course clash with other EEVDF assumptions but blends with the
question on how long lag should be preserved.
I stumbled upon a performance degredation caused by this nuance. EEVDF is
not necessarily at fault here. The problem rather lies in a component
that relies on CFS allowing the above mentioned bonus aggregation while
knowing that the corresponding task 2 has to run.
See: https://lore.kernel.org/netdev/ZWbapeL34Z8AMR5f@DESKTOP-2CCOB1S./T/
With the reasoning above, the degradation can be explained perfectly, and
even be fixed by allowing the accumulation of lag by applying the patch
below.
I was just wondering if my assumptions here are correct and whether this
would be an implicit change that one should be aware about.
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 03be0d1330a6..b83a72311d2a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -701,7 +701,7 @@ static void update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
s64 lag, limit;
SCHED_WARN_ON(!se->on_rq);
- lag = avg_vruntime(cfs_rq) - se->vruntime;
+ lag = se->vlag + avg_vruntime(cfs_rq) - se->vruntime;
limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
se->vlag = clamp(lag, -limit, limit);
next prev parent reply other threads:[~2024-04-22 13:14 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-05 10:27 [RFC][PATCH 00/10] sched/fair: Complete EEVDF Peter Zijlstra
2024-04-05 10:27 ` [RFC][PATCH 01/10] sched/eevdf: Add feature comments Peter Zijlstra
2024-04-05 10:27 ` [RFC][PATCH 02/10] sched/eevdf: Remove min_vruntime_copy Peter Zijlstra
2024-04-05 10:27 ` [RFC][PATCH 03/10] sched/fair: Cleanup pick_task_fair() vs throttle Peter Zijlstra
2024-04-05 21:11 ` Benjamin Segall
2024-04-05 10:27 ` [RFC][PATCH 04/10] sched/fair: Cleanup pick_task_fair()s curr Peter Zijlstra
2024-04-05 10:27 ` [RFC][PATCH 05/10] sched/fair: Unify pick_{,next_}_task_fair() Peter Zijlstra
2024-04-06 2:20 ` Mike Galbraith
2024-04-05 10:28 ` [RFC][PATCH 06/10] sched: Allow sched_class::dequeue_task() to fail Peter Zijlstra
2024-04-05 10:28 ` [RFC][PATCH 07/10] sched/fair: Re-organize dequeue_task_fair() Peter Zijlstra
2024-04-05 10:28 ` [RFC][PATCH 08/10] sched/fair: Implement delayed dequeue Peter Zijlstra
2024-04-06 9:23 ` Chen Yu
2024-04-08 9:06 ` Peter Zijlstra
2024-04-11 1:32 ` Yan-Jie Wang
2024-04-25 10:25 ` Peter Zijlstra
2024-04-12 10:42 ` K Prateek Nayak
2024-04-15 10:56 ` Mike Galbraith
2024-04-16 3:18 ` K Prateek Nayak
2024-04-16 5:36 ` Mike Galbraith
2024-04-18 16:24 ` Mike Galbraith
2024-04-18 17:08 ` K Prateek Nayak
2024-04-24 15:20 ` Peter Zijlstra
2024-04-25 11:28 ` Peter Zijlstra
2024-04-26 10:56 ` Peter Zijlstra
2024-04-26 11:16 ` Peter Zijlstra
2024-04-26 16:03 ` Mike Galbraith
2024-04-27 6:42 ` Mike Galbraith
2024-04-28 16:32 ` Mike Galbraith
2024-04-29 12:14 ` Peter Zijlstra
2024-04-15 17:07 ` Luis Machado
2024-04-24 15:15 ` Luis Machado
2024-04-25 10:42 ` Peter Zijlstra
2024-04-25 11:49 ` Peter Zijlstra
2024-04-26 9:32 ` Peter Zijlstra
2024-04-26 9:36 ` Peter Zijlstra
2024-04-26 10:16 ` Luis Machado
2024-04-29 14:33 ` Luis Machado
2024-05-02 10:26 ` Luis Machado
2024-05-10 14:49 ` Luis Machado
2024-05-15 9:36 ` Peter Zijlstra
2024-05-15 11:48 ` Peter Zijlstra
2024-05-15 18:03 ` Mike Galbraith
2024-05-20 15:20 ` Luis Machado
2024-04-26 10:15 ` Luis Machado
2024-04-20 5:57 ` Mike Galbraith
2024-04-22 13:13 ` Tobias Huschle [this message]
2024-04-05 10:28 ` [RFC][PATCH 09/10] sched/eevdf: Allow shorter slices to wakeup-preempt Peter Zijlstra
2024-04-05 10:28 ` [RFC][PATCH 10/10] sched/eevdf: Use sched_attr::sched_runtime to set request/slice suggestion Peter Zijlstra
2024-04-06 8:16 ` Hillf Danton
2024-05-07 5:34 ` Mike Galbraith
2024-05-15 10:13 ` Peter Zijlstra
2024-05-07 15:15 ` Chen Yu
2024-05-08 13:52 ` Mike Galbraith
2024-05-09 3:48 ` Chen Yu
2024-05-09 5:00 ` Mike Galbraith
2024-05-13 4:07 ` K Prateek Nayak
2024-05-14 9:18 ` Chen Yu
2024-05-14 15:23 ` K Prateek Nayak
2024-05-14 16:15 ` Chen Yu
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=ZiZic8t72/tG+z0T@DESKTOP-2CCOB1S. \
--to=huschle@linux.ibm.com \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=efault@gmx.de \
--cc=juri.lelli@redhat.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=wuyun.abel@bytedance.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).