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

[John Stultz]

On Thu, Dec 21, 2023 at 7:03 AM Metin Kaya <metin.kaya@xxxxxxx> 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