On 26-Apr-2019 05:03:37 PM, Peter Zijlstra wrote:
> On Tue, Apr 23, 2019 at 04:18:21PM +0000, Vineeth Remanan Pillai wrote:
>
> (you lost From: Julien)
>
> > During core scheduling, it can happen that the current rq selects a
> > non-tagged process while the sibling might be idling even though it
> > had something to run (because the sibling selected idle to match the
> > tagged process in previous tag matching iteration). We need to wake up
> > the sibling if such a situation arise.
> >
> > Signed-off-by: Vineeth Remanan Pillai <vpil...@digitalocean.com>
> > Signed-off-by: Julien Desfossez <jdesfos...@digitalocean.com>
> > ---
> > kernel/sched/core.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index e8f5ec641d0a..0e3c51a1b54a 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3775,6 +3775,21 @@ pick_next_task(struct rq *rq, struct task_struct
> > *prev, struct rq_flags *rf)
> > */
> > if (i == cpu && !rq->core->core_cookie &&
> > !p->core_cookie) {
> > next = p;
> > + rq->core_pick = NULL;
> > +
> > + /*
> > + * If the sibling is idling, we might want to
> > wake it
> > + * so that it can check for any runnable tasks
> > that did
> > + * not get a chance to run due to previous task
> > matching.
> > + */
> > + for_each_cpu(j, smt_mask) {
> > + struct rq *rq_j = cpu_rq(j);
> > + rq_j->core_pick = NULL;
> > + if (j != cpu &&
> > + is_idle_task(rq_j->curr) &&
> > rq_j->nr_running) {
> > + resched_curr(rq_j);
> > + }
> > + }
> > goto done;
> > }
>
> Anyway, as written here:
>
>
> https://lkml.kernel.org/r/20190410150116.gi2...@worktop.programming.kicks-ass.net
>
> I think this isn't quite right. Does the below patch (which actually
> removes lines) also work?
>
> As written before; the intent was to not allow that optimization if the
> last pick had a cookie; thereby doing a (last) core wide selection when
> we go to a 0-cookie, and this then includes kicking forced-idle cores.
It works and the performance is similar to our previous solution :-)
Thanks,
Julien
