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

John Stultz
2023-12-21T21:02:20Z

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