linux-kernel - Re: [RFD/RFC PATCH 5/8] sched: Add proxy execution

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]

Date:   Thu, 11 Oct 2018 14:53:25 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Juri Lelli <juri.lelli@...hat.com>
Cc:     luca abeni <luca.abeni@...tannapisa.it>, mingo@...hat.com,
        rostedt@...dmis.org, tglx@...utronix.de,
        linux-kernel@...r.kernel.org, claudio@...dence.eu.com,
        tommaso.cucinotta@...tannapisa.it, alessio.balsini@...il.com,
        bristot@...hat.com, will.deacon@....com,
        andrea.parri@...rulasolutions.com, dietmar.eggemann@....com,
        patrick.bellasi@....com, henrik@...tad.us,
        linux-rt-users@...r.kernel.org
Subject: Re: [RFD/RFC PATCH 5/8] sched: Add proxy execution

On Thu, Oct 11, 2018 at 02:34:48PM +0200, Juri Lelli wrote:
> Hi Luca,
> 
> On 10/10/18 13:10, luca abeni wrote:
> > Hi,
> > 
> > On Tue,  9 Oct 2018 11:24:31 +0200
> > Juri Lelli <juri.lelli@...hat.com> wrote:
> > [...]
> > > +migrate_task:
> > [...]
> > > +	put_prev_task(rq, next);
> > > +	if (rq->curr != rq->idle) {
> > > +		rq->proxy = rq->idle;
> > > +		set_tsk_need_resched(rq->idle);
> > > +		/*
> > > +		 * XXX [juril] don't we still need to migrate @next
> > > to
> > > +		 * @owner's CPU?
> > > +		 */
> > > +		return rq->idle;
> > > +	}
> > 
> > If I understand well, this code ends up migrating the task only if the
> > CPU was previously idle? (scheduling the idle task if the CPU was not
> > previously idle)
> > 
> > Out of curiosity (I admit this is my ignorance), why is this needed?
> > If I understand well, after scheduling the idle task the scheduler will
> > be invoked again (because of the set_tsk_need_resched(rq->idle)) but I
> > do not understand why it is not possible to migrate task "p" immediately
> > (I would just check "rq->curr != p", to avoid migrating the currently
> > scheduled task).
> 
> As the comment suggests, I was also puzzled by this bit.
> 
> I'd be inclined to agree with you, it seems that the only case in which
> we want to "temporarily" schedule the idle task is if the proxy was
> executing (so it just blocked on the mutex and being scheduled out).
> 
> If it wasn't we should be able to let the current "curr" continue
> executing, in this case returning it as next will mean that schedule
> takes the else branch and there isn't an actual context switch.
> 
> > > +	rq->proxy = &fake_task;
> 
> [...]
> 
> We can maybe also rq->proxy = rq->curr and return rq->curr in such a
> case, instead of the below?
> 
> > > +	return NULL; /* Retry task selection on _this_ CPU. */
> 
> Peter, what are we missing? :-)

I think it was the safe and simple choice; note that we're not migrating
just a single @p, but a whole chain of @p. rq->curr must not be any of the
possible @p's. rq->idle, is per definition not one of the @p's.

Does that make sense?