[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87pmqmc16f.mognet@arm.com>
Date: Fri, 26 Nov 2021 16:49:12 +0000
From: Valentin Schneider <Valentin.Schneider@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: Vincent Donnefort <Vincent.Donnefort@....com>,
peterz@...radead.org, mingo@...hat.com,
linux-kernel@...r.kernel.org, mgorman@...hsingularity.net,
dietmar.eggemann@....com
Subject: Re: [PATCH] sched/fair: Fix detection of per-CPU kthreads waking a task
On 26/11/21 15:40, Vincent Guittot wrote:
> On Fri, 26 Nov 2021 at 14:32, Valentin Schneider
> <Valentin.Schneider@....com> wrote:
>> /*
>> - * Allow a per-cpu kthread to stack with the wakee if the
>> - * kworker thread and the tasks previous CPUs are the same.
>> - * The assumption is that the wakee queued work for the
>> - * per-cpu kthread that is now complete and the wakeup is
>> - * essentially a sync wakeup. An obvious example of this
>> + * Allow a per-cpu kthread to stack with the wakee if the kworker thread
>> + * and the tasks previous CPUs are the same. The assumption is that the
>> + * wakee queued work for the per-cpu kthread that is now complete and
>> + * the wakeup is essentially a sync wakeup. An obvious example of this
>> * pattern is IO completions.
>> + *
>> + * Ensure the wakeup is issued by the kthread itself, and don't match
>> + * against the idle task because that could override the
>> + * available_idle_cpu(target) check done higher up.
>> */
>> - if (is_per_cpu_kthread(current) &&
>> + if (is_per_cpu_kthread(current) && !is_idle_task(current) &&
>
> still i don't see the need of !is_idle_task(current)
>
Admittedly, belts and braces. The existing condition checks rq->nr_running <= 1
which can lead to coscheduling when the wakeup is issued by the idle task
(or even if rq->nr_running == 0, you can have rq->ttwu_pending without
having sent an IPI due to polling). Essentially this overrides the first
check in sis() that uses idle_cpu(target) (prev == smp_processor_id() ==
target).
I couldn't prove such wakeups can happen right now, but if/when they do
(AIUI it would just take someone to add a wake_up_process() down some
smp_call_function() callback) then we'll need the above. If you're still
not convinced by now, I won't push it further.
>
>> + in_task() &&
>> prev == smp_processor_id() &&
>> this_rq()->nr_running <= 1) {
>> return prev;
>>
Powered by blists - more mailing lists