linux-kernel - Re: [PATCH 27/30] sched_ext: Implement core-sched support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABk29NuwpSDAy6inXD5dPjtw9SqjxNr0hK5SkM98b7jHinWFFw@mail.gmail.com>
Date:   Mon, 30 Jan 2023 17:45:26 -0800
From:   Josh Don <joshdon@...gle.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     torvalds@...ux-foundation.org, mingo@...hat.com,
        peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, vschneid@...hat.com, ast@...nel.org,
        daniel@...earbox.net, andrii@...nel.org, martin.lau@...nel.org,
        brho@...gle.com, pjt@...gle.com, derkling@...gle.com,
        haoluo@...gle.com, dvernet@...a.com, dschatzberg@...a.com,
        dskarlat@...cmu.edu, riel@...riel.com,
        linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        kernel-team@...a.com
Subject: Re: [PATCH 27/30] sched_ext: Implement core-sched support

On Mon, Jan 30, 2023 at 4:26 PM Tejun Heo <tj@...nel.org> wrote:
>
> Hello,
>
> On Mon, Jan 30, 2023 at 01:38:15PM -0800, Josh Don wrote:
> > > The core-sched support is composed of the following parts:
> >
> > Thanks, this looks pretty reasonable overall.
> >
> > One meta comment is that I think we can shortcircuit from
> > touch_core_sched when we have sched_core_disabled().
>
> Yeah, touch_core_sched() is really cheap (it's just an assignment from an rq
> field to a task field) but sched_core_disabled() is also just a static
> branch. Will update.

Yep, true, I was just going through and reasoning about whether
anything needed to be done in the !sched_core_disabled() case.

> > Reviewed-by: Josh Don <joshdon@...gle.com>
> >
> > > +                       /*
> > > +                        * While core-scheduling, rq lock is shared among
> > > +                        * siblings but the debug annotations and rq clock
> > > +                        * aren't. Do pinning dance to transfer the ownership.
> > > +                        */
> > > +                       WARN_ON_ONCE(__rq_lockp(rq) != __rq_lockp(srq));
> > > +                       rq_unpin_lock(rq, rf);
> > > +                       rq_pin_lock(srq, &srf);
> > > +
> > > +                       update_rq_clock(srq);
> >
> > Unfortunate that we have to do this superfluous update; maybe we can
> > save/restore the clock flags from before the pinning shenanigans?
>
> So, this one isn't really superflous. There are two rq's involved - self and
> sibling. self's rq clock is saved and restored through rq_unpin_lock() and
> rq_repin_lock(). We're transferring the lock owner ship from self to sibling
> without actually unlocking and relocking the lock as they should be sharing
> the same lock; however, that doesn't mean that the two queues share rq
> clocks, so the sibling needs to update its rq clock upon getting the lock
> transferred to it. It might make sense to make the siblings share the rq
> clock when core-sched is enabled but that's probably for some other time.

Yep, whoops, I forgot that part didn't make it.