[lkml]   [2017]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] sched/rt: Do not pull from current CPU if only one cpu to pull
Hi Steve,

On 02/12/17 13:04, Steven Rostedt wrote:
> Daniel Wagner reported a crash on the beaglebone black. This is a
> single CPU architecture, and does not have a functional:
> arch_send_call_function_single_ipi() and can crash if that is called.
> As it only has one CPU, it shouldn't be called, but if the kernel is
> compiled for SMP, the push/pull RT scheduling logic now calls it for
> irq_work if the one CPU is overloaded, it can use that function to call
> itself and crash the kernel.
> Ideally, we should disable the SCHED_FEAT(RT_PUSH_IPI) if the system
> only has a single CPU. But SCHED_FEAT is a constant if sched debugging
> is turned off. Another fix can also be used, and this should also help
> with normal SMP machines. That is, do not initiate the pull code if
> there's only one RT overloaded CPU, and that CPU happens to be the
> current CPU that is scheduling in a lower priority task.
> Even on a system with many CPUs, if there's many RT tasks waiting to
> run on a single CPU, and that CPU schedules in another RT task of lower
> priority, it will initiate the PULL logic in case there's a higher
> priority RT task on another CPU that is waiting to run. But if there is
> no other CPU with waiting RT tasks, it will initiate the RT pull logic
> on itself (as it still has RT tasks waiting to run). This is a wasted
> effort.
> Not only does this help with SMP code where the current CPU is the only
> one with RT overloaded tasks, it should also solve the issue that
> Daniel encountered, because it will prevent the PULL logic from
> executing, as there's only one CPU on the system, and the check added
> here will cause it to exit the RT pull code.
> Link:
> Fixes: 4bdced5c9 ("sched/rt: Simplify the IPI based RT balancing logic")
> Cc:
> Reported-by: Daniel Wagner <>
> ---
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 4056c19ca3f0..665ace2fc558 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -2034,8 +2034,9 @@ static void pull_rt_task(struct rq *this_rq)
> bool resched = false;
> struct task_struct *p;
> struct rq *src_rq;
> + int rt_overload_count = rt_overloaded(this_rq);
> - if (likely(!rt_overloaded(this_rq)))
> + if (likely(!rt_overload_count))
> return;
> /*
> @@ -2044,6 +2045,11 @@ static void pull_rt_task(struct rq *this_rq)
> */
> smp_rmb();
> + /* If we are the only overloaded CPU do nothing */
> + if (rt_overload_count == 1 &&
> + cpumask_test_cpu(this_rq->cpu, this_rq->rd->rto_mask))
> + return;
> +

Right. I was wondering however if for the truly UP case we shouldn't be
initiating/queueing callbacks (pull/push) at all?

DEADLINE doesn't use (yet?) the PUSH_IPI, but we will need a similar
patch to keep logics aligned.



 \ /
  Last update: 2017-12-04 08:46    [W:0.077 / U:14.048 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site