cgroup/cpuset: Miscellaneous fixes and cleanup

[PATCH 1/3] cgroup/cpuset: Use static_branch_enable_cpuslocked() on cpusets_insane_config_key

Posted by Waiman Long 1 month, 4 weeks ago

The following lockdep splat was observed.

[  812.359086] ============================================
[  812.359089] WARNING: possible recursive locking detected
[  812.359097] --------------------------------------------
[  812.359100] runtest.sh/30042 is trying to acquire lock:
[  812.359105] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: static_key_enable+0xe/0x20
[  812.359131]
[  812.359131] but task is already holding lock:
[  812.359134] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: cpuset_write_resmask+0x98/0xa70
     :
[  812.359267] Call Trace:
[  812.359272]  <TASK>
[  812.359367]  cpus_read_lock+0x3c/0xe0
[  812.359382]  static_key_enable+0xe/0x20
[  812.359389]  check_insane_mems_config.part.0+0x11/0x30
[  812.359398]  cpuset_write_resmask+0x9f2/0xa70
[  812.359411]  cgroup_file_write+0x1c7/0x660
[  812.359467]  kernfs_fop_write_iter+0x358/0x530
[  812.359479]  vfs_write+0xabe/0x1250
[  812.359529]  ksys_write+0xf9/0x1d0
[  812.359558]  do_syscall_64+0x5f/0xe0

Since commit d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem
and hotplug lock order"), the ordering of cpu hotplug lock
and cpuset_mutex had been reversed. That patch correctly
used the cpuslocked version of the static branch API to enable
cpusets_pre_enable_key and cpusets_enabled_key, but it didn't do the
same for cpusets_insane_config_key.

The cpusets_insane_config_key can be enabled in the
check_insane_mems_config() which is called from update_nodemask()
or cpuset_hotplug_update_tasks() with both cpu hotplug lock and
cpuset_mutex held. Deadlock can happen with a pending hotplug event that
tries to acquire the cpu hotplug write lock which will block further
cpus_read_lock() attempt from check_insane_mems_config(). Fix that by
switching to use static_branch_enable_cpuslocked().

