From: Peter Zijlstra <peterz@infradead.org>
To: mingo@kernel.org, tj@kernel.org, void@manifault.com
Cc: peterz@infradead.org, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, linux-kernel@vger.kernel.org,
joelaf@google.com
Subject: [PATCH 7/9] sched: Rework dl_server
Date: Wed, 14 Aug 2024 00:25:55 +0200 [thread overview]
Message-ID: <20240813224016.259853414@infradead.org> (raw)
In-Reply-To: 20240813222548.049744955@infradead.org
When a task is selected through a dl_server, it will have p->dl_server
set, such that it can account runtime to the dl_server, see
update_curr_task().
Currently p->dl_server is set in pick*task() whenever it goes through
the dl_server, clearing it is a bit of a mess though. The trivial
solution is clearing it on the final put (now that we have this
location).
However, this gives a problem when:
p = pick_task(rq);
if (p)
put_prev_set_next_task(rq, prev, next);
picks the same task but through a different path, notably when it goes
from picking through the dl_server to a direct pick or vice-versa. In
that case we cannot readily determine wether we should clear or
preserve p->dl_server.
An additional complication is pick_*task() setting p->dl_server for a
remote pick, it might still need to update runtime before it schedules
the core_pick.
Close all these holes and remove all the random clearing of
p->dl_server by:
- having pick_*task() manage rq->dl_server
- having the final put_prev_task() clear p->dl_server
- having the first set_next_task() set p->dl_server = rq->dl_server
- complicate the core_sched code to save/restore rq->dl_server where
appropriate.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
kernel/sched/core.c | 40 +++++++++++++++-------------------------
kernel/sched/deadline.c | 2 +-
kernel/sched/fair.c | 10 ++--------
kernel/sched/sched.h | 14 ++++++++++++++
4 files changed, 32 insertions(+), 34 deletions(-)
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3668,8 +3668,6 @@ ttwu_do_activate(struct rq *rq, struct t
rq->idle_stamp = 0;
}
#endif
-
- p->dl_server = NULL;
}
/*
@@ -5859,14 +5857,6 @@ static void prev_balance(struct rq *rq,
break;
}
#endif
-
- /*
- * We've updated @prev and no longer need the server link, clear it.
- * Must be done before ->pick_next_task() because that can (re)set
- * ->dl_server.
- */
- if (prev->dl_server)
- prev->dl_server = NULL;
}
/*
@@ -5878,6 +5868,8 @@ __pick_next_task(struct rq *rq, struct t
const struct sched_class *class;
struct task_struct *p;
+ rq->dl_server = NULL;
+
/*
* Optimization: we know that if all tasks are in the fair class we can
* call that function directly, but only if the @prev task wasn't of a
@@ -5897,20 +5889,6 @@ __pick_next_task(struct rq *rq, struct t
put_prev_set_next_task(rq, prev, p);
}
- /*
- * This is a normal CFS pick, but the previous could be a DL pick.
- * Clear it as previous is no longer picked.
- */
- if (prev->dl_server)
- prev->dl_server = NULL;
-
- /*
- * This is the fast path; it cannot be a DL server pick;
- * therefore even if @p == @prev, ->dl_server must be NULL.
- */
- if (p->dl_server)
- p->dl_server = NULL;
-
return p;
}
@@ -5958,6 +5936,8 @@ static inline struct task_struct *pick_t
const struct sched_class *class;
struct task_struct *p;
+ rq->dl_server = NULL;
+
for_each_class(class) {
p = class->pick_task(rq);
if (p)
@@ -5996,6 +5976,7 @@ pick_next_task(struct rq *rq, struct tas
* another cpu during offline.
*/
rq->core_pick = NULL;
+ rq->core_dl_server = NULL;
return __pick_next_task(rq, prev, rf);
}
@@ -6014,7 +5995,9 @@ pick_next_task(struct rq *rq, struct tas
WRITE_ONCE(rq->core_sched_seq, rq->core->core_pick_seq);
next = rq->core_pick;
+ rq->dl_server = rq->core_dl_server;
rq->core_pick = NULL;
+ rq->core_dl_server = NULL;
goto out_set_next;
}
@@ -6059,6 +6042,7 @@ pick_next_task(struct rq *rq, struct tas
next = pick_task(rq);
if (!next->core_cookie) {
rq->core_pick = NULL;
+ rq->core_dl_server = NULL;
/*
* For robustness, update the min_vruntime_fi for
* unconstrained picks as well.
@@ -6086,7 +6070,9 @@ pick_next_task(struct rq *rq, struct tas
if (i != cpu && (rq_i != rq->core || !core_clock_updated))
update_rq_clock(rq_i);
- p = rq_i->core_pick = pick_task(rq_i);
+ rq_i->core_pick = p = pick_task(rq_i);
+ rq_i->core_dl_server = rq_i->dl_server;
+
if (!max || prio_less(max, p, fi_before))
max = p;
}
@@ -6110,6 +6096,7 @@ pick_next_task(struct rq *rq, struct tas
}
rq_i->core_pick = p;
+ rq_i->core_dl_server = NULL;
if (p == rq_i->idle) {
if (rq_i->nr_running) {
@@ -6170,6 +6157,7 @@ pick_next_task(struct rq *rq, struct tas
if (i == cpu) {
rq_i->core_pick = NULL;
+ rq_i->core_dl_server = NULL;
continue;
}
@@ -6178,6 +6166,7 @@ pick_next_task(struct rq *rq, struct tas
if (rq_i->curr == rq_i->core_pick) {
rq_i->core_pick = NULL;
+ rq_i->core_dl_server = NULL;
continue;
}
@@ -8401,6 +8390,7 @@ void __init sched_init(void)
#ifdef CONFIG_SCHED_CORE
rq->core = rq;
rq->core_pick = NULL;
+ rq->core_dl_server = NULL;
rq->core_enabled = 0;
rq->core_tree = RB_ROOT;
rq->core_forceidle_count = 0;
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2429,7 +2429,7 @@ static struct task_struct *__pick_task_d
update_curr_dl_se(rq, dl_se, 0);
goto again;
}
- p->dl_server = dl_se;
+ rq->dl_server = dl_se;
} else {
p = dl_task_of(dl_se);
}
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8747,14 +8747,6 @@ static struct task_struct *pick_task_fai
cfs_rq = group_cfs_rq(se);
} while (cfs_rq);
- /*
- * This can be called from directly from CFS's ->pick_task() or indirectly
- * from DL's ->pick_task when fair server is enabled. In the indirect case,
- * DL will set ->dl_server just after this function is called, so its Ok to
- * clear. In the direct case, we are picking directly so we must clear it.
- */
- task_of(se)->dl_server = NULL;
-
return task_of(se);
}
@@ -8778,6 +8770,8 @@ pick_next_task_fair(struct rq *rq, struc
if (prev->sched_class != &fair_sched_class)
goto simple;
+ __put_prev_set_next_dl_server(rq, prev, p);
+
/*
* Because of the set_next_buddy() in dequeue_task_fair() it is rather
* likely that a next task is from the same cgroup as the current.
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1066,6 +1066,7 @@ struct rq {
unsigned int nr_uninterruptible;
struct task_struct __rcu *curr;
+ struct sched_dl_entity *dl_server;
struct task_struct *idle;
struct task_struct *stop;
unsigned long next_balance;
@@ -1193,6 +1194,7 @@ struct rq {
/* per rq */
struct rq *core;
struct task_struct *core_pick;
+ struct sched_dl_entity *core_dl_server;
unsigned int core_enabled;
unsigned int core_sched_seq;
struct rb_root core_tree;
@@ -2370,12 +2372,24 @@ static inline void set_next_task(struct
next->sched_class->set_next_task(rq, next, false);
}
+static inline void
+__put_prev_set_next_dl_server(struct rq *rq,
+ struct task_struct *prev,
+ struct task_struct *next)
+{
+ prev->dl_server = NULL;
+ next->dl_server = rq->dl_server;
+ rq->dl_server = NULL;
+}
+
static inline void put_prev_set_next_task(struct rq *rq,
struct task_struct *prev,
struct task_struct *next)
{
WARN_ON_ONCE(rq->curr != prev);
+ __put_prev_set_next_dl_server(rq, prev, next);
+
if (next == prev)
return;
next prev parent reply other threads:[~2024-08-13 22:50 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-13 22:25 [PATCH 0/9] sched: Prepare for sched_ext Peter Zijlstra
2024-08-13 22:25 ` [PATCH 1/9] sched: Use set_next_task(.first) where required Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 2/9] sched: Fixup set_next_task() implementations Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 3/9] sched: Clean up DL server vs core sched Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 4/9] sched: Split up put_prev_task_balance() Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 5/9] sched: Rework pick_next_task() Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 6/9] sched: Combine the last put_prev_task() and the first set_next_task() Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` Peter Zijlstra [this message]
2024-09-03 13:38 ` [tip: sched/core] sched: Rework dl_server tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 8/9] sched: Add put_prev_task(.next) Peter Zijlstra
2024-09-03 13:38 ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-08-13 22:25 ` [PATCH 9/9] sched: Add pick_task(.core) Peter Zijlstra
2024-08-13 22:59 ` Peter Zijlstra
2024-08-14 10:42 ` Juri Lelli
2024-08-21 23:05 ` Tejun Heo
2024-09-03 13:31 ` Peter Zijlstra
2024-08-17 21:56 ` [PATCH 0/9] sched: Prepare for sched_ext Joel Fernandes
2024-08-21 21:41 ` Joel Fernandes
2024-08-22 12:58 ` Joel Fernandes
2024-08-22 13:15 ` Joel Fernandes
2024-08-22 13:37 ` Joel Fernandes
2024-08-22 15:48 ` Peter Zijlstra
2024-09-04 13:56 ` Peter Zijlstra
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=20240813224016.259853414@infradead.org \
--to=peterz@infradead.org \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=joelaf@google.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.org \
--cc=tj@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=void@manifault.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.