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

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello John,

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 :-)

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()?

> +			proxy_resched_idle(rq);
> +			proxy_enqueue_on_owner(rq, owner, p);
> +			raw_spin_unlock(&owner->blocked_lock);
> +			raw_spin_lock(&p->blocked_lock);
> +
> +			return NULL; /* retry task selection */
>  		}
>  
>  		owner_cpu = task_cpu(owner);

-- 
Thanks and Regards,
Prateek