Create wrappers for sched_domains_mutex so that it can transparently be
used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
do.
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow earlier for hotplug")
Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
---
v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
either in that case
---
include/linux/sched.h | 2 ++
kernel/cgroup/cpuset.c | 4 ++--
kernel/sched/core.c | 4 ++--
kernel/sched/debug.c | 8 ++++----
kernel/sched/topology.c | 12 ++++++++++--
5 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 9632e3318e0d..d5f8c161d852 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -383,6 +383,8 @@ enum uclamp_id {
extern struct root_domain def_root_domain;
extern struct mutex sched_domains_mutex;
#endif
+extern void sched_domains_mutex_lock(void);
+extern void sched_domains_mutex_unlock(void);
struct sched_param {
int sched_priority;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0f910c828973..f87526edb2a4 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -994,10 +994,10 @@ static void
partition_and_rebuild_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
struct sched_domain_attr *dattr_new)
{
- mutex_lock(&sched_domains_mutex);
+ sched_domains_mutex_lock();
partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
dl_rebuild_rd_accounting();
- mutex_unlock(&sched_domains_mutex);
+ sched_domains_mutex_unlock();
}
/*
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 67189907214d..58593f4d09a1 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8424,9 +8424,9 @@ void __init sched_init_smp(void)
* CPU masks are stable and all blatant races in the below code cannot
* happen.
*/
- mutex_lock(&sched_domains_mutex);
+ sched_domains_mutex_lock();
sched_init_domains(cpu_active_mask);
- mutex_unlock(&sched_domains_mutex);
+ sched_domains_mutex_unlock();
/* Move init over to a non-isolated CPU */
if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_TYPE_DOMAIN)) < 0)
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index ef047add7f9e..a0893a483d35 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -292,7 +292,7 @@ static ssize_t sched_verbose_write(struct file *filp, const char __user *ubuf,
bool orig;
cpus_read_lock();
- mutex_lock(&sched_domains_mutex);
+ sched_domains_mutex_lock();
orig = sched_debug_verbose;
result = debugfs_write_file_bool(filp, ubuf, cnt, ppos);
@@ -304,7 +304,7 @@ static ssize_t sched_verbose_write(struct file *filp, const char __user *ubuf,
sd_dentry = NULL;
}
- mutex_unlock(&sched_domains_mutex);
+ sched_domains_mutex_unlock();
cpus_read_unlock();
return result;
@@ -515,9 +515,9 @@ static __init int sched_init_debug(void)
debugfs_create_u32("migration_cost_ns", 0644, debugfs_sched, &sysctl_sched_migration_cost);
debugfs_create_u32("nr_migrate", 0644, debugfs_sched, &sysctl_sched_nr_migrate);
- mutex_lock(&sched_domains_mutex);
+ sched_domains_mutex_lock();
update_sched_domain_debugfs();
- mutex_unlock(&sched_domains_mutex);
+ sched_domains_mutex_unlock();
#endif
#ifdef CONFIG_NUMA_BALANCING
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index c49aea8c1025..296ff2acfd32 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -6,6 +6,14 @@
#include <linux/bsearch.h>
DEFINE_MUTEX(sched_domains_mutex);
+void sched_domains_mutex_lock(void)
+{
+ mutex_lock(&sched_domains_mutex);
+}
+void sched_domains_mutex_unlock(void)
+{
+ mutex_unlock(&sched_domains_mutex);
+}
/* Protected by sched_domains_mutex: */
static cpumask_var_t sched_domains_tmpmask;
@@ -2791,7 +2799,7 @@ 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)
{
- mutex_lock(&sched_domains_mutex);
+ sched_domains_mutex_lock();
partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
- mutex_unlock(&sched_domains_mutex);
+ sched_domains_mutex_unlock();
}
--
2.48.1On 3/6/25 9:10 AM, Juri Lelli wrote:
> Create wrappers for sched_domains_mutex so that it can transparently be
> used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
> do.
>
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow earlier for hotplug")
> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> ---
> v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
> either in that case
> ---
> include/linux/sched.h | 2 ++
> kernel/cgroup/cpuset.c | 4 ++--
> kernel/sched/core.c | 4 ++--
> kernel/sched/debug.c | 8 ++++----
> kernel/sched/topology.c | 12 ++++++++++--
> 5 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 9632e3318e0d..d5f8c161d852 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -383,6 +383,8 @@ enum uclamp_id {
> extern struct root_domain def_root_domain;
> extern struct mutex sched_domains_mutex;
> #endif
> +extern void sched_domains_mutex_lock(void);
> +extern void sched_domains_mutex_unlock(void);
As discussed in the other thread, move the
sched_domains_mutex_{lock/unlock}{} inside the "#if CONFIG_SMP" block
and define the else part so that it can be used in code block that will
also be compiled in the !CONFIG_SMP case.
Other than that, the rest looks good to me.
Cheers,
Longman
On 3/7/25 10:11 AM, Waiman Long wrote:
> On 3/6/25 9:10 AM, Juri Lelli wrote:
>> Create wrappers for sched_domains_mutex so that it can transparently be
>> used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
>> do.
>>
>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>> Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow
>> earlier for hotplug")
>> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
>> ---
>> v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
>> either in that case
>> ---
>> include/linux/sched.h | 2 ++
>> kernel/cgroup/cpuset.c | 4 ++--
>> kernel/sched/core.c | 4 ++--
>> kernel/sched/debug.c | 8 ++++----
>> kernel/sched/topology.c | 12 ++++++++++--
>> 5 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 9632e3318e0d..d5f8c161d852 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -383,6 +383,8 @@ enum uclamp_id {
>> extern struct root_domain def_root_domain;
>> extern struct mutex sched_domains_mutex;
>> #endif
>> +extern void sched_domains_mutex_lock(void);
>> +extern void sched_domains_mutex_unlock(void);
>
> As discussed in the other thread, move the
> sched_domains_mutex_{lock/unlock}{} inside the "#if CONFIG_SMP" block
> and define the else part so that it can be used in code block that
> will also be compiled in the !CONFIG_SMP case.
>
> Other than that, the rest looks good to me.
Actually, you can also remove sched_domains_mutex from the header and
make it static as it is no longer directly accessed.
Cheers,
Longman
On 07/03/25 10:19, Waiman Long wrote:
>
> On 3/7/25 10:11 AM, Waiman Long wrote:
> > On 3/6/25 9:10 AM, Juri Lelli wrote:
> > > Create wrappers for sched_domains_mutex so that it can transparently be
> > > used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
> > > do.
> > >
> > > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > > Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow
> > > earlier for hotplug")
> > > Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> > > ---
> > > v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
> > > either in that case
> > > ---
> > > include/linux/sched.h | 2 ++
> > > kernel/cgroup/cpuset.c | 4 ++--
> > > kernel/sched/core.c | 4 ++--
> > > kernel/sched/debug.c | 8 ++++----
> > > kernel/sched/topology.c | 12 ++++++++++--
> > > 5 files changed, 20 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > index 9632e3318e0d..d5f8c161d852 100644
> > > --- a/include/linux/sched.h
> > > +++ b/include/linux/sched.h
> > > @@ -383,6 +383,8 @@ enum uclamp_id {
> > > extern struct root_domain def_root_domain;
> > > extern struct mutex sched_domains_mutex;
> > > #endif
> > > +extern void sched_domains_mutex_lock(void);
> > > +extern void sched_domains_mutex_unlock(void);
> >
> > As discussed in the other thread, move the
> > sched_domains_mutex_{lock/unlock}{} inside the "#if CONFIG_SMP" block
> > and define the else part so that it can be used in code block that will
> > also be compiled in the !CONFIG_SMP case.
> >
> > Other than that, the rest looks good to me.
>
> Actually, you can also remove sched_domains_mutex from the header and make
> it static as it is no longer directly accessed.
Apart from a lockdep_assert_held() in cpuset.c, no? Guess I can create a
wrapper for that, but is it really better?
Thanks,
Juri
On 3/7/25 10:59 AM, Juri Lelli wrote:
> On 07/03/25 10:19, Waiman Long wrote:
>> On 3/7/25 10:11 AM, Waiman Long wrote:
>>> On 3/6/25 9:10 AM, Juri Lelli wrote:
>>>> Create wrappers for sched_domains_mutex so that it can transparently be
>>>> used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
>>>> do.
>>>>
>>>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>>>> Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow
>>>> earlier for hotplug")
>>>> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
>>>> ---
>>>> v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
>>>> either in that case
>>>> ---
>>>> include/linux/sched.h | 2 ++
>>>> kernel/cgroup/cpuset.c | 4 ++--
>>>> kernel/sched/core.c | 4 ++--
>>>> kernel/sched/debug.c | 8 ++++----
>>>> kernel/sched/topology.c | 12 ++++++++++--
>>>> 5 files changed, 20 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>> index 9632e3318e0d..d5f8c161d852 100644
>>>> --- a/include/linux/sched.h
>>>> +++ b/include/linux/sched.h
>>>> @@ -383,6 +383,8 @@ enum uclamp_id {
>>>> extern struct root_domain def_root_domain;
>>>> extern struct mutex sched_domains_mutex;
>>>> #endif
>>>> +extern void sched_domains_mutex_lock(void);
>>>> +extern void sched_domains_mutex_unlock(void);
>>> As discussed in the other thread, move the
>>> sched_domains_mutex_{lock/unlock}{} inside the "#if CONFIG_SMP" block
>>> and define the else part so that it can be used in code block that will
>>> also be compiled in the !CONFIG_SMP case.
>>>
>>> Other than that, the rest looks good to me.
>> Actually, you can also remove sched_domains_mutex from the header and make
>> it static as it is no longer directly accessed.
> Apart from a lockdep_assert_held() in cpuset.c, no? Guess I can create a
> wrapper for that, but is it really better?
I forgot about that. Please ignore this comment.
Thanks,
Longman
On 07/03/25 10:11, Waiman Long wrote:
> On 3/6/25 9:10 AM, Juri Lelli wrote:
> > Create wrappers for sched_domains_mutex so that it can transparently be
> > used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
> > do.
> >
> > Reported-by: Jon Hunter <jonathanh@nvidia.com>
> > Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow earlier for hotplug")
> > Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> > ---
> > v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
> > either in that case
> > ---
> > include/linux/sched.h | 2 ++
> > kernel/cgroup/cpuset.c | 4 ++--
> > kernel/sched/core.c | 4 ++--
> > kernel/sched/debug.c | 8 ++++----
> > kernel/sched/topology.c | 12 ++++++++++--
> > 5 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 9632e3318e0d..d5f8c161d852 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -383,6 +383,8 @@ enum uclamp_id {
> > extern struct root_domain def_root_domain;
> > extern struct mutex sched_domains_mutex;
> > #endif
> > +extern void sched_domains_mutex_lock(void);
> > +extern void sched_domains_mutex_unlock(void);
>
> As discussed in the other thread, move the
> sched_domains_mutex_{lock/unlock}{} inside the "#if CONFIG_SMP" block and
> define the else part so that it can be used in code block that will also be
> compiled in the !CONFIG_SMP case.
Ack.
> Other than that, the rest looks good to me.
Thanks! I will be sending out a v3 early next week (waiting a few more
hours in case anyone spots something else that needs rework).
Best,
Juri
Hi Juri.
On 3/6/25 19:40, Juri Lelli wrote:
> Create wrappers for sched_domains_mutex so that it can transparently be
> used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to
> do.
>
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Fixes: 53916d5fd3c0 ("sched/deadline: Check bandwidth overflow earlier for hotplug")
> Signed-off-by: Juri Lelli <juri.lelli@redhat.com>
> ---
> v1 -> v2: Remove wrappers for the !SMP case as all users are not defined
> either in that case
> ---
> include/linux/sched.h | 2 ++
> kernel/cgroup/cpuset.c | 4 ++--
> kernel/sched/core.c | 4 ++--
> kernel/sched/debug.c | 8 ++++----
> kernel/sched/topology.c | 12 ++++++++++--
> 5 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 9632e3318e0d..d5f8c161d852 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -383,6 +383,8 @@ enum uclamp_id {
> extern struct root_domain def_root_domain;
> extern struct mutex sched_domains_mutex;
> #endif
> +extern void sched_domains_mutex_lock(void);
> +extern void sched_domains_mutex_unlock(void);
>
> struct sched_param {
> int sched_priority;
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0f910c828973..f87526edb2a4 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -994,10 +994,10 @@ static void
> partition_and_rebuild_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
> struct sched_domain_attr *dattr_new)
> {
> - mutex_lock(&sched_domains_mutex);
> + sched_domains_mutex_lock();
> partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
> dl_rebuild_rd_accounting();
> - mutex_unlock(&sched_domains_mutex);
> + sched_domains_mutex_unlock();
> }
>
> /*
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 67189907214d..58593f4d09a1 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8424,9 +8424,9 @@ void __init sched_init_smp(void)
> * CPU masks are stable and all blatant races in the below code cannot
> * happen.
> */
> - mutex_lock(&sched_domains_mutex);
> + sched_domains_mutex_lock();
> sched_init_domains(cpu_active_mask);
> - mutex_unlock(&sched_domains_mutex);
> + sched_domains_mutex_unlock();
>
> /* Move init over to a non-isolated CPU */
> if (set_cpus_allowed_ptr(current, housekeeping_cpumask(HK_TYPE_DOMAIN)) < 0)
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index ef047add7f9e..a0893a483d35 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -292,7 +292,7 @@ static ssize_t sched_verbose_write(struct file *filp, const char __user *ubuf,
> bool orig;
>
> cpus_read_lock();
> - mutex_lock(&sched_domains_mutex);
> + sched_domains_mutex_lock();
>
> orig = sched_debug_verbose;
> result = debugfs_write_file_bool(filp, ubuf, cnt, ppos);
> @@ -304,7 +304,7 @@ static ssize_t sched_verbose_write(struct file *filp, const char __user *ubuf,
> sd_dentry = NULL;
> }
>
> - mutex_unlock(&sched_domains_mutex);
> + sched_domains_mutex_unlock();
> cpus_read_unlock();
>
> return result;
> @@ -515,9 +515,9 @@ static __init int sched_init_debug(void)
> debugfs_create_u32("migration_cost_ns", 0644, debugfs_sched, &sysctl_sched_migration_cost);
> debugfs_create_u32("nr_migrate", 0644, debugfs_sched, &sysctl_sched_nr_migrate);
>
> - mutex_lock(&sched_domains_mutex);
> + sched_domains_mutex_lock();
> update_sched_domain_debugfs();
> - mutex_unlock(&sched_domains_mutex);
> + sched_domains_mutex_unlock();
> #endif
>
> #ifdef CONFIG_NUMA_BALANCING
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index c49aea8c1025..296ff2acfd32 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -6,6 +6,14 @@
> #include <linux/bsearch.h>
>
> DEFINE_MUTEX(sched_domains_mutex);
> +void sched_domains_mutex_lock(void)
> +{
> + mutex_lock(&sched_domains_mutex);
> +}
> +void sched_domains_mutex_unlock(void)
> +{
> + mutex_unlock(&sched_domains_mutex);
> +}
>
> /* Protected by sched_domains_mutex: */
> static cpumask_var_t sched_domains_tmpmask;
> @@ -2791,7 +2799,7 @@ 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)
> {
> - mutex_lock(&sched_domains_mutex);
> + sched_domains_mutex_lock();
> partition_sched_domains_locked(ndoms_new, doms_new, dattr_new);
> - mutex_unlock(&sched_domains_mutex);
> + sched_domains_mutex_unlock();
> }
Maybe sched_domains_mutex can be static since its only used in topology.c ?
Other than the above
Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
Hi, Thanks for the overall review! On 07/03/25 12:04, Shrikanth Hegde wrote: > Hi Juri. > > On 3/6/25 19:40, Juri Lelli wrote: > > Create wrappers for sched_domains_mutex so that it can transparently be > > used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to > > do. ... > Maybe sched_domains_mutex can be static since its only used in topology.c ? We have a lockdep_assert in cpuset.c, don't we? We can create another wrapper for that, but I am not sure it is going to be cleaner. Thanks, Juri
On 3/7/25 14:23, Juri Lelli wrote: > Hi, > > Thanks for the overall review! > > On 07/03/25 12:04, Shrikanth Hegde wrote: >> Hi Juri. >> >> On 3/6/25 19:40, Juri Lelli wrote: >>> Create wrappers for sched_domains_mutex so that it can transparently be >>> used on both CONFIG_SMP and !CONFIG_SMP, as some function will need to >>> do. > > ... > >> Maybe sched_domains_mutex can be static since its only used in topology.c ? > > We have a lockdep_assert in cpuset.c, don't we? We can create another > wrapper for that, but I am not sure it is going to be cleaner. Ok. Sorry, my bad. I searched only in kernel/sched. You can ignore this comment. > > Thanks, > Juri >
© 2016 - 2026 Red Hat, Inc.