Received-SPF: pass (google.com: domain of linux-kernel+bounces-9069-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20231220001856.3710363-1-jstultz@google.com> <20231220001856.3710363-13-jstultz@google.com>
 <de0ac26f-1fde-4472-8570-7d0bf730176a@arm.com>
In-Reply-To: <de0ac26f-1fde-4472-8570-7d0bf730176a@arm.com>
From: John Stultz <jstultz@google.com>
Date: Thu, 21 Dec 2023 13:02:05 -0800
Message-ID: <CANDhNCqULO0fEf-SgG6yvW104145eDPsAcxCLBxXZ1FUCodtfg@mail.gmail.com>
Subject: Re: [PATCH v7 12/23] sched: Fix proxy/current (push,pull)ability
To: Metin Kaya <metin.kaya@arm.com>
Cc: LKML <linux-kernel@vger.kernel.org>, 
	Valentin Schneider <valentin.schneider@arm.com>, Joel Fernandes <joelaf@google.com>, 
	Qais Yousef <qyousef@google.com>, Ingo Molnar <mingo@redhat.com>, 
	Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, 
	Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, 
	Valentin Schneider <vschneid@redhat.com>, Steven Rostedt <rostedt@goodmis.org>, 
	Ben Segall <bsegall@google.com>, Zimuzo Ezeozue <zezeozue@google.com>, 
	Youssef Esmat <youssefesmat@google.com>, Mel Gorman <mgorman@suse.de>, 
	Daniel Bristot de Oliveira <bristot@redhat.com>, Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>, 
	Boqun Feng <boqun.feng@gmail.com>, "Paul E. McKenney" <paulmck@kernel.org>, 
	Xuewen Yan <xuewen.yan94@gmail.com>, K Prateek Nayak <kprateek.nayak@amd.com>, 
	Thomas Gleixner <tglx@linutronix.de>, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Dec 21, 2023 at 7:03=E2=80=AFAM Metin Kaya <metin.kaya@arm.com> wro=
te:
> On 20/12/2023 12:18 am, John Stultz wrote:
> > +static inline void proxy_tag_curr(struct rq *rq, struct task_struct *n=
ext)
> > +{
> > +     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 =3D context_switch(rq, prev, next, &rf);
> >       } else {
> > +             /* In case next was already curr but just got blocked_don=
or*/
>
> Super-nit: please keep a space before */.

Fixed up.

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