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: <20230328092622.062917921@infradead.org> <20230328110354.141543852@infradead.org>
 <CABk29Nt4T67S+L9Qs1qeOUyo5gY1Qy5KuOwuCYNM74E58J81Eg@mail.gmail.com>
 <20230329080646.GL4253@hirez.programming.kicks-ass.net> <20230329082235.GA38236@hirez.programming.kicks-ass.net>
In-Reply-To: <20230329082235.GA38236@hirez.programming.kicks-ass.net>
From:   Josh Don <joshdon@google.com>
Date:   Wed, 29 Mar 2023 11:48:41 -0700
Message-ID: <CABk29NstO+-=T-91fc7rNRYNSgP5=+SGvGEFxejrS7qPsMOKbQ@mail.gmail.com>
Subject: Re: [PATCH 08/17] sched/fair: Implement an EEVDF like policy
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     mingo@kernel.org, vincent.guittot@linaro.org,
        linux-kernel@vger.kernel.org, juri.lelli@redhat.com,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        mgorman@suse.de, bristot@redhat.com, corbet@lwn.net,
        qyousef@layalina.io, chris.hyser@oracle.com,
        patrick.bellasi@matbug.net, pjt@google.com, pavel@ucw.cz,
        qperret@google.com, tim.c.chen@linux.intel.com, timj@gnu.org,
        kprateek.nayak@amd.com, yu.c.chen@intel.com,
        youssefesmat@chromium.org, joel@joelfernandes.org, efault@gmx.de
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Mar 29, 2023 at 1:22=E2=80=AFAM Peter Zijlstra <peterz@infradead.or=
g> wrote:
>
> On Wed, Mar 29, 2023 at 10:06:46AM +0200, Peter Zijlstra wrote:
> > On Tue, Mar 28, 2023 at 06:26:51PM -0700, Josh Don wrote:
> > > > +static struct sched_entity *pick_eevdf(struct cfs_rq *cfs_rq)
> > > > +{
> > > > +       struct rb_node *node =3D cfs_rq->tasks_timeline.rb_root.rb_=
node;
> > > > +       struct sched_entity *curr =3D cfs_rq->curr;
> > > > +       struct sched_entity *best =3D NULL;
> > > > +
> > > > +       if (curr && (!curr->on_rq || !entity_eligible(cfs_rq, curr)=
))
> > > > +               curr =3D NULL;
> > > > +
> > > > +       while (node) {
> > > > +               struct sched_entity *se =3D __node_2_se(node);
> > > > +
> > > > +               /*
> > > > +                * If this entity is not eligible, try the left sub=
tree.
> > > > +                */
> > > > +               if (!entity_eligible(cfs_rq, se)) {
> > > > +                       node =3D node->rb_left;
> > > > +                       continue;
> > > > +               }
> > > > +
> > > > +               /*
> > > > +                * If this entity has an earlier deadline than the =
previous
> > > > +                * best, take this one. If it also has the earliest=
 deadline
> > > > +                * of its subtree, we're done.
> > > > +                */
> > > > +               if (!best || deadline_gt(deadline, best, se)) {
> > > > +                       best =3D se;
> > > > +                       if (best->deadline =3D=3D best->min_deadlin=
e)
> > > > +                               break;
> > >
> > > Isn't it possible to have a child with less vruntime (ie. rb->left)
> > > but with the same deadline? Wouldn't it be preferable to choose the
> > > child instead since the deadlines are equivalent but the child has
> > > received less service time?
> >
> > Possible, yes I suppose. But given this is ns granular virtual time,
> > somewhat unlikely. You can modify the last (validation) patch and have
> > it detect the case, see if you can trigger it.

Agreed on unlikely, was just checking my understanding here, since
this becomes a question of tradeoff (likelihood of decent vs the ideal
scheduling decision). Leaving as-is seems fine, with potentially a
short comment.

> > Doing that will make the pick always do a full decent of the tree
> > through, which is a little more expensive. Not sure it's worth the
> > effort.
>
> Hmm, maybe not, if there is no smaller-or-equal deadline then the
> min_deadline of the child will be greater and we can terminate the
> decent right there.
>