linux-kernel - Re: [PATCH v7 12/23] sched: Fix proxy/current (push,pull)ability

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, 21 Dec 2023 13:02:05 -0800
From: John Stultz <jstultz@...gle.com>
To: Metin Kaya <metin.kaya@....com>
Cc: LKML <linux-kernel@...r.kernel.org>, 
	Valentin Schneider <valentin.schneider@....com>, Joel Fernandes <joelaf@...gle.com>, 
	Qais Yousef <qyousef@...gle.com>, Ingo Molnar <mingo@...hat.com>, 
	Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, 
	Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>, 
	Valentin Schneider <vschneid@...hat.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Ben Segall <bsegall@...gle.com>, Zimuzo Ezeozue <zezeozue@...gle.com>, 
	Youssef Esmat <youssefesmat@...gle.com>, Mel Gorman <mgorman@...e.de>, 
	Daniel Bristot de Oliveira <bristot@...hat.com>, Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>, 
	Boqun Feng <boqun.feng@...il.com>, "Paul E. McKenney" <paulmck@...nel.org>, 
	Xuewen Yan <xuewen.yan94@...il.com>, K Prateek Nayak <kprateek.nayak@....com>, 
	Thomas Gleixner <tglx@...utronix.de>, kernel-team@...roid.com
Subject: Re: [PATCH v7 12/23] sched: Fix proxy/current (push,pull)ability

On Thu, Dec 21, 2023 at 7:03 AM Metin Kaya <metin.kaya@....com> wrote:
> On 20/12/2023 12:18 am, John Stultz wrote:
> > +static inline void proxy_tag_curr(struct rq *rq, struct task_struct *next)
> > +{
> > +     if (sched_proxy_exec()) {
>
> Should we immediately return in !sched_proxy_exec() case to save one
> level of indentation?

Sure.

> > +             /*
> > +              * pick_next_task() calls set_next_task() on the selected task
> > +              * at some point, which ensures it is not push/pullable.
> > +              * However, the selected task *and* the ,mutex owner form an
>
> Super-nit: , before mutex should be dropped.
>
> > +              * atomic pair wrt push/pull.
> > +              *
> > +              * Make sure owner is not pushable. Unfortunately we can only
> > +              * deal with that by means of a dequeue/enqueue cycle. :-/
> > +              */
> > +             dequeue_task(rq, next, DEQUEUE_NOCLOCK | DEQUEUE_SAVE);
> > +             enqueue_task(rq, next, ENQUEUE_NOCLOCK | ENQUEUE_RESTORE);
> > +     }
> > +}
> > +
> >   /*
> >    * __schedule() is the main scheduler function.
> >    *
> > @@ -6796,6 +6813,10 @@ static void __sched notrace __schedule(unsigned int sched_mode)
> >                * changes to task_struct made by pick_next_task().
> >                */
> >               RCU_INIT_POINTER(rq->curr, next);
> > +
> > +             if (!task_current_selected(rq, next))
> > +                     proxy_tag_curr(rq, next);
> > +
> >               /*
> >                * The membarrier system call requires each architecture
> >                * to have a full memory barrier after updating
> > @@ -6820,6 +6841,10 @@ static void __sched notrace __schedule(unsigned int sched_mode)
> >               /* Also unlocks the rq: */
> >               rq = context_switch(rq, prev, next, &rf);
> >       } else {
> > +             /* In case next was already curr but just got blocked_donor*/
>
> Super-nit: please keep a space before */.

Fixed up.

Thanks for continuing to provide so much detailed feedback!
-john