Re: [PATCH v30 7/7] sched: Add deactivated (sleeping) owner handling to find_proxy_task()

[John Stultz]

On Thu, Jul 2, 2026 at 8:07 PM K Prateek Nayak <kprateek.nayak@xxxxxxx> wrote:
> On 7/2/2026 3:16 AM, John Stultz wrote:
> > @@ -6852,6 +7073,28 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,
> >       proxy_reacquire_rq_lock(rq, rf);
> >  }
> >
> > +static void proxy_enqueue_on_owner(struct rq *rq, struct task_struct *owner,
> > +                                struct task_struct *p)
> > +{
> > +     lockdep_assert_rq_held(rq);
> > +     lockdep_assert_held(&owner->blocked_lock);
> > +     /*
> > +      * ttwu_activate() will pick them up and place them on whatever rq
> > +      * @owner will run next.
> > +      */
> > +     WARN_ON(p == owner);
> > +     WARN_ON(!p->on_rq);
> > +     WARN_ON(p->sleeping_owner);
> > +     get_task_struct(owner);
> > +     WRITE_ONCE(p->sleeping_owner, owner);
> > +     /*
> > +      * ttwu_do_activate must not have a chance to activate p
> > +      * elsewhere before it's fully extricated from its old rq.
> > +      */
> > +     list_add(&p->blocked_node, &owner->blocked_head);
>
> I'll refer to this list_add() as (1) below ...
>
> > +     block_task(rq, p, READ_ONCE(p->__state));
> > +}
> > +
> >  /*
> >   * Find runnable lock owner to proxy for mutex blocked donor
> >   *
> > @@ -6938,11 +7181,31 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> >               }
> >
> >               if (!READ_ONCE(owner->on_rq) || owner->se.sched_delayed) {
>
> I'm having a sneaky feeling that, with enough bad luck, we
> can go from seeing !owner->on_rq and here to actually finish
> enqueuing the owner by the time we get to (1) ...
>
> > -                     /* XXX Don't handle blocked owners/delayed dequeue yet */
> > +                     /*
> > +                      * rq->curr must not be added to the blocked_head list or else
> > +                      * ttwu_do_activate could enqueue it elsewhere before it switches
> > +                      * out here. The approach to avoid this is the same as in the
> > +                      * migrate_task case.
> > +                      */
> >                       if (curr_in_chain)
> >                               return proxy_resched_idle(rq);
> > -                     __clear_task_blocked_on(p, NULL);
> > -                     goto deactivate;
> > +                     /*
> > +                      * If !@owner->on_rq, holding @rq->lock will not pin the task,
> > +                      * so we cannot drop @mutex->wait_lock until we're sure its a blocked
> > +                      * task on this rq.
> > +                      *
> > +                      * We use @owner->blocked_lock to serialize against ttwu_activate().
> > +                      * Either we see its new owner->on_rq or it will see our list_add().
> > +                      */
> > +                     WARN_ON(owner == p);
> > +                     raw_spin_unlock(&p->blocked_lock);
> > +                     raw_spin_lock(&owner->blocked_lock);
>
> ... by the time we get here, activate_blocked_waiters() could have
> already bailed out for:
>
>   !list_empty(&owner->blocked_activation_node)
>
> and then we go and add the task to list. I feel, like most conditionally
> adding to list patterns, we should add the task to list, check the
> condition once again under owner->blocked_lock, and then proceed.
>
> That way we can sure the activate will definitely see the queued task.
> Sorry in advance in case I've been blind and missed something
> obvious :-)

Hrm.  That's a good point!

Let me take a stab at fixing that.  I'm thinking it might be easier to
check on_rq again after we take the blocked lock before calling
proxy_enqueue_on_owner(), because otherwise unwinding that is a little
complicated (pulling it off the list isn't, but the task will have
been blocked, which isn't what we want if we are going to pick again
to figure out if we can proxy or have to migrate the waiter).  Am I
still missing a race with that approach (instead of adding to the list
before the check)?

> On a separate note, is it worth tracking if the task is a mutex / lock
> owner and only go through the activate_blocked_waiters() path only when
> a __mutex_owner() somewhere can return the task being activated? Perhaps
> a per-task counter that increments at every mutex_lock() and decrements
> at a mutex_unlock()?

This as an optimization to avoid taking the owner->blocked_lock and
the list checking?
Let me think on that a bit.
For normal mutexes it seems easy enough but I want to make sure it
doesn't get complicated with ww_mutexes or the later proxy-rwsem
support from the full series.

thanks
-john