Re: [PATCH] sched/deadline: Use task_on_rq_migrating() helper

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

Hello Liang,

On 6/8/2026 12:23 PM, luoliang@kylinos.cn wrote:
> From: Liang Luo <luoliang@kylinos.cn>
> 
> Replace the open-coded "p->on_rq == TASK_ON_RQ_MIGRATING" comparisons
> in enqueue_task_dl() and dequeue_task_dl() with the existing
> task_on_rq_migrating() helper, consistent with the rest of the
> scheduler code.

Good cleanup but ...

> 
> The helper uses READ_ONCE() when loading ->on_rq, which was added by
> commit c546951d9c93 ("sched/core: Use READ_ONCE()/WRITE_ONCE() in
> move_queued_task()/task_rq_lock()") for LKMM compliance.  As documented
> in that commit, move_queued_task() synchronizes with task_rq_lock():
> 
>   move_queued_task()                task_rq_lock()
>   [S] ->on_rq = MIGRATING          [L] rq = task_rq()
>   WMB (__set_task_cpu())           ACQUIRE (rq->lock)
>   [S] ->cpu = new_cpu              [L] ->on_rq
> 
> The READ_ONCE() on the ->on_rq load ensures proper pairing with the
> WRITE_ONCE() on the writer side.  The open-coded checks in deadline.c
> lack this annotation.

... this extra context feels unnecessary since the p->on_rq indicator
is always stable at the time of dequeue / enqueue.

The calling context already holds the rq_lock() (and sometimes, also
the p->pi_lock) so there are no concurrent writers for it to pair with
- in fact this same context will set p->on_rq back to TASK_ON_RQ_QUEUED
after it is done enqueuing and before dropping the rq_lock().

> 
> No functional change.
> 
> Signed-off-by: Liang Luo <luoliang@kylinos.cn>

For the change itself, feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>

> ---
>  kernel/sched/deadline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 7db4c87df83b..9d2c42b8661f 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2335,7 +2335,7 @@ static void enqueue_task_dl(struct rq *rq, struct task_struct *p, int flags)
>         check_schedstat_required();
>         update_stats_wait_start_dl(dl_rq_of_se(&p->dl), &p->dl);
> 
> -       if (p->on_rq == TASK_ON_RQ_MIGRATING)
> +       if (task_on_rq_migrating(p))
>                 flags |= ENQUEUE_MIGRATING;
> 
>         enqueue_dl_entity(&p->dl, flags);
> @@ -2354,7 +2354,7 @@ static bool dequeue_task_dl(struct rq *rq, struct task_struct *p, int flags)
>  {
>         update_curr_dl(rq);
> 
> -       if (p->on_rq == TASK_ON_RQ_MIGRATING)
> +       if (task_on_rq_migrating(p))
>                 flags |= DEQUEUE_MIGRATING;
> 
>         dequeue_dl_entity(&p->dl, flags);
> --
> 2.25.1
> 

-- 
Thanks and Regards,
Prateek