Commit 2d935df7 authored by Vincent Guittot's avatar Vincent Guittot Committed by Greg Kroah-Hartman

sched/fair: Fix insertion in rq->leaf_cfs_rq_list

commit f6783319 upstream.

Sargun reported a crash:

  "I picked up c40f7d74 sched/fair: Fix
   infinite loop in update_blocked_averages() by reverting a9e7f654
   and put it on top of 4.19.13. In addition to this, I uninlined
   list_add_leaf_cfs_rq for debugging.

   This revealed a new bug that we didn't get to because we kept getting
   crashes from the previous issue. When we are running with cgroups that
   are rapidly changing, with CFS bandwidth control, and in addition
   using the cpusets cgroup, we see this crash. Specifically, it seems to
   occur with cgroups that are throttled and we change the allowed
   cpuset."

The algorithm used to order cfs_rq in rq->leaf_cfs_rq_list assumes that
it will walk down to root the 1st time a cfs_rq is used and we will finish
to add either a cfs_rq without parent or a cfs_rq with a parent that is
already on the list. But this is not always true in presence of throttling.
Because a cfs_rq can be throttled even if it has never been used but other CPUs
of the cgroup have already used all the bandwdith, we are not sure to go down to
the root and add all cfs_rq in the list.

Ensure that all cfs_rq will be added in the list even if they are throttled.

[ mingo: Fix !CGROUPS build. ]
Reported-by: default avatarSargun Dhillon <sargun@sargun.me>
Signed-off-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: tj@kernel.org
Fixes: 9c2791f9 ("Fix hierarchical order in rq->leaf_cfs_rq_list")
Link: https://lkml.kernel.org/r/1548825767-10799-1-git-send-email-vincent.guittot@linaro.orgSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
Cc: Janne Huttunen <janne.huttunen@nokia.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 6c11530e
...@@ -282,13 +282,13 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp) ...@@ -282,13 +282,13 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp)
return grp->my_q; return grp->my_q;
} }
static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) static inline bool list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
{ {
struct rq *rq = rq_of(cfs_rq); struct rq *rq = rq_of(cfs_rq);
int cpu = cpu_of(rq); int cpu = cpu_of(rq);
if (cfs_rq->on_list) if (cfs_rq->on_list)
return; return rq->tmp_alone_branch == &rq->leaf_cfs_rq_list;
cfs_rq->on_list = 1; cfs_rq->on_list = 1;
...@@ -317,7 +317,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) ...@@ -317,7 +317,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
* list. * list.
*/ */
rq->tmp_alone_branch = &rq->leaf_cfs_rq_list; rq->tmp_alone_branch = &rq->leaf_cfs_rq_list;
return; return true;
} }
if (!cfs_rq->tg->parent) { if (!cfs_rq->tg->parent) {
...@@ -332,7 +332,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) ...@@ -332,7 +332,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
* tmp_alone_branch to the beginning of the list. * tmp_alone_branch to the beginning of the list.
*/ */
rq->tmp_alone_branch = &rq->leaf_cfs_rq_list; rq->tmp_alone_branch = &rq->leaf_cfs_rq_list;
return; return true;
} }
/* /*
...@@ -347,6 +347,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) ...@@ -347,6 +347,7 @@ static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
* of the branch * of the branch
*/ */
rq->tmp_alone_branch = &cfs_rq->leaf_cfs_rq_list; rq->tmp_alone_branch = &cfs_rq->leaf_cfs_rq_list;
return false;
} }
static inline void list_del_leaf_cfs_rq(struct cfs_rq *cfs_rq) static inline void list_del_leaf_cfs_rq(struct cfs_rq *cfs_rq)
...@@ -448,8 +449,9 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp) ...@@ -448,8 +449,9 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp)
return NULL; return NULL;
} }
static inline void list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq) static inline bool list_add_leaf_cfs_rq(struct cfs_rq *cfs_rq)
{ {
return true;
} }
static inline void list_del_leaf_cfs_rq(struct cfs_rq *cfs_rq) static inline void list_del_leaf_cfs_rq(struct cfs_rq *cfs_rq)
...@@ -5019,6 +5021,12 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq) ...@@ -5019,6 +5021,12 @@ static void __maybe_unused unthrottle_offline_cfs_rqs(struct rq *rq)
} }
#else /* CONFIG_CFS_BANDWIDTH */ #else /* CONFIG_CFS_BANDWIDTH */
static inline bool cfs_bandwidth_used(void)
{
return false;
}
static inline u64 cfs_rq_clock_task(struct cfs_rq *cfs_rq) static inline u64 cfs_rq_clock_task(struct cfs_rq *cfs_rq)
{ {
return rq_clock_task(rq_of(cfs_rq)); return rq_clock_task(rq_of(cfs_rq));
...@@ -5174,6 +5182,21 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags) ...@@ -5174,6 +5182,21 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
if (!se) if (!se)
add_nr_running(rq, 1); add_nr_running(rq, 1);
if (cfs_bandwidth_used()) {
/*
* When bandwidth control is enabled; the cfs_rq_throttled()
* breaks in the above iteration can result in incomplete
* leaf list maintenance, resulting in triggering the assertion
* below.
*/
for_each_sched_entity(se) {
cfs_rq = cfs_rq_of(se);
if (list_add_leaf_cfs_rq(cfs_rq))
break;
}
}
assert_list_leaf_cfs_rq(rq); assert_list_leaf_cfs_rq(rq);
hrtick_update(rq); hrtick_update(rq);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment