All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains()
@ 2026-08-12  9:58 Sebastian Andrzej Siewior
  2026-08-12 12:54 ` Chen, Yu C
  0 siblings, 1 reply; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-08-12  9:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ben Segall, Chen Yu, Dietmar Eggemann, Ingo Molnar, Juri Lelli,
	K Prateek Nayak, Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Tim Chen, Valentin Schneider, Vincent Guittot

A read from /proc/sys/kernel/sched_rt_runtime_us leads to backtrace due
to missing cpu_hotplug_lock. The callchain is sched_rt_handler() ->
partition_sched_domains() -> sched_cache_set() ->
static_key_enable_cpuslocked(&sched_cache_present).

sched_cache_set() itself is also invoked from sched_init_domains() which
is early during the boot, holding just the sched_domains_mutex_lock().
Here is no warning because it happens before user space is running (and
hotplug operations are not possible).

There is also sched_cache_active_set() which acquires the hotplug lock
via before invoking any of the _cpuslocked() functions.

Acquire CPU hotplug lock before in partition_sched_domains(), before
sched_domains_mutex before the _cpuslocked() functions are invoked.

Fixes: a7660ce1590fc ("sched/cache: Fix has_multi_llcs iff at least one partition has multiple LLCs")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/sched/topology.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 622e2e01974c4..ef037d6664d98 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -3498,6 +3498,7 @@ static void partition_sched_domains_locked(int ndoms_new, cpumask_var_t doms_new
 void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
 			     struct sched_domain_attr *dattr_new)
 {
+	guard(cpus_read_lock)();
 	sched_domains_mutex_lock();
 	partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
 	sched_domains_mutex_unlock();
-- 
2.55.0

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

* Re: [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains()
  2026-08-12  9:58 [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains() Sebastian Andrzej Siewior
@ 2026-08-12 12:54 ` Chen, Yu C
  2026-08-12 18:05   ` Tim Chen
  2026-08-13  7:00   ` Sebastian Andrzej Siewior
  0 siblings, 2 replies; 5+ messages in thread
From: Chen, Yu C @ 2026-08-12 12:54 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Ben Segall, Dietmar Eggemann, Ingo Molnar, Juri Lelli,
	K Prateek Nayak, Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Tim Chen, Valentin Schneider, Vincent Guittot, linux-kernel,
	chen.yu@linux.dev

Hi Sebastian,

On 8/12/2026 5:58 PM, Sebastian Andrzej Siewior wrote:
> A read from /proc/sys/kernel/sched_rt_runtime_us leads to backtrace due
> to missing cpu_hotplug_lock. The callchain is sched_rt_handler() ->
> partition_sched_domains() -> sched_cache_set() ->
> static_key_enable_cpuslocked(&sched_cache_present).
> 
> sched_cache_set() itself is also invoked from sched_init_domains() which
> is early during the boot, holding just the sched_domains_mutex_lock().
> Here is no warning because it happens before user space is running (and
> hotplug operations are not possible).
> 
> There is also sched_cache_active_set() which acquires the hotplug lock
> via before invoking any of the _cpuslocked() functions.
> 
> Acquire CPU hotplug lock before in partition_sched_domains(), before
> sched_domains_mutex before the _cpuslocked() functions are invoked.
> 
> Fixes: a7660ce1590fc ("sched/cache: Fix has_multi_llcs iff at least one partition has multiple LLCs")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   kernel/sched/topology.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 622e2e01974c4..ef037d6664d98 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -3498,6 +3498,7 @@ static void partition_sched_domains_locked(int ndoms_new, cpumask_var_t doms_new
>   void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
>   			     struct sched_domain_attr *dattr_new)
>   {
> +	guard(cpus_read_lock)();
>   	sched_domains_mutex_lock();
>   	partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
>   	sched_domains_mutex_unlock();

Thanks for taking a look at this issue. I found that there is a comment 
around
partition_sched_domains() says: "Call with hotplug lock held"
Not sure if the caller of partition_sched_domains() rather than 
partition_sched_domains()
should grab the cpuhotplug lock? I guess the issue is triggered when
CONFIG_CPUSETS=n, in this case rebuild_sched_domains() will not grab
cpuhotplug lock, should we add guard(cpus_read_lock) in 
rebuild_sched_domains()
instead?

thanks,
Chenyu



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

* Re: [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains()
  2026-08-12 12:54 ` Chen, Yu C
@ 2026-08-12 18:05   ` Tim Chen
  2026-08-13  2:24     ` Chen Yu
  2026-08-13  7:00   ` Sebastian Andrzej Siewior
  1 sibling, 1 reply; 5+ messages in thread
From: Tim Chen @ 2026-08-12 18:05 UTC (permalink / raw)
  To: Chen, Yu C, Sebastian Andrzej Siewior
  Cc: Ben Segall, Dietmar Eggemann, Ingo Molnar, Juri Lelli,
	K Prateek Nayak, Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Valentin Schneider, Vincent Guittot, linux-kernel,
	chen.yu@linux.dev

On Wed, 2026-08-12 at 20:54 +0800, Chen, Yu C wrote:
> Hi Sebastian,
> 
> On 8/12/2026 5:58 PM, Sebastian Andrzej Siewior wrote:
> > A read from /proc/sys/kernel/sched_rt_runtime_us leads to backtrace due
> > to missing cpu_hotplug_lock. The callchain is sched_rt_handler() ->
> > partition_sched_domains() -> sched_cache_set() ->
> > static_key_enable_cpuslocked(&sched_cache_present).
> > 
> > sched_cache_set() itself is also invoked from sched_init_domains() which
> > is early during the boot, holding just the sched_domains_mutex_lock().
> > Here is no warning because it happens before user space is running (and
> > hotplug operations are not possible).
> > 
> > There is also sched_cache_active_set() which acquires the hotplug lock
> > via before invoking any of the _cpuslocked() functions.
> > 
> > Acquire CPU hotplug lock before in partition_sched_domains(), before
> > sched_domains_mutex before the _cpuslocked() functions are invoked.
> > 
> > Fixes: a7660ce1590fc ("sched/cache: Fix has_multi_llcs iff at least one partition has multiple LLCs")
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >   kernel/sched/topology.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 622e2e01974c4..ef037d6664d98 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -3498,6 +3498,7 @@ static void partition_sched_domains_locked(int ndoms_new, cpumask_var_t doms_new
> >   void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
> >   			     struct sched_domain_attr *dattr_new)
> >   {
> > +	guard(cpus_read_lock)();
> >   	sched_domains_mutex_lock();
> >   	partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
> >   	sched_domains_mutex_unlock();
> 
> Thanks for taking a look at this issue. I found that there is a comment 
> around
> partition_sched_domains() says: "Call with hotplug lock held"
> Not sure if the caller of partition_sched_domains() rather than 
> partition_sched_domains()
> should grab the cpuhotplug lock? I guess the issue is triggered when
> CONFIG_CPUSETS=n, in this case rebuild_sched_domains() will not grab
> cpuhotplug lock, should we add guard(cpus_read_lock) in 
> rebuild_sched_domains()
> instead?

When CONFIG_CPUSETS=n, rebuild_sched_domains() is stubbed to

static inline void rebuild_sched_domains(void)
{
        partition_sched_domains(1, NULL, NULL);
}

without cpus_read_lock.  Are you suggesting adding the lock
here?  We do acquire the lock for CONFIG_CPUSETS=y.

If we do follow this convention, cpuset_reset_sched_domains()
and cpuset_reset_sched_domains() also stubbed to
partition_sched_domains(1, NULL, NULL);
Probably should add cpus_read_lock there too for consistency.

Tim

> 
> thanks,
> Chenyu
> 

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

* Re: [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains()
  2026-08-12 18:05   ` Tim Chen
@ 2026-08-13  2:24     ` Chen Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Chen Yu @ 2026-08-13  2:24 UTC (permalink / raw)
  To: Tim Chen
  Cc: Chen, Yu C, Sebastian Andrzej Siewior, Ben Segall,
	Dietmar Eggemann, Ingo Molnar, Juri Lelli, K Prateek Nayak,
	Mel Gorman, Peter Zijlstra, Steven Rostedt, Valentin Schneider,
	Vincent Guittot, linux-kernel

On Wed, Aug 12, 2026 at 11:05:24AM -0700, Tim Chen wrote:
> > >   void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
> > >   			     struct sched_domain_attr *dattr_new)
> > >   {
> > > +	guard(cpus_read_lock)();
> > >   	sched_domains_mutex_lock();
> > >   	partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
> > >   	sched_domains_mutex_unlock();
> > 
> > Thanks for taking a look at this issue. I found that there is a comment 
> > around
> > partition_sched_domains() says: "Call with hotplug lock held"
> > Not sure if the caller of partition_sched_domains() rather than 
> > partition_sched_domains()
> > should grab the cpuhotplug lock? I guess the issue is triggered when
> > CONFIG_CPUSETS=n, in this case rebuild_sched_domains() will not grab
> > cpuhotplug lock, should we add guard(cpus_read_lock) in 
> > rebuild_sched_domains()
> > instead?
> 
> When CONFIG_CPUSETS=n, rebuild_sched_domains() is stubbed to
> 
> static inline void rebuild_sched_domains(void)
> {
>         partition_sched_domains(1, NULL, NULL);
> }
> 
> without cpus_read_lock.  Are you suggesting adding the lock
> here?  We do acquire the lock for CONFIG_CPUSETS=y.
>

Yes, I think so.
 
> If we do follow this convention, cpuset_reset_sched_domains()
> and cpuset_reset_sched_domains() also stubbed to
> partition_sched_domains(1, NULL, NULL);
> Probably should add cpus_read_lock there too for consistency.
>

cpuset_reset_sched_domains() is invoked by cpuset_cpu_inactive() 
and via CPU-hotplug callback, and in cpuhp_thread_fun:
static void cpuhp_thread_fun(unsigned int cpu)
{
	lockdep_acquire_cpus_lock(); <--- lockdep
	call_back();
}
and according to the comments:
"
/*
 * The BP holds the hotplug lock, but we're now running on the AP,
 * ensure that anybody asserting the lock is held, will actually find
 * it so.
 */
"
it pretend that AP has grabed the lock on behalf of the BP in
_cpu_down(), which has grabed the cpu-hotplug write lock:
cpus_write_lock().

So it should be ok to not take hotplug lock in cpuset_reset_sched_domains().

thanks,
Chenyu
 
 
> Tim
> 
> > 
> > thanks,
> > Chenyu
> > 

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

* Re: [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains()
  2026-08-12 12:54 ` Chen, Yu C
  2026-08-12 18:05   ` Tim Chen
@ 2026-08-13  7:00   ` Sebastian Andrzej Siewior
  1 sibling, 0 replies; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-08-13  7:00 UTC (permalink / raw)
  To: Chen, Yu C
  Cc: Ben Segall, Dietmar Eggemann, Ingo Molnar, Juri Lelli,
	K Prateek Nayak, Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Tim Chen, Valentin Schneider, Vincent Guittot, linux-kernel,
	chen.yu@linux.dev

On 2026-08-12 20:54:49 [+0800], Chen, Yu C wrote:
> Hi Sebastian,
Hi,

> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -3498,6 +3498,7 @@ static void partition_sched_domains_locked(int ndoms_new, cpumask_var_t doms_new
> >   void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
> >   			     struct sched_domain_attr *dattr_new)
> >   {
> > +	guard(cpus_read_lock)();
> >   	sched_domains_mutex_lock();
> >   	partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
> >   	sched_domains_mutex_unlock();
> 
> Thanks for taking a look at this issue. I found that there is a comment
> around
> partition_sched_domains() says: "Call with hotplug lock held"
> Not sure if the caller of partition_sched_domains() rather than
> partition_sched_domains()
> should grab the cpuhotplug lock? I guess the issue is triggered when
How did I miss that part?

> CONFIG_CPUSETS=n, in this case rebuild_sched_domains() will not grab
> cpuhotplug lock, should we add guard(cpus_read_lock) in
> rebuild_sched_domains()
> instead?

Let me look…

> thanks,
> Chenyu

Sebastian

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

end of thread, other threads:[~2026-08-13  7:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12  9:58 [PATCH] sched/topology: Add a cpus_read_lock to partition_sched_domains() Sebastian Andrzej Siewior
2026-08-12 12:54 ` Chen, Yu C
2026-08-12 18:05   ` Tim Chen
2026-08-13  2:24     ` Chen Yu
2026-08-13  7:00   ` Sebastian Andrzej Siewior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.