Re: SCX_ENQ_IMMED potentially leaving dispatched tasks lingering on local DSQs

[Kuba Piecuch <jpiecuch@google.com>]

On Thu Apr 23, 2026 at 4:53 PM UTC, Tejun Heo wrote:
> Hello,
>
> On Thu, Apr 23, 2026 at 09:48:11AM +0000, Kuba Piecuch wrote:
>> I'm not sure if we should limit ourselves to just remote tasks.
>> If we call wakeup_preempt(rq, p, flags) when adding @p to @rq's local DSQ
>> regardless of whether @p/@rq is remote, then I think that should cover all
>> cases and the patch above wouldn't be needed.
>
> Sorry, I wasn't clear. I meant adding wakeup_preempt() in
> move_remote_task_to_local_dsq() *in addition to* your nr_immed/RETRY_TASK
> patch. That combination covers both cases:
>
> - Local (your stepwise scenario): the task stays on the dispatching CPU, no
>   migration happens, so there's no insertion-side hook to arm. Your nr_immed
>   check in do_pick_task_scx() catches it on the RETRY_TASK path.
>
> - Remote: move_remote_task_to_local_dsq() is SCX's analog of
>   move_queued_task() and is missing the wakeup_preempt() call that
>   move_queued_task() does after activate_task(). Once added,
>   dst_rq->next_class is bumped to &ext_sched_class, and any subsequent
>   higher-class wakeup on dst_rq routes through wakeup_preempt_scx() which
>   already has the nr_immed reenqueue.

There's another case we need to handle: dispatching to a remote CPU's
local DSQ, but without migrating the task. In dispatch_to_local_dsq()
this corresponds to rq != src_rq && src_rq == dst_rq. The check for the local
case on the remote CPU doesn't cover it because an RT task can swoop in and
prevent pick_task_scx() from running (the assumption is that the remote CPU
is idle).
The check for the remote case doesn't cover it either because we do
dispatch_enqueue() directly from dispatch_to_local_dsq().

Then there's the same case, but instead of dispatching we're moving the task
from a user DSQ. If my understanding of the code is correct, the path is:

  scx_dsq_move(p, dsq_id);
    raw_spin_rq_unlock(this_rq);
    raw_spin_lock(src_rq);
    dst_dsq = find_dsq_for_dispatch(dsq_id);
    move_task_between_dsqs(p, src_dsq, dst_dsq);
      dst_rq = container_of(dst_dsq, struct rq, scx.local_dsq);
      task_unlink_from_dsq(p, src_dsq);
      move_local_task_to_local_dsq(p, src_dsq, dst_rq);

I think these two cases show that we don't necessarily have to migrate a task
for wakeup_preempt() to be warranted.

So to cover all cases (that I'm aware of), we need four checks:

* wakeup_preempt() in dispatch_to_local_dsq() in the case of rq != src_rq &&
  src_rq == dst_rq

* wakeup_preempt() at the end of move_remote_task_to_local_dsq()

* wakeup_preempt() somewhere on the scx_dsq_move() path, but only if we're
  moving the task to the local DSQ of a remote CPU (?)

* nr_immed check before returning RETRY_TASK

>
> Adding wakeup_preempt() to local_dsq_post_enq() for every insertion would
> work too, but I'd rather keep it in sync with how core sched handles
> move_queued_task() and only add it where it's actually needed.

Does having four separate cases to handle, not all of which involve task
migration, change that calculus somewhat?
I believe all of these cases will be handled if we add wakeup_preempt()
to local_dsq_post_enq().

Thanks,
Kuba