mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] sched/deadline: Use task_on_rq_migrating() helper
@ 2026-06-08  6:53 luoliang
  2026-06-08  7:10 ` K Prateek Nayak
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: luoliang @ 2026-06-08  6:53 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra
  Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, K Prateek Nayak,
	linux-kernel, Liang Luo

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.

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.

No functional change.

Signed-off-by: Liang Luo <luoliang@kylinos.cn>
---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/deadline: Use task_on_rq_migrating() helper
  2026-06-08  6:53 [PATCH] sched/deadline: Use task_on_rq_migrating() helper luoliang
@ 2026-06-08  7:10 ` K Prateek Nayak
  2026-06-08  7:55 ` [PATCH v2] " luoliang
  2026-06-08  8:01 ` [PATCH] " Peter Zijlstra
  2 siblings, 0 replies; 6+ messages in thread
From: K Prateek Nayak @ 2026-06-08  7:10 UTC (permalink / raw)
  To: luoliang, Ingo Molnar, Peter Zijlstra
  Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] sched/deadline: Use task_on_rq_migrating() helper
  2026-06-08  6:53 [PATCH] sched/deadline: Use task_on_rq_migrating() helper luoliang
  2026-06-08  7:10 ` K Prateek Nayak
@ 2026-06-08  7:55 ` luoliang
  2026-06-09  8:32   ` [tip: sched/core] " tip-bot2 for Liang Luo
  2026-06-08  8:01 ` [PATCH] " Peter Zijlstra
  2 siblings, 1 reply; 6+ messages in thread
From: luoliang @ 2026-06-08  7:55 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra
  Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, K Prateek Nayak,
	linux-kernel, Liang Luo

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.

No functional change.

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Liang Luo <luoliang@kylinos.cn>
---
(no changes since v1)

 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/deadline: Use task_on_rq_migrating() helper
  2026-06-08  6:53 [PATCH] sched/deadline: Use task_on_rq_migrating() helper luoliang
  2026-06-08  7:10 ` K Prateek Nayak
  2026-06-08  7:55 ` [PATCH v2] " luoliang
@ 2026-06-08  8:01 ` Peter Zijlstra
  2026-06-09  5:52   ` luoliang
  2 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2026-06-08  8:01 UTC (permalink / raw)
  To: luoliang
  Cc: Ingo Molnar, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
	K Prateek Nayak, linux-kernel

On Mon, Jun 08, 2026 at 02:53:07PM +0800, 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.
> 
> 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.
> 
> No functional change.

This is completely irrelevant here. p->on_rq is set with rq->lock held,
and tasks are {en,de}queued with rq->lock held. There is no concurrency.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sched/deadline: Use task_on_rq_migrating() helper
  2026-06-08  8:01 ` [PATCH] " Peter Zijlstra
@ 2026-06-09  5:52   ` luoliang
  0 siblings, 0 replies; 6+ messages in thread
From: luoliang @ 2026-06-09  5:52 UTC (permalink / raw)
  To: peterz
  Cc: mingo, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt,
	bsegall, mgorman, vschneid, kprateek.nayak, linux-kernel

On Mon, Jun 08, 2026 at 10:01:56AM +0200, Peter Zijlstra wrote:
> This is completely irrelevant here. p->on_rq is set with rq->lock held,
> and tasks are {en,de}queued with rq->lock held. There is no concurrency.

You are right, my mistake. The READ_ONCE() explanation was incorrect.
I've already sent v2 with the commit message fixed:

  https://lore.kernel.org/all/20260608075500.387271-1-luoliang@kylinos.cn/

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip: sched/core] sched/deadline: Use task_on_rq_migrating() helper
  2026-06-08  7:55 ` [PATCH v2] " luoliang
@ 2026-06-09  8:32   ` tip-bot2 for Liang Luo
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Liang Luo @ 2026-06-09  8:32 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Liang Luo, Peter Zijlstra (Intel), K Prateek Nayak, x86, linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     9ebe5c3c29f6217412ff256134516d4dff0e5624
Gitweb:        https://git.kernel.org/tip/9ebe5c3c29f6217412ff256134516d4dff0e5624
Author:        Liang Luo <luoliang@kylinos.cn>
AuthorDate:    Mon, 08 Jun 2026 15:55:00 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 09 Jun 2026 10:28:08 +02:00

sched/deadline: Use task_on_rq_migrating() helper

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.

No functional change.

Signed-off-by: Liang Luo <luoliang@kylinos.cn>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Link: https://patch.msgid.link/20260608075500.387271-1-luoliang@kylinos.cn
---
 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 4754dbe..5ccb06e 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2530,7 +2530,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, dl_se);
 
-	if (p->on_rq == TASK_ON_RQ_MIGRATING)
+	if (task_on_rq_migrating(p))
 		flags |= ENQUEUE_MIGRATING;
 
 	enqueue_dl_entity(dl_se, flags);
@@ -2552,7 +2552,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);

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-09  8:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-08  6:53 [PATCH] sched/deadline: Use task_on_rq_migrating() helper luoliang
2026-06-08  7:10 ` K Prateek Nayak
2026-06-08  7:55 ` [PATCH v2] " luoliang
2026-06-09  8:32   ` [tip: sched/core] " tip-bot2 for Liang Luo
2026-06-08  8:01 ` [PATCH] " Peter Zijlstra
2026-06-09  5:52   ` luoliang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome