Re: [PATCH v16 5/7] sched: Add an initial sketch of the find_proxy_task() function

[John Stultz]

On Mon, Apr 14, 2025 at 2:42 AM Juri Lelli <juri.lelli@xxxxxxxxxx> wrote:
>
> Hi John,
>
> On 11/04/25 23:02, John Stultz wrote:
> > Add a find_proxy_task() function which doesn't do much.
> >
> > When we select a blocked task to run, we will just deactivate it
> > and pick again. The exception being if it has become unblocked
> > after find_proxy_task() was called.
> >
> > Greatly simplified from patch by:
> >   Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> >   Juri Lelli <juri.lelli@xxxxxxxxxx>
> >   Valentin Schneider <valentin.schneider@xxxxxxx>
> >   Connor O'Brien <connoro@xxxxxxxxxx>
> >
> > Cc: Joel Fernandes <joelagnelf@xxxxxxxxxx>
> > Cc: Qais Yousef <qyousef@xxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Cc: Juri Lelli <juri.lelli@xxxxxxxxxx>
> > Cc: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > Cc: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
> > Cc: Valentin Schneider <vschneid@xxxxxxxxxx>
> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > Cc: Ben Segall <bsegall@xxxxxxxxxx>
> > Cc: Zimuzo Ezeozue <zezeozue@xxxxxxxxxx>
> > Cc: Mel Gorman <mgorman@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > Cc: Waiman Long <longman@xxxxxxxxxx>
> > Cc: Boqun Feng <boqun.feng@xxxxxxxxx>
> > Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> > Cc: Metin Kaya <Metin.Kaya@xxxxxxx>
> > Cc: Xuewen Yan <xuewen.yan94@xxxxxxxxx>
> > Cc: K Prateek Nayak <kprateek.nayak@xxxxxxx>
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> > Cc: Suleiman Souhlal <suleiman@xxxxxxxxxx>
> > Cc: kernel-team@xxxxxxxxxxx
> > [jstultz: Split out from larger proxy patch and simplified
> >  for review and testing.]
> > Signed-off-by: John Stultz <jstultz@xxxxxxxxxx>
> > ---
>
> ...
>
> > +static struct task_struct *
> > +find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> > +{
> > +     struct task_struct *p = donor;
> > +     struct mutex *mutex;
> > +
> > +     mutex = p->blocked_on;
> > +     /* Something changed in the chain, so pick again */
> > +     if (!mutex)
> > +             return NULL;
> > +     /*
> > +      * By taking mutex->wait_lock we hold off concurrent mutex_unlock()
> > +      * and ensure @owner sticks around.
> > +      */
> > +     guard(raw_spinlock)(&mutex->wait_lock);
> > +
> > +     /* Check again that p is blocked with blocked_lock held */
> > +     if (!task_is_blocked(p) || mutex != __get_task_blocked_on(p)) {
> > +             /*
> > +              * Something changed in the blocked_on chain and
> > +              * we don't know if only at this level. So, let's
> > +              * just bail out completely and let __schedule
> > +              * figure things out (pick_again loop).
> > +              */
> > +             return NULL; /* do pick_next_task again */
> > +     }
> > +     return proxy_deactivate(rq, donor);
>
> Nitpick. We might either want to use donor (and remove p) or use p
> everywhere in the function; think makes it more clear.

Ah, yes, in this step it, it would be more clear.  The two separate
values are important later when we walk the chain, since we use p to
iterate down the chain, but if we cannot find a proxy to run, we need
to deactivate the donor.

I'll rework this chunk to just use donor in this step.

thanks
-john