Fixes: d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem and hotplug lock order")
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index f74d04429a29..bf149246e001 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -280,7 +280,7 @@ static inline void check_insane_mems_config(nodemask_t *nodes)
 {
 	if (!cpusets_insane_config() &&
 		movable_only_nodes(nodes)) {
-		static_branch_enable(&cpusets_insane_config_key);
+		static_branch_enable_cpuslocked(&cpusets_insane_config_key);
 		pr_info("Unsupported (movable nodes only) cpuset configuration detected (nmask=%*pbl)!\n"
 			"Cpuset allocations might fail even with a lot of memory available.\n",
 			nodemask_pr_args(nodes));
-- 
2.50.0

Re: [PATCH 1/3] cgroup/cpuset: Use static_branch_enable_cpuslocked() on cpusets_insane_config_key

Posted by Juri Lelli 1 month, 4 weeks ago

Hi,

On 06/08/25 13:24, Waiman Long wrote:
> The following lockdep splat was observed.
> 
> [  812.359086] ============================================
> [  812.359089] WARNING: possible recursive locking detected
> [  812.359097] --------------------------------------------
> [  812.359100] runtest.sh/30042 is trying to acquire lock:
> [  812.359105] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: static_key_enable+0xe/0x20
> [  812.359131]
> [  812.359131] but task is already holding lock:
> [  812.359134] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: cpuset_write_resmask+0x98/0xa70
>      :
> [  812.359267] Call Trace:
> [  812.359272]  <TASK>
> [  812.359367]  cpus_read_lock+0x3c/0xe0
> [  812.359382]  static_key_enable+0xe/0x20
> [  812.359389]  check_insane_mems_config.part.0+0x11/0x30
> [  812.359398]  cpuset_write_resmask+0x9f2/0xa70
> [  812.359411]  cgroup_file_write+0x1c7/0x660
> [  812.359467]  kernfs_fop_write_iter+0x358/0x530
> [  812.359479]  vfs_write+0xabe/0x1250
> [  812.359529]  ksys_write+0xf9/0x1d0
> [  812.359558]  do_syscall_64+0x5f/0xe0
> 
> Since commit d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem
> and hotplug lock order"), the ordering of cpu hotplug lock
> and cpuset_mutex had been reversed. That patch correctly
> used the cpuslocked version of the static branch API to enable
> cpusets_pre_enable_key and cpusets_enabled_key, but it didn't do the
> same for cpusets_insane_config_key.
> 
> The cpusets_insane_config_key can be enabled in the
> check_insane_mems_config() which is called from update_nodemask()
> or cpuset_hotplug_update_tasks() with both cpu hotplug lock and
> cpuset_mutex held. Deadlock can happen with a pending hotplug event that
> tries to acquire the cpu hotplug write lock which will block further
> cpus_read_lock() attempt from check_insane_mems_config(). Fix that by
> switching to use static_branch_enable_cpuslocked().
> 
> Fixes: d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem and hotplug lock order")
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---

Looks good to me. Thanks for spotting and fixing this.

Reviewed-by: Juri Lelli <juri.lelli@redhat.com>

Best,
Juri

Re: [PATCH 1/3] cgroup/cpuset: Use static_branch_enable_cpuslocked() on cpusets_insane_config_key

Posted by Waiman Long 1 month, 4 weeks ago

On 8/7/25 9:15 AM, Juri Lelli wrote:
> Hi,
>
> On 06/08/25 13:24, Waiman Long wrote:
>> The following lockdep splat was observed.
>>
>> [  812.359086] ============================================
>> [  812.359089] WARNING: possible recursive locking detected
>> [  812.359097] --------------------------------------------
>> [  812.359100] runtest.sh/30042 is trying to acquire lock:
>> [  812.359105] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: static_key_enable+0xe/0x20
>> [  812.359131]
>> [  812.359131] but task is already holding lock:
>> [  812.359134] ffffffffa7f27420 (cpu_hotplug_lock){++++}-{0:0}, at: cpuset_write_resmask+0x98/0xa70
>>       :
>> [  812.359267] Call Trace:
>> [  812.359272]  <TASK>
>> [  812.359367]  cpus_read_lock+0x3c/0xe0
>> [  812.359382]  static_key_enable+0xe/0x20
>> [  812.359389]  check_insane_mems_config.part.0+0x11/0x30
>> [  812.359398]  cpuset_write_resmask+0x9f2/0xa70
>> [  812.359411]  cgroup_file_write+0x1c7/0x660
>> [  812.359467]  kernfs_fop_write_iter+0x358/0x530
>> [  812.359479]  vfs_write+0xabe/0x1250
>> [  812.359529]  ksys_write+0xf9/0x1d0
>> [  812.359558]  do_syscall_64+0x5f/0xe0
>>
>> Since commit d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem
>> and hotplug lock order"), the ordering of cpu hotplug lock
>> and cpuset_mutex had been reversed. That patch correctly
>> used the cpuslocked version of the static branch API to enable
>> cpusets_pre_enable_key and cpusets_enabled_key, but it didn't do the
>> same for cpusets_insane_config_key.
>>
>> The cpusets_insane_config_key can be enabled in the
>> check_insane_mems_config() which is called from update_nodemask()
>> or cpuset_hotplug_update_tasks() with both cpu hotplug lock and
>> cpuset_mutex held. Deadlock can happen with a pending hotplug event that
>> tries to acquire the cpu hotplug write lock which will block further
>> cpus_read_lock() attempt from check_insane_mems_config(). Fix that by
>> switching to use static_branch_enable_cpuslocked().
>>
>> Fixes: d74b27d63a8b ("cgroup/cpuset: Change cpuset_rwsem and hotplug lock order")
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
> Looks good to me. Thanks for spotting and fixing this.
>
> Reviewed-by: Juri Lelli <juri.lelli@redhat.com>

It is really a corner case that is not easy to trigger. I would have 
missed that myself.

Thanks,
Longman

>
> Best,
> Juri
>