[PATCH 1/2] sched_ext: Drop unlocked scx_rq_clock_invalidate() from scx_root_disable()

Tejun Heo <[email protected]>
Newsgroups dev.linux.lists.sched-ext,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
scx_root_disable() invalidates each rq's clock before taking the rq lock.
scx_rq_clock_invalidate() is a plain read-modify-write of rq->scx.flags and
every other writer of the word runs under the rq lock, so the unlocked
update can race a concurrent flags update and lose one side's bits.

The invalidation is also redundant. The dl_server rebalancing right below
cycles each rq's lock and rq_unpin_lock() clears SCX_RQ_CLK_VALID on every
unlock, while __scx_enabled is turned off only later in the function, so the
clocks end up invalidated either way. Drop the call and note the
invalidation in the comment.

Fixes: 3a9910b5904d ("sched_ext: Implement scx_bpf_now()")
Signed-off-by: Tejun Heo <[email protected]>
Cc: Changwoo Min <[email protected]>
---
 kernel/sched/ext/ext.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 2adf2bde0cb1..d301fe757193 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -6476,12 +6476,13 @@ static void scx_root_disable(struct scx_sched *sch)
 	percpu_up_write(&scx_fork_rwsem);
 
 	/*
-	 * Invalidate all the rq clocks to prevent getting outdated
-	 * rq clocks from a previous scx scheduler.
+	 * Re-balance the dl_server bandwidth reservations: detach ext_server
+	 * (no more sched_ext tasks) and reinstate fair_server if it was
+	 * previously detached because we were running in full mode.
 	 *
-	 * Also re-balance the dl_server bandwidth reservations: detach
-	 * ext_server (no more sched_ext tasks) and reinstate fair_server if it
-	 * was previously detached because we were running in full mode.
+	 * The rq lock cycle also invalidates each rq's clock (rq_unpin_lock()
+	 * clears SCX_RQ_CLK_VALID on unlock), preventing the next enable from
+	 * seeing outdated rq clocks from this scheduler.
 	 *
 	 * Unlike the enable path, this runs on a recovery path that cannot
 	 * fail, so we use dl_server_swap_bw() to atomically free ext_server's
@@ -6494,8 +6495,6 @@ static void scx_root_disable(struct scx_sched *sch)
 	for_each_possible_cpu(cpu) {
 		struct rq *rq = cpu_rq(cpu);
 
-		scx_rq_clock_invalidate(rq);
-
 		scoped_guard(rq_lock_irqsave, rq) {
 			update_rq_clock(rq);
 			if (was_switched_all) {
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.