lkml.org 
[lkml]   [2021]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] sched/fair: Fix detection of per-CPU kthreads waking a task
    Date
    On 26/11/21 15:40, Vincent Guittot wrote:
    > On Fri, 26 Nov 2021 at 14:32, Valentin Schneider
    > <Valentin.Schneider@arm.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;
    >>

    \
     
     \ /
      Last update: 2021-11-26 17:53    [W:4.329 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site