Re: [PATCH v25 9/9] sched: Handle blocked-waiter migration (and return migration)

[Juri Lelli]

Hello,

I couldn't convince myself the below is not potentially racy ...

On 13/03/26 02:30, John Stultz wrote:

...

> +static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,
> +			       struct task_struct *p, int target_cpu)
>  {
> -	if (!__proxy_deactivate(rq, donor)) {
> -		/*
> -		 * XXX: For now, if deactivation failed, set donor
> -		 * as unblocked, as we aren't doing proxy-migrations
> -		 * yet (more logic will be needed then).
> -		 */
> -		clear_task_blocked_on(donor, NULL);
> +	struct rq *target_rq = cpu_rq(target_cpu);
> +
> +	lockdep_assert_rq_held(rq);
> +
> +	/*
> +	 * Since we're going to drop @rq, we have to put(@rq->donor) first,
> +	 * otherwise we have a reference that no longer belongs to us.
> +	 *
> +	 * Additionally, as we put_prev_task(prev) earlier, its possible that
> +	 * prev will migrate away as soon as we drop the rq lock, however we
> +	 * still have it marked as rq->curr, as we've not yet switched tasks.
> +	 *
> +	 * So call proxy_resched_idle() to let go of the references before
> +	 * we release the lock.
> +	 */
> +	proxy_resched_idle(rq);
> +
> +	WARN_ON(p == rq->curr);
> +
> +	deactivate_task(rq, p, DEQUEUE_NOCLOCK);
> +	proxy_set_task_cpu(p, target_cpu);
> +
> +	/*
> +	 * We have to zap callbacks before unlocking the rq
> +	 * as another CPU may jump in and call sched_balance_rq
> +	 * which can trip the warning in rq_pin_lock() if we
> +	 * leave callbacks set.
> +	 */
> +	zap_balance_callbacks(rq);
> +	rq_unpin_lock(rq, rf);
> +	raw_spin_rq_unlock(rq);
> +
> +	attach_one_task(target_rq, p);

We release rq lock between deactivate and attach (and we don't hold
neither wait_lock nor blocked_lock as they are out of scope at this
point). Can't something like the following happen?

  - Task A: blocked on mutex M, queued on CPU 0
  - Task B: owns mutex M, running on CPU 1

  CPU 0 (migrating A→CPU 1)        CPU 1 (B finishes critical section)
  -------------------------        ------------------------------------
  find_proxy_task(donor=A):
    owner = B, owner_cpu = 1
    action = MIGRATE
    // guard releases wait_lock

  proxy_migrate_task(A, cpu=1):
    deactivate_task(rq0, A)
      → A->on_rq = 0
    proxy_set_task_cpu(A, 1)
      → A->cpu = 1
    raw_spin_rq_unlock(rq0)
      → RQ0 LOCK RELEASED
                                   // Task B running
                                   mutex_unlock(M):
                                     lock(&M->wait_lock)   // ← Can grab it
                                     A->blocked_on = PROXY_WAKING
                                     unlock(&M->wait_lock)
                                     wake_up_q():
                                       try_to_wake_up(A):
                                         sees A->on_rq == 0
                                         cpu = select_task_rq(A)
                                           → returns CPU 2
                                         set_task_cpu(A, 2)
                                         ttwu_queue(A, 2)
                                           → A enqueued on CPU 2
                                           → A->on_rq = 1, A->cpu = 2

  attach_one_task(rq1, A):
    attach_task(rq1, A):
      WARN_ON_ONCE(task_rq(A) != rq1)
        → Fires! task_rq(A) = rq2
      activate_task(rq1, A)
        → Double-enqueue! A->on_rq already = 1

What am I missing? :)

Thanks,
Juri