Skip to content

Commit bd9bbc9

Browse files
author
Peter Zijlstra
committed
sched: Rework dl_server
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) <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 436f3ee commit bd9bbc9

File tree

4 files changed

+32
-34
lines changed

4 files changed

+32
-34
lines changed

kernel/sched/core.c

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3668,8 +3668,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
36683668
rq->idle_stamp = 0;
36693669
}
36703670
#endif
3671-
3672-
p->dl_server = NULL;
36733671
}
36743672

36753673
/*
@@ -5859,14 +5857,6 @@ static void prev_balance(struct rq *rq, struct task_struct *prev,
58595857
break;
58605858
}
58615859
#endif
5862-
5863-
/*
5864-
* We've updated @prev and no longer need the server link, clear it.
5865-
* Must be done before ->pick_next_task() because that can (re)set
5866-
* ->dl_server.
5867-
*/
5868-
if (prev->dl_server)
5869-
prev->dl_server = NULL;
58705860
}
58715861

58725862
/*
@@ -5878,6 +5868,8 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
58785868
const struct sched_class *class;
58795869
struct task_struct *p;
58805870

5871+
rq->dl_server = NULL;
5872+
58815873
/*
58825874
* Optimization: we know that if all tasks are in the fair class we can
58835875
* 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 task_struct *prev, struct rq_flags *rf)
58975889
put_prev_set_next_task(rq, prev, p);
58985890
}
58995891

5900-
/*
5901-
* This is a normal CFS pick, but the previous could be a DL pick.
5902-
* Clear it as previous is no longer picked.
5903-
*/
5904-
if (prev->dl_server)
5905-
prev->dl_server = NULL;
5906-
5907-
/*
5908-
* This is the fast path; it cannot be a DL server pick;
5909-
* therefore even if @p == @prev, ->dl_server must be NULL.
5910-
*/
5911-
if (p->dl_server)
5912-
p->dl_server = NULL;
5913-
59145892
return p;
59155893
}
59165894

@@ -5958,6 +5936,8 @@ static inline struct task_struct *pick_task(struct rq *rq)
59585936
const struct sched_class *class;
59595937
struct task_struct *p;
59605938

5939+
rq->dl_server = NULL;
5940+
59615941
for_each_class(class) {
59625942
p = class->pick_task(rq);
59635943
if (p)
@@ -5996,6 +5976,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
59965976
* another cpu during offline.
59975977
*/
59985978
rq->core_pick = NULL;
5979+
rq->core_dl_server = NULL;
59995980
return __pick_next_task(rq, prev, rf);
60005981
}
60015982

@@ -6014,7 +5995,9 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
60145995
WRITE_ONCE(rq->core_sched_seq, rq->core->core_pick_seq);
60155996

60165997
next = rq->core_pick;
5998+
rq->dl_server = rq->core_dl_server;
60175999
rq->core_pick = NULL;
6000+
rq->core_dl_server = NULL;
60186001
goto out_set_next;
60196002
}
60206003

@@ -6059,6 +6042,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
60596042
next = pick_task(rq);
60606043
if (!next->core_cookie) {
60616044
rq->core_pick = NULL;
6045+
rq->core_dl_server = NULL;
60626046
/*
60636047
* For robustness, update the min_vruntime_fi for
60646048
* unconstrained picks as well.
@@ -6086,7 +6070,9 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
60866070
if (i != cpu && (rq_i != rq->core || !core_clock_updated))
60876071
update_rq_clock(rq_i);
60886072

6089-
p = rq_i->core_pick = pick_task(rq_i);
6073+
rq_i->core_pick = p = pick_task(rq_i);
6074+
rq_i->core_dl_server = rq_i->dl_server;
6075+
60906076
if (!max || prio_less(max, p, fi_before))
60916077
max = p;
60926078
}
@@ -6110,6 +6096,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
61106096
}
61116097

61126098
rq_i->core_pick = p;
6099+
rq_i->core_dl_server = NULL;
61136100

61146101
if (p == rq_i->idle) {
61156102
if (rq_i->nr_running) {
@@ -6170,6 +6157,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
61706157

61716158
if (i == cpu) {
61726159
rq_i->core_pick = NULL;
6160+
rq_i->core_dl_server = NULL;
61736161
continue;
61746162
}
61756163

@@ -6178,6 +6166,7 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
61786166

61796167
if (rq_i->curr == rq_i->core_pick) {
61806168
rq_i->core_pick = NULL;
6169+
rq_i->core_dl_server = NULL;
61816170
continue;
61826171
}
61836172

@@ -8401,6 +8390,7 @@ void __init sched_init(void)
84018390
#ifdef CONFIG_SCHED_CORE
84028391
rq->core = rq;
84038392
rq->core_pick = NULL;
8393+
rq->core_dl_server = NULL;
84048394
rq->core_enabled = 0;
84058395
rq->core_tree = RB_ROOT;
84068396
rq->core_forceidle_count = 0;

kernel/sched/deadline.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2423,7 +2423,7 @@ static struct task_struct *__pick_task_dl(struct rq *rq)
24232423
update_curr_dl_se(rq, dl_se, 0);
24242424
goto again;
24252425
}
2426-
p->dl_server = dl_se;
2426+
rq->dl_server = dl_se;
24272427
} else {
24282428
p = dl_task_of(dl_se);
24292429
}

kernel/sched/fair.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8749,14 +8749,6 @@ static struct task_struct *pick_task_fair(struct rq *rq)
87498749
cfs_rq = group_cfs_rq(se);
87508750
} while (cfs_rq);
87518751

8752-
/*
8753-
* This can be called from directly from CFS's ->pick_task() or indirectly
8754-
* from DL's ->pick_task when fair server is enabled. In the indirect case,
8755-
* DL will set ->dl_server just after this function is called, so its Ok to
8756-
* clear. In the direct case, we are picking directly so we must clear it.
8757-
*/
8758-
task_of(se)->dl_server = NULL;
8759-
87608752
return task_of(se);
87618753
}
87628754

@@ -8780,6 +8772,8 @@ pick_next_task_fair(struct rq *rq, struct task_struct *prev, struct rq_flags *rf
87808772
if (prev->sched_class != &fair_sched_class)
87818773
goto simple;
87828774

8775+
__put_prev_set_next_dl_server(rq, prev, p);
8776+
87838777
/*
87848778
* Because of the set_next_buddy() in dequeue_task_fair() it is rather
87858779
* likely that a next task is from the same cgroup as the current.

kernel/sched/sched.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,7 @@ struct rq {
10661066
unsigned int nr_uninterruptible;
10671067

10681068
struct task_struct __rcu *curr;
1069+
struct sched_dl_entity *dl_server;
10691070
struct task_struct *idle;
10701071
struct task_struct *stop;
10711072
unsigned long next_balance;
@@ -1193,6 +1194,7 @@ struct rq {
11931194
/* per rq */
11941195
struct rq *core;
11951196
struct task_struct *core_pick;
1197+
struct sched_dl_entity *core_dl_server;
11961198
unsigned int core_enabled;
11971199
unsigned int core_sched_seq;
11981200
struct rb_root core_tree;
@@ -2370,12 +2372,24 @@ static inline void set_next_task(struct rq *rq, struct task_struct *next)
23702372
next->sched_class->set_next_task(rq, next, false);
23712373
}
23722374

2375+
static inline void
2376+
__put_prev_set_next_dl_server(struct rq *rq,
2377+
struct task_struct *prev,
2378+
struct task_struct *next)
2379+
{
2380+
prev->dl_server = NULL;
2381+
next->dl_server = rq->dl_server;
2382+
rq->dl_server = NULL;
2383+
}
2384+
23732385
static inline void put_prev_set_next_task(struct rq *rq,
23742386
struct task_struct *prev,
23752387
struct task_struct *next)
23762388
{
23772389
WARN_ON_ONCE(rq->curr != prev);
23782390

2391+
__put_prev_set_next_dl_server(rq, prev, next);
2392+
23792393
if (next == prev)
23802394
return;
23812395

0 commit comments

Comments
 (0)