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]

Message-ID: <20181011123448.GS9130@localhost.localdomain>
Date:   Thu, 11 Oct 2018 14:34:48 +0200
From:   Juri Lelli <juri.lelli@...hat.com>
To:     luca abeni <luca.abeni@...tannapisa.it>
Cc:     peterz@...radead.org, 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

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? :-)