Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220311161406.23497-1-vincent.guittot@linaro.org>
 <20220311161406.23497-6-vincent.guittot@linaro.org> <CABk29Numf595wF6PiLd673TmFt6AV9qTEsWP7tSf2xPOKcGgLQ@mail.gmail.com>
In-Reply-To: <CABk29Numf595wF6PiLd673TmFt6AV9qTEsWP7tSf2xPOKcGgLQ@mail.gmail.com>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Tue, 15 Mar 2022 14:54:51 +0100
Message-ID: <CAKfTPtD0kFuyh+Q3po0UHR3GQz1uKGxRrFWDfsHcCuQr4ZWbNQ@mail.gmail.com>
Subject: Re: [RFC 5/6] sched/fair: Take into account latency nice at wakeup
To:     Josh Don <joshdon@google.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Benjamin Segall <bsegall@google.com>,
        Mel Gorman <mgorman@suse.de>,
        linux-kernel <linux-kernel@vger.kernel.org>, parth@linux.ibm.com,
        Qais Yousef <qais.yousef@arm.com>,
        "Hyser,Chris" <chris.hyser@oracle.com>,
        Pavan Kondeti <pkondeti@codeaurora.org>,
        Valentin Schneider <Valentin.Schneider@arm.com>,
        patrick.bellasi@matbug.net, David.Laight@aculab.com,
        Paul Turner <pjt@google.com>, pavel@ucw.cz,
        Tejun Heo <tj@kernel.org>,
        Dhaval Giani <dhaval.giani@oracle.com>, qperret@google.com,
        Tim Chen <tim.c.chen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 15 Mar 2022 at 01:53, Josh Don <joshdon@google.com> wrote:
>
> Hi Vincent,
>
> On Fri, Mar 11, 2022 at 8:21 AM Vincent Guittot
> <vincent.guittot@linaro.org> wrote:
> >
> [snip]
> > +
> > +static void check_preempt_from_idle(struct cfs_rq *cfs, struct sched_entity *se)
> > +{
> > +       struct sched_entity *next;
> > +
> > +       if (se->latency_weight <= 0)
> > +               return;
> > +
> > +       if (cfs->nr_running <= 1)
> > +               return;
>
> I don't quite understand this nr_running check.

just to return early if there is no other task running on the cfs

>
> > +       /*
> > +        * When waking from idle, we don't need to check to preempt at wakeup
> > +        * the idle thread and don't set next buddy as a candidate for being
> > +        * picked in priority.
> > +        * In case of simultaneous wakeup from idle, the latency sensitive tasks
> > +        * lost opportunity to preempt non sensitive tasks which woke up
> > +        * simultaneously.
> > +        */
> > +
> > +       if (cfs->next)
> > +               next = cfs->next;
> > +       else
> > +               next = __pick_first_entity(cfs);
> > +
> > +       if (next && wakeup_preempt_entity(next, se) == 1)
> > +               set_next_buddy(se);
> > +}
> > +
>
> What's the motivation to do this with the next buddy vs using wakeup
> entity placement to achieve a similar result? The latter would also

I don't want to modify vruntime because of latency prio because it
would mean impacting the cpu bandwidth of the task which is managed
with nice priority. latency nice is only  about preempting current
running task

> more generically work when we aren't transitioning from idle. It also
> doesn't suffer from some slight weirdness here in the interaction with
> core scheduling, where rq->curr can be idle despite the presence of
> runnable tasks if the cpu is forced idle.

TBH, I haven't looked in details the core scheduling part which adds
another level of complexity when selecting which task should run on
the core