Commit 2feab249 authored by Josh Don's avatar Josh Don Committed by Peter Zijlstra

Revert "sched/fair: Make sure to try to detach at least one movable task"

This reverts commit b0defa7a.

b0defa7a changed the load balancing logic to ignore env.max_loop if
all tasks examined to that point were pinned. The goal of the patch was
to make it more likely to be able to detach a task buried in a long list
of pinned tasks. However, this has the unfortunate side effect of
creating an O(n) iteration in detach_tasks(), as we now must fully
iterate every task on a cpu if all or most are pinned. Since this load
balance code is done with rq lock held, and often in softirq context, it
is very easy to trigger hard lockups. We observed such hard lockups with
a user who affined O(10k) threads to a single cpu.

When I discussed this with Vincent he initially suggested that we keep
the limit on the number of tasks to detach, but increase the number of
tasks we can search. However, after some back and forth on the mailing
list, he recommended we instead revert the original patch, as it seems
likely no one was actually getting hit by the original issue.

Fixes: b0defa7a ("sched/fair: Make sure to try to detach at least one movable task")
Signed-off-by: default avatarJosh Don <joshdon@google.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240620214450.316280-1-joshdon@google.com
parent f2661062
...@@ -9149,12 +9149,8 @@ static int detach_tasks(struct lb_env *env) ...@@ -9149,12 +9149,8 @@ static int detach_tasks(struct lb_env *env)
break; break;
env->loop++; env->loop++;
/* /* We've more or less seen every task there is, call it quits */
* We've more or less seen every task there is, call it quits if (env->loop > env->loop_max)
* unless we haven't found any movable task yet.
*/
if (env->loop > env->loop_max &&
!(env->flags & LBF_ALL_PINNED))
break; break;
/* take a breather every nr_migrate tasks */ /* take a breather every nr_migrate tasks */
...@@ -11393,8 +11389,6 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq, ...@@ -11393,8 +11389,6 @@ static int sched_balance_rq(int this_cpu, struct rq *this_rq,
if (env.flags & LBF_NEED_BREAK) { if (env.flags & LBF_NEED_BREAK) {
env.flags &= ~LBF_NEED_BREAK; env.flags &= ~LBF_NEED_BREAK;
/* Stop if we tried all running tasks */
if (env.loop < busiest->nr_running)
goto more_balance; goto more_balance;
} }
......
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