[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250910155808.987080375@infradead.org>
Date: Wed, 10 Sep 2025 17:44:15 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: tj@...nel.org
Cc: linux-kernel@...r.kernel.org,
peterz@...radead.org,
mingo@...hat.com,
juri.lelli@...hat.com,
vincent.guittot@...aro.org,
dietmar.eggemann@....com,
rostedt@...dmis.org,
bsegall@...gle.com,
mgorman@...e.de,
vschneid@...hat.com,
longman@...hat.com,
hannes@...xchg.org,
mkoutny@...e.com,
void@...ifault.com,
arighi@...dia.com,
changwoo@...lia.com,
cgroups@...r.kernel.org,
sched-ext@...ts.linux.dev,
liuwenfang@...or.com,
tglx@...utronix.de
Subject: [PATCH 06/14] sched: Fix migrate_disable_switch() locking
For some reason migrate_disable_switch() was more complicated than it
needs to be, resulting in mind bending locking of dubious quality.
Recognise that migrate_disable_switch() must be called before a
context switch, but any place before that switch is equally good.
Since the current place results in troubled locking, simply move the
thing before taking rq->lock.
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
kernel/sched/core.c | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2344,10 +2344,10 @@ static void migrate_disable_switch(struc
if (p->cpus_ptr != &p->cpus_mask)
return;
- /*
- * Violates locking rules! See comment in __do_set_cpus_allowed().
- */
- __do_set_cpus_allowed(p, &ac);
+ scoped_guard (task_rq_lock, p) {
+ update_rq_clock(scope.rq);
+ __do_set_cpus_allowed(p, &ac);
+ }
}
void migrate_disable(void)
@@ -2702,22 +2702,7 @@ __do_set_cpus_allowed(struct task_struct
struct rq *rq = task_rq(p);
bool queued, running;
- /*
- * This here violates the locking rules for affinity, since we're only
- * supposed to change these variables while holding both rq->lock and
- * p->pi_lock.
- *
- * HOWEVER, it magically works, because ttwu() is the only code that
- * accesses these variables under p->pi_lock and only does so after
- * smp_cond_load_acquire(&p->on_cpu, !VAL), and we're in __schedule()
- * before finish_task().
- *
- * XXX do further audits, this smells like something putrid.
- */
- if (ctx->flags & SCA_MIGRATE_DISABLE)
- WARN_ON_ONCE(!p->on_cpu);
- else
- lockdep_assert_held(&p->pi_lock);
+ lockdep_assert_held(&p->pi_lock);
queued = task_on_rq_queued(p);
running = task_current_donor(rq, p);
@@ -6816,6 +6801,7 @@ static void __sched notrace __schedule(i
local_irq_disable();
rcu_note_context_switch(preempt);
+ migrate_disable_switch(rq, prev);
/*
* Make sure that signal_pending_state()->signal_pending() below
@@ -6922,7 +6908,6 @@ static void __sched notrace __schedule(i
*/
++*switch_count;
- migrate_disable_switch(rq, prev);
psi_account_irqtime(rq, prev, next);
psi_sched_switch(prev, next, !task_on_rq_queued(prev) ||
prev->se.sched_delayed);
Powered by blists - more mailing lists