Skip to content

Commit 9c2791f

Browse files
vingu-linaroIngo Molnar
authored andcommitted
sched/fair: Fix hierarchical order in rq->leaf_cfs_rq_list
Fix the insertion of cfs_rq in rq->leaf_cfs_rq_list to ensure that a child will always be called before its parent. The hierarchical order in shares update list has been introduced by commit: 67e8625 ("sched: Introduce hierarchal order on shares update list") With the current implementation a child can be still put after its parent. Lets take the example of: root \ b /\ c d* | e* with root -> b -> c already enqueued but not d -> e so the leaf_cfs_rq_list looks like: head -> c -> b -> root -> tail The branch d -> e will be added the first time that they are enqueued, starting with e then d. When e is added, its parents is not already on the list so e is put at the tail : head -> c -> b -> root -> e -> tail Then, d is added at the head because its parent is already on the list: head -> d -> c -> b -> root -> e -> tail e is not placed at the right position and will be called the last whereas it should be called at the beginning. Because it follows the bottom-up enqueue sequence, we are sure that we will finished to add either a cfs_rq without parent or a cfs_rq with a parent that is already on the list. We can use this event to detect when we have finished to add a new branch. For the others, whose parents are not already added, we have to ensure that they will be added after their children that have just been inserted the steps before, and after any potential parents that are already in the list. The easiest way is to put the cfs_rq just after the last inserted one and to keep track of it untl the branch is fully added. Signed-off-by: Vincent Guittot <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Acked-by: Dietmar Eggemann <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: [email protected] Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Ingo Molnar <[email protected]>
1 parent df21791 commit 9c2791f

File tree

3 files changed

+49
-7
lines changed

3 files changed

+49
-7
lines changed

kernel/sched/core.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7604,6 +7604,7 @@ void __init sched_init(void)
76047604
#ifdef CONFIG_FAIR_GROUP_SCHED
76057605
root_task_group.shares = ROOT_TASK_GROUP_LOAD;
76067606
INIT_LIST_HEAD(&rq->leaf_cfs_rq_list);
7607+
rq->tmp_alone_branch = &rq->leaf_cfs_rq_list;
76077608
/*
76087609
* How much cpu bandwidth does root_task_group get?
76097610
*

kernel/sched/fair.c

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -283,19 +283,59 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp)
283283
static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
284284
{
285285
if (!cfs_rq->on_list) {
286+
struct rq *rq = rq_of(cfs_rq);
287+
int cpu = cpu_of(rq);
286288
/*
287289
* Ensure we either appear before our parent (if already
288290
* enqueued) or force our parent to appear after us when it is
289-
* enqueued. The fact that we always enqueue bottom-up
290-
* reduces this to two cases.
291+
* enqueued. The fact that we always enqueue bottom-up
292+
* reduces this to two cases and a special case for the root
293+
* cfs_rq. Furthermore, it also means that we will always reset
294+
* tmp_alone_branch either when the branch is connected
295+
* to a tree or when we reach the beg of the tree
291296
*/
292297
if (cfs_rq->tg->parent &&
293-
cfs_rq->tg->parent->cfs_rq[cpu_of(rq_of(cfs_rq))]->on_list) {
294-
list_add_rcu(&cfs_rq->leaf_cfs_rq_list,
295-
&rq_of(cfs_rq)->leaf_cfs_rq_list);
296-
} else {
298+
cfs_rq->tg->parent->cfs_rq[cpu]->on_list) {
299+
/*
300+
* If parent is already on the list, we add the child
301+
* just before. Thanks to circular linked property of
302+
* the list, this means to put the child at the tail
303+
* of the list that starts by parent.
304+
*/
305+
list_add_tail_rcu(&cfs_rq->leaf_cfs_rq_list,
306+
&(cfs_rq->tg->parent->cfs_rq[cpu]->leaf_cfs_rq_list));
307+
/*
308+
* The branch is now connected to its tree so we can
309+
* reset tmp_alone_branch to the beginning of the
310+
* list.
311+
*/
312+
rq->tmp_alone_branch = &rq->leaf_cfs_rq_list;
313+
} else if (!cfs_rq->tg->parent) {
314+
/*
315+
* cfs rq without parent should be put
316+
* at the tail of the list.
317+
*/
297318
list_add_tail_rcu(&cfs_rq->leaf_cfs_rq_list,
298-
&rq_of(cfs_rq)->leaf_cfs_rq_list);
319+
&rq->leaf_cfs_rq_list);
320+
/*
321+
* We have reach the beg of a tree so we can reset
322+
* tmp_alone_branch to the beginning of the list.
323+
*/
324+
rq->tmp_alone_branch = &rq->leaf_cfs_rq_list;
325+
} else {
326+
/*
327+
* The parent has not already been added so we want to
328+
* make sure that it will be put after us.
329+
* tmp_alone_branch points to the beg of the branch
330+
* where we will add parent.
331+
*/
332+
list_add_rcu(&cfs_rq->leaf_cfs_rq_list,
333+
rq->tmp_alone_branch);
334+
/*
335+
* update tmp_alone_branch to points to the new beg
336+
* of the branch
337+
*/
338+
rq->tmp_alone_branch = &cfs_rq->leaf_cfs_rq_list;
299339
}
300340

301341
cfs_rq->on_list = 1;

kernel/sched/sched.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,7 @@ struct rq {
623623
#ifdef CONFIG_FAIR_GROUP_SCHED
624624
/* list of leaf cfs_rq on this cpu: */
625625
struct list_head leaf_cfs_rq_list;
626+
struct list_head *tmp_alone_branch;
626627
#endif /* CONFIG_FAIR_GROUP_SCHED */
627628

628629
/*

0 commit comments

Comments
 (0)