regression 4.4: deadlock in with cgroup percpu_rwsem

Folks,

With 4.4 I can easily bring the system into a hang like situation by putting stress on the cgroup_threadgroup rwsem. (e.g. starting/stopping kvm guests via libvirt and many vCPUs). Here is my preliminary analysis:

When the hang happens, the system is idle for all CPUs. There are some processes waiting for the cgroup_thread_rwsem, e.g.

crash> bt 87399
PID: 87399 TASK: faef084998 CPU: 59 COMMAND: "systemd-udevd"
#0 [f9e762fc88] __schedule at 83b2cc
#1 [f9e762fcf0] schedule at 83ba26
#2 [f9e762fd08] rwsem_down_read_failed at 83fb64
#3 [f9e762fd68] percpu_down_read at 1bdf56
#4 [f9e762fdd0] exit_signals at 1742ae
#5 [f9e762fe00] do_exit at 163be0
#6 [f9e762fe60] do_group_exit at 165c62
#7 [f9e762fe90] __wake_up_parent at 165d00
#8 [f9e762fea8] system_call at 842386

of course, any new process would wait for the same lock during fork.

Looking at the rwsem, while all CPUs are idle, it appears that the lock is taken for write:

crash> print /x cgroup_threadgroup_rwsem.rw_sem
$8 = {
count = 0xfffffffe00000001,
[..]
owner = 0xfabf28c998,
}

Looking at the owner field:

crash> bt 0xfabf28c998
PID: 11867 TASK: fabf28c998 CPU: 42 COMMAND: "libvirtd"
#0 [fadeccb5e8] __schedule at 83b2cc
#1 [fadeccb650] schedule at 83ba26
#2 [fadeccb668] schedule_timeout at 8403c6 #3 [fadeccb748] wait_for_common at 83c850
#4 [fadeccb7b8] flush_work at 18064a
#5 [fadeccb8d8] lru_add_drain_all at 2abd10 #6 [fadeccb938] migrate_prep at 309ed2
#7 [fadeccb950] do_migrate_pages at 2f7644 #8 [fadeccb9f0] cpuset_migrate_mm at 220848 #9 [fadeccba58] cpuset_attach at 223248
#10 [fadeccbaa0] cgroup_taskset_migrate at 21a678 #11 [fadeccbaf8] cgroup_migrate at 21a942
#12 [fadeccbba0] cgroup_attach_task at 21ab8a #13 [fadeccbc18] __cgroup_procs_write at 21affa #14 [fadeccbc98] cgroup_file_write at 216be0 #15 [fadeccbd08] kernfs_fop_write at 3aa088 #16 [fadeccbd50] __vfs_write at 319782
#17 [fadeccbe08] vfs_write at 31a1ac
#18 [fadeccbe68] sys_write at 31af06
#19 [fadeccbea8] system_call at 842386
PSW: 0705100180000000 000003ff9438f9f0 (user space)

it appears that the write holder scheduled away and waits for a completion. Now what happens is, that the write lock holder finally calls flush_work for the lru_add_drain_all work.

As far as I can see, this work is now tries to create a new kthread and waits for that, as the backtrace for the kworker on that cpu has:

PID: 81913 TASK: fab5356220 CPU: 42 COMMAND: "kworker/42:2"
#0 [fadd6d7998] __schedule at 83b2cc
#1 [fadd6d7a00] schedule at 83ba26
#2 [fadd6d7a18] schedule_timeout at 8403c6
#3 [fadd6d7af8] wait_for_common at 83c850
#4 [fadd6d7b68] wait_for_completion_killable at 83c996 #5 [fadd6d7b88] kthread_create_on_node at 1876a4 #6 [fadd6d7cc0] create_worker at 17d7fa
#7 [fadd6d7d30] worker_thread at 17fff0
#8 [fadd6d7da0] kthread at 187884
#9 [fadd6d7ea8] kernel_thread_starter at 842552

Problem is that kthreadd then needs the cgroup lock for reading, while libvirtd still has the lock for writing.

crash> bt 0xfaf031e220
PID: 2 TASK: faf031e220 CPU: 40 COMMAND: "kthreadd"
#0 [faf034bad8] __schedule at 83b2cc
#1 [faf034bb40] schedule at 83ba26
#2 [faf034bb58] rwsem_down_read_failed at 83fb64
#3 [faf034bbb8] percpu_down_read at 1bdf56
#4 [faf034bc20] copy_process at 15eab6
#5 [faf034bd08] _do_fork at 160430
#6 [faf034bdd0] kernel_thread at 160a82
#7 [faf034be30] kthreadd at 188580
#8 [faf034bea8] kernel_thread_starter at 842552

BANG.kthreadd waits for the lock that libvirtd hold, and libvirtd waits for kthreadd to finish some task

Reverting 001dac627ff374 ("locking/percpu-rwsem: Make use of the rcu_sync infrastructure") does not help, so it does not seem to be related to the rcu_sync rework.

Any ideas, questions (dump is still available)

PS: not sure if lockdep could detect such a situation. it is running but silent.

Christian

Folks,

With 4.4 I can easily bring the system into a hang like situation by putting stress on the cgroup_threadgroup rwsem. (e.g. starting/stopping kvm guests via libvirt and many vCPUs). Here is my preliminary analysis:

When the hang happens, the system is idle for all CPUs. There are some processes waiting for the cgroup_thread_rwsem, e.g.

crash> bt 87399
PID: 87399 TASK: faef084998 CPU: 59 COMMAND: "systemd-udevd"
#0 [f9e762fc88] __schedule at 83b2cc
#1 [f9e762fcf0] schedule at 83ba26
#2 [f9e762fd08] rwsem_down_read_failed at 83fb64
#3 [f9e762fd68] percpu_down_read at 1bdf56
#4 [f9e762fdd0] exit_signals at 1742ae
#5 [f9e762fe00] do_exit at 163be0
#6 [f9e762fe60] do_group_exit at 165c62
#7 [f9e762fe90] __wake_up_parent at 165d00
#8 [f9e762fea8] system_call at 842386

of course, any new process would wait for the same lock during fork.

Looking at the rwsem, while all CPUs are idle, it appears that the lock is taken for write:

crash> print /x cgroup_threadgroup_rwsem.rw_sem
$8 = {
count = 0xfffffffe00000001,
[..]
owner = 0xfabf28c998,
}

Looking at the owner field:

crash> bt 0xfabf28c998
PID: 11867 TASK: fabf28c998 CPU: 42 COMMAND: "libvirtd"
#0 [fadeccb5e8] __schedule at 83b2cc
#1 [fadeccb650] schedule at 83ba26
#2 [fadeccb668] schedule_timeout at 8403c6 #3 [fadeccb748] wait_for_common at 83c850
#4 [fadeccb7b8] flush_work at 18064a
#5 [fadeccb8d8] lru_add_drain_all at 2abd10 #6 [fadeccb938] migrate_prep at 309ed2
#7 [fadeccb950] do_migrate_pages at 2f7644 #8 [fadeccb9f0] cpuset_migrate_mm at 220848 #9 [fadeccba58] cpuset_attach at 223248
#10 [fadeccbaa0] cgroup_taskset_migrate at 21a678 #11 [fadeccbaf8] cgroup_migrate at 21a942
#12 [fadeccbba0] cgroup_attach_task at 21ab8a #13 [fadeccbc18] __cgroup_procs_write at 21affa #14 [fadeccbc98] cgroup_file_write at 216be0 #15 [fadeccbd08] kernfs_fop_write at 3aa088 #16 [fadeccbd50] __vfs_write at 319782
#17 [fadeccbe08] vfs_write at 31a1ac
#18 [fadeccbe68] sys_write at 31af06
#19 [fadeccbea8] system_call at 842386
PSW: 0705100180000000 000003ff9438f9f0 (user space)

it appears that the write holder scheduled away and waits for a completion. Now what happens is, that the write lock holder finally calls flush_work for the lru_add_drain_all work.

As far as I can see, this work is now tries to create a new kthread and waits for that, as the backtrace for the kworker on that cpu has:

PID: 81913 TASK: fab5356220 CPU: 42 COMMAND: "kworker/42:2"
#0 [fadd6d7998] __schedule at 83b2cc
#1 [fadd6d7a00] schedule at 83ba26
#2 [fadd6d7a18] schedule_timeout at 8403c6
#3 [fadd6d7af8] wait_for_common at 83c850
#4 [fadd6d7b68] wait_for_completion_killable at 83c996 #5 [fadd6d7b88] kthread_create_on_node at 1876a4 #6 [fadd6d7cc0] create_worker at 17d7fa
#7 [fadd6d7d30] worker_thread at 17fff0
#8 [fadd6d7da0] kthread at 187884
#9 [fadd6d7ea8] kernel_thread_starter at 842552

Problem is that kthreadd then needs the cgroup lock for reading, while libvirtd still has the lock for writing.

crash> bt 0xfaf031e220
PID: 2 TASK: faf031e220 CPU: 40 COMMAND: "kthreadd"
#0 [faf034bad8] __schedule at 83b2cc
#1 [faf034bb40] schedule at 83ba26
#2 [faf034bb58] rwsem_down_read_failed at 83fb64
#3 [faf034bbb8] percpu_down_read at 1bdf56
#4 [faf034bc20] copy_process at 15eab6
#5 [faf034bd08] _do_fork at 160430
#6 [faf034bdd0] kernel_thread at 160a82
#7 [faf034be30] kthreadd at 188580
#8 [faf034bea8] kernel_thread_starter at 842552

BANG.kthreadd waits for the lock that libvirtd hold, and libvirtd waits for kthreadd to finish some task

Reverting 001dac627ff374 ("locking/percpu-rwsem: Make use of the rcu_sync infrastructure") does not help, so it does not seem to be related to the rcu_sync rework.

Any ideas, questions (dump is still available)

PS: not sure if lockdep could detect such a situation. it is running but silent.

Christian

Quoted 59 lines · Christian Borntraeger

Folks,

With 4.4 I can easily bring the system into a hang like situation by putting stress on the cgroup_threadgroup rwsem. (e.g. starting/stopping kvm guests via libvirt and many vCPUs). Here is my preliminary analysis:

When the hang happens, the system is idle for all CPUs. There are some processes waiting for the cgroup_thread_rwsem, e.g.

crash> bt 87399
PID: 87399 TASK: faef084998 CPU: 59 COMMAND: "systemd-udevd"
#0 [f9e762fc88] __schedule at 83b2cc
#1 [f9e762fcf0] schedule at 83ba26
#2 [f9e762fd08] rwsem_down_read_failed at 83fb64
#3 [f9e762fd68] percpu_down_read at 1bdf56
#4 [f9e762fdd0] exit_signals at 1742ae
#5 [f9e762fe00] do_exit at 163be0
#6 [f9e762fe60] do_group_exit at 165c62
#7 [f9e762fe90] __wake_up_parent at 165d00
#8 [f9e762fea8] system_call at 842386

of course, any new process would wait for the same lock during fork.

Looking at the rwsem, while all CPUs are idle, it appears that the lock is taken for write:

crash> print /x cgroup_threadgroup_rwsem.rw_sem
$8 = {
count = 0xfffffffe00000001,
[..]
owner = 0xfabf28c998,
}

Looking at the owner field:

crash> bt 0xfabf28c998
PID: 11867 TASK: fabf28c998 CPU: 42 COMMAND: "libvirtd"
#0 [fadeccb5e8] __schedule at 83b2cc
#1 [fadeccb650] schedule at 83ba26
#2 [fadeccb668] schedule_timeout at 8403c6 #3 [fadeccb748] wait_for_common at 83c850
#4 [fadeccb7b8] flush_work at 18064a
#5 [fadeccb8d8] lru_add_drain_all at 2abd10 #6 [fadeccb938] migrate_prep at 309ed2
#7 [fadeccb950] do_migrate_pages at 2f7644 #8 [fadeccb9f0] cpuset_migrate_mm at 220848 #9 [fadeccba58] cpuset_attach at 223248
#10 [fadeccbaa0] cgroup_taskset_migrate at 21a678 #11 [fadeccbaf8] cgroup_migrate at 21a942
#12 [fadeccbba0] cgroup_attach_task at 21ab8a #13 [fadeccbc18] __cgroup_procs_write at 21affa #14 [fadeccbc98] cgroup_file_write at 216be0 #15 [fadeccbd08] kernfs_fop_write at 3aa088 #16 [fadeccbd50] __vfs_write at 319782
#17 [fadeccbe08] vfs_write at 31a1ac
#18 [fadeccbe68] sys_write at 31af06
#19 [fadeccbea8] system_call at 842386
PSW: 0705100180000000 000003ff9438f9f0 (user space)

Quoted 4 lines

it appears that the write holder scheduled away and waits for a completion. Now what happens is, that the write lock holder finally calls flush_work for the lru_add_drain_all work.

So what's happening is that libvirtd wants to move some processes in the cgroup subtree and it to the respective cgroup file. So cgroup_threadgroup_rwsem is acquired in __cgroup_procs_write, then as part of this process the pages for that process have to be migrated, hence the do_migrate_pages. And this call chain boils down to calling lru_add_drain_cpu on every cpu.

Quoted 31 lines

As far as I can see, this work is now tries to create a new kthread and waits for that, as the backtrace for the kworker on that cpu has:

PID: 81913 TASK: fab5356220 CPU: 42 COMMAND: "kworker/42:2"
#0 [fadd6d7998] __schedule at 83b2cc
#1 [fadd6d7a00] schedule at 83ba26
#2 [fadd6d7a18] schedule_timeout at 8403c6
#3 [fadd6d7af8] wait_for_common at 83c850
#4 [fadd6d7b68] wait_for_completion_killable at 83c996 #5 [fadd6d7b88] kthread_create_on_node at 1876a4 #6 [fadd6d7cc0] create_worker at 17d7fa
#7 [fadd6d7d30] worker_thread at 17fff0
#8 [fadd6d7da0] kthread at 187884
#9 [fadd6d7ea8] kernel_thread_starter at 842552

Problem is that kthreadd then needs the cgroup lock for reading, while libvirtd still has the lock for writing.

crash> bt 0xfaf031e220
PID: 2 TASK: faf031e220 CPU: 40 COMMAND: "kthreadd"
#0 [faf034bad8] __schedule at 83b2cc
#1 [faf034bb40] schedule at 83ba26
#2 [faf034bb58] rwsem_down_read_failed at 83fb64
#3 [faf034bbb8] percpu_down_read at 1bdf56
#4 [faf034bc20] copy_process at 15eab6
#5 [faf034bd08] _do_fork at 160430
#6 [faf034bdd0] kernel_thread at 160a82
#7 [faf034be30] kthreadd at 188580
#8 [faf034bea8] kernel_thread_starter at 842552

Quoted 2 lines

BANG.kthreadd waits for the lock that libvirtd hold, and libvirtd waits for kthreadd to finish some task

I don't see percpu_down_read being invoked from copy_process. According to LXR, this semaphore is used only in __cgroup_procs_write and cgroup_update_dfl_csses. And cgroup_update_dfl_csses is invoked when cgroup.subtree_control is written to. And I don't see this happening in this call chain.

Quoted 106 lines · Nikolay Borisov

Quoted 59 lines · Christian Borntraeger

Folks,

With 4.4 I can easily bring the system into a hang like situation by putting stress on the cgroup_threadgroup rwsem. (e.g. starting/stopping kvm guests via libvirt and many vCPUs). Here is my preliminary analysis:

When the hang happens, the system is idle for all CPUs. There are some processes waiting for the cgroup_thread_rwsem, e.g.

crash> bt 87399
PID: 87399 TASK: faef084998 CPU: 59 COMMAND: "systemd-udevd"
#0 [f9e762fc88] __schedule at 83b2cc
#1 [f9e762fcf0] schedule at 83ba26
#2 [f9e762fd08] rwsem_down_read_failed at 83fb64
#3 [f9e762fd68] percpu_down_read at 1bdf56
#4 [f9e762fdd0] exit_signals at 1742ae
#5 [f9e762fe00] do_exit at 163be0
#6 [f9e762fe60] do_group_exit at 165c62
#7 [f9e762fe90] __wake_up_parent at 165d00
#8 [f9e762fea8] system_call at 842386

of course, any new process would wait for the same lock during fork.

Looking at the rwsem, while all CPUs are idle, it appears that the lock is taken for write:

crash> print /x cgroup_threadgroup_rwsem.rw_sem
$8 = {
count = 0xfffffffe00000001,
[..]
owner = 0xfabf28c998,
}

Looking at the owner field:

crash> bt 0xfabf28c998
PID: 11867 TASK: fabf28c998 CPU: 42 COMMAND: "libvirtd"
#0 [fadeccb5e8] __schedule at 83b2cc
#1 [fadeccb650] schedule at 83ba26
#2 [fadeccb668] schedule_timeout at 8403c6 #3 [fadeccb748] wait_for_common at 83c850
#4 [fadeccb7b8] flush_work at 18064a
#5 [fadeccb8d8] lru_add_drain_all at 2abd10 #6 [fadeccb938] migrate_prep at 309ed2
#7 [fadeccb950] do_migrate_pages at 2f7644 #8 [fadeccb9f0] cpuset_migrate_mm at 220848 #9 [fadeccba58] cpuset_attach at 223248
#10 [fadeccbaa0] cgroup_taskset_migrate at 21a678 #11 [fadeccbaf8] cgroup_migrate at 21a942
#12 [fadeccbba0] cgroup_attach_task at 21ab8a #13 [fadeccbc18] __cgroup_procs_write at 21affa #14 [fadeccbc98] cgroup_file_write at 216be0 #15 [fadeccbd08] kernfs_fop_write at 3aa088 #16 [fadeccbd50] __vfs_write at 319782
#17 [fadeccbe08] vfs_write at 31a1ac
#18 [fadeccbe68] sys_write at 31af06
#19 [fadeccbea8] system_call at 842386
PSW: 0705100180000000 000003ff9438f9f0 (user space)

Quoted 4 lines

it appears that the write holder scheduled away and waits for a completion. Now what happens is, that the write lock holder finally calls flush_work for the lru_add_drain_all work.

So what's happening is that libvirtd wants to move some processes in the cgroup subtree and it to the respective cgroup file. So cgroup_threadgroup_rwsem is acquired in __cgroup_procs_write, then as part of this process the pages for that process have to be migrated, hence the do_migrate_pages. And this call chain boils down to calling lru_add_drain_cpu on every cpu.

Quoted 31 lines

As far as I can see, this work is now tries to create a new kthread and waits for that, as the backtrace for the kworker on that cpu has:

PID: 81913 TASK: fab5356220 CPU: 42 COMMAND: "kworker/42:2"
#0 [fadd6d7998] __schedule at 83b2cc
#1 [fadd6d7a00] schedule at 83ba26
#2 [fadd6d7a18] schedule_timeout at 8403c6
#3 [fadd6d7af8] wait_for_common at 83c850
#4 [fadd6d7b68] wait_for_completion_killable at 83c996 #5 [fadd6d7b88] kthread_create_on_node at 1876a4 #6 [fadd6d7cc0] create_worker at 17d7fa
#7 [fadd6d7d30] worker_thread at 17fff0
#8 [fadd6d7da0] kthread at 187884
#9 [fadd6d7ea8] kernel_thread_starter at 842552

Problem is that kthreadd then needs the cgroup lock for reading, while libvirtd still has the lock for writing.

crash> bt 0xfaf031e220
PID: 2 TASK: faf031e220 CPU: 40 COMMAND: "kthreadd"
#0 [faf034bad8] __schedule at 83b2cc
#1 [faf034bb40] schedule at 83ba26
#2 [faf034bb58] rwsem_down_read_failed at 83fb64
#3 [faf034bbb8] percpu_down_read at 1bdf56
#4 [faf034bc20] copy_process at 15eab6
#5 [faf034bd08] _do_fork at 160430
#6 [faf034bdd0] kernel_thread at 160a82
#7 [faf034be30] kthreadd at 188580
#8 [faf034bea8] kernel_thread_starter at 842552

Quoted 8 lines

Quoted 2 lines

BANG.kthreadd waits for the lock that libvirtd hold, and libvirtd waits for kthreadd to finish some task

I don't see percpu_down_read being invoked from copy_process. According to LXR, this semaphore is used only in __cgroup_procs_write and cgroup_update_dfl_csses. And cgroup_update_dfl_csses is invoked when cgroup.subtree_control is written to. And I don't see this happening in this call chain.

The callchain is inlined and as follows:

_do_fork
copy_process
threadgroup_change_begin
cgroup_threadgroup_change_begin

Quoted 116 lines · Christian Borntraeger

Quoted 106 lines · Nikolay Borisov

Quoted 59 lines · Christian Borntraeger

Folks,

With 4.4 I can easily bring the system into a hang like situation by putting stress on the cgroup_threadgroup rwsem. (e.g. starting/stopping kvm guests via libvirt and many vCPUs). Here is my preliminary analysis:

When the hang happens, the system is idle for all CPUs. There are some processes waiting for the cgroup_thread_rwsem, e.g.

crash> bt 87399
PID: 87399 TASK: faef084998 CPU: 59 COMMAND: "systemd-udevd"
#0 [f9e762fc88] __schedule at 83b2cc
#1 [f9e762fcf0] schedule at 83ba26
#2 [f9e762fd08] rwsem_down_read_failed at 83fb64
#3 [f9e762fd68] percpu_down_read at 1bdf56
#4 [f9e762fdd0] exit_signals at 1742ae
#5 [f9e762fe00] do_exit at 163be0
#6 [f9e762fe60] do_group_exit at 165c62
#7 [f9e762fe90] __wake_up_parent at 165d00
#8 [f9e762fea8] system_call at 842386

of course, any new process would wait for the same lock during fork.

Looking at the rwsem, while all CPUs are idle, it appears that the lock is taken for write:

crash> print /x cgroup_threadgroup_rwsem.rw_sem
$8 = {
count = 0xfffffffe00000001,
[..]
owner = 0xfabf28c998,
}

Looking at the owner field:

crash> bt 0xfabf28c998
PID: 11867 TASK: fabf28c998 CPU: 42 COMMAND: "libvirtd"
#0 [fadeccb5e8] __schedule at 83b2cc
#1 [fadeccb650] schedule at 83ba26
#2 [fadeccb668] schedule_timeout at 8403c6 #3 [fadeccb748] wait_for_common at 83c850
#4 [fadeccb7b8] flush_work at 18064a
#5 [fadeccb8d8] lru_add_drain_all at 2abd10 #6 [fadeccb938] migrate_prep at 309ed2
#7 [fadeccb950] do_migrate_pages at 2f7644 #8 [fadeccb9f0] cpuset_migrate_mm at 220848 #9 [fadeccba58] cpuset_attach at 223248
#10 [fadeccbaa0] cgroup_taskset_migrate at 21a678 #11 [fadeccbaf8] cgroup_migrate at 21a942
#12 [fadeccbba0] cgroup_attach_task at 21ab8a #13 [fadeccbc18] __cgroup_procs_write at 21affa #14 [fadeccbc98] cgroup_file_write at 216be0 #15 [fadeccbd08] kernfs_fop_write at 3aa088 #16 [fadeccbd50] __vfs_write at 319782
#17 [fadeccbe08] vfs_write at 31a1ac
#18 [fadeccbe68] sys_write at 31af06
#19 [fadeccbea8] system_call at 842386
PSW: 0705100180000000 000003ff9438f9f0 (user space)

Quoted 4 lines

it appears that the write holder scheduled away and waits for a completion. Now what happens is, that the write lock holder finally calls flush_work for the lru_add_drain_all work.

So what's happening is that libvirtd wants to move some processes in the cgroup subtree and it to the respective cgroup file. So cgroup_threadgroup_rwsem is acquired in __cgroup_procs_write, then as part of this process the pages for that process have to be migrated, hence the do_migrate_pages. And this call chain boils down to calling lru_add_drain_cpu on every cpu.

Quoted 31 lines

As far as I can see, this work is now tries to create a new kthread and waits for that, as the backtrace for the kworker on that cpu has:

PID: 81913 TASK: fab5356220 CPU: 42 COMMAND: "kworker/42:2"
#0 [fadd6d7998] __schedule at 83b2cc
#1 [fadd6d7a00] schedule at 83ba26
#2 [fadd6d7a18] schedule_timeout at 8403c6
#3 [fadd6d7af8] wait_for_common at 83c850
#4 [fadd6d7b68] wait_for_completion_killable at 83c996 #5 [fadd6d7b88] kthread_create_on_node at 1876a4 #6 [fadd6d7cc0] create_worker at 17d7fa
#7 [fadd6d7d30] worker_thread at 17fff0
#8 [fadd6d7da0] kthread at 187884
#9 [fadd6d7ea8] kernel_thread_starter at 842552

Problem is that kthreadd then needs the cgroup lock for reading, while libvirtd still has the lock for writing.

crash> bt 0xfaf031e220
PID: 2 TASK: faf031e220 CPU: 40 COMMAND: "kthreadd"
#0 [faf034bad8] __schedule at 83b2cc
#1 [faf034bb40] schedule at 83ba26
#2 [faf034bb58] rwsem_down_read_failed at 83fb64
#3 [faf034bbb8] percpu_down_read at 1bdf56
#4 [faf034bc20] copy_process at 15eab6
#5 [faf034bd08] _do_fork at 160430
#6 [faf034bdd0] kernel_thread at 160a82
#7 [faf034be30] kthreadd at 188580
#8 [faf034bea8] kernel_thread_starter at 842552

Quoted 8 lines

Quoted 2 lines

BANG.kthreadd waits for the lock that libvirtd hold, and libvirtd waits for kthreadd to finish some task

I don't see percpu_down_read being invoked from copy_process. According to LXR, this semaphore is used only in __cgroup_procs_write and cgroup_update_dfl_csses. And cgroup_update_dfl_csses is invoked when cgroup.subtree_control is written to. And I don't see this happening in this call chain.

Quoted 7 lines

The callchain is inlined and as follows:

_do_fork
copy_process
threadgroup_change_begin
cgroup_threadgroup_change_begin

Ah, I see I have missed that one. So essentially what's happening is that while migrating processes using a gobal rw semaphore essentially "disables" forking, but in this case in order to finish the migration a task has to be spawned (the workqueue worker) and this causes the lock. Such problems were non-existent before the percpu_rwsem rework since the lock used was a per-threadgroup. Bummer...

Hello,

Thanks a lot for the report and detailed analysis. Can you please test whether the following patch fixes the issue?

Thanks.

---
 include/linux/cpuset.h |    6 ++++++
 kernel/cgroup.c        |    2 ++
 kernel/cpuset.c        |   48 +++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 51 insertions(+), 5 deletions(-)

include/linux/cpuset.h +6 -0

@@ -137,6 +137,8 @@ static inline void set_mems_allowed(node
 	task_unlock(current);
 }
 
+extern void cpuset_post_attach_flush(void);
+
 #else /* !CONFIG_CPUSETS */
 
 static inline bool cpusets_enabled(void) { return false; }

@@ -243,6 +245,10 @@ static inline bool read_mems_allowed_ret
 	return false;
 }
 
+static inline void cpuset_post_attach_flush(void)
+{
+}
+
 #endif /* !CONFIG_CPUSETS */
 
 #endif /* _LINUX_CPUSET_H */

kernel/cgroup.c +2 -0

@@ -57,6 +57,7 @@
 #include <linux/vmalloc.h> /* TODO: replace with more sophisticated array */
 #include <linux/kthread.h>
 #include <linux/delay.h>
+#include <linux/cpuset.h>
 
 #include <linux/atomic.h>
 

@@ -2739,6 +2740,7 @@ out_unlock_rcu:
 out_unlock_threadgroup:
 	percpu_up_write(&cgroup_threadgroup_rwsem);
 	cgroup_kn_unlock(of->kn);
+	cpuset_post_attach_flush();
 	return ret ?: nbytes;
 }
 

kernel/cpuset.c +43 -5

@@ -287,6 +287,8 @@ static struct cpuset top_cpuset = {
 static DEFINE_MUTEX(cpuset_mutex);
 static DEFINE_SPINLOCK(callback_lock);
 
+static struct workqueue_struct *cpuset_migrate_mm_wq;
+
 /*
  * CPU / memory hotplug is handled asynchronously.
  */

@@ -971,6 +973,23 @@ static int update_cpumask(struct cpuset
 	return 0;
 }
 
+struct cpuset_migrate_mm_work {
+	struct work_struct	work;
+	struct mm_struct	*mm;
+	nodemask_t		from;
+	nodemask_t		to;
+};
+
+static void cpuset_migrate_mm_workfn(struct work_struct *work)
+{
+	struct cpuset_migrate_mm_work *mwork =
+		container_of(work, struct cpuset_migrate_mm_work, work);
+
+	do_migrate_pages(mwork->mm, &mwork->from, &mwork->to, MPOL_MF_MOVE_ALL);
+	mmput(mwork->mm);
+	kfree(mwork);
+}
+
 /*
  * cpuset_migrate_mm
  *

@@ -989,16 +1008,31 @@ static void cpuset_migrate_mm(struct mm_
 							const nodemask_t *to)
 {
 	struct task_struct *tsk = current;
+	struct cpuset_migrate_mm_work *mwork;
 
 	tsk->mems_allowed = *to;
 
-	do_migrate_pages(mm, from, to, MPOL_MF_MOVE_ALL);
+	mwork = kzalloc(sizeof(*mwork), GFP_KERNEL);
+	if (mwork) {
+		mwork->mm = mm;
+		mwork->from = *from;
+		mwork->to = *to;
+		INIT_WORK(&mwork->work, cpuset_migrate_mm_workfn);
+		queue_work(cpuset_migrate_mm_wq, &mwork->work);
+	} else {
+		mmput(mm);
+	}
 
 	rcu_read_lock();
 	guarantee_online_mems(task_cs(tsk), &tsk->mems_allowed);
 	rcu_read_unlock();
 }
 
+void cpuset_post_attach_flush(void)
+{
+	flush_workqueue(cpuset_migrate_mm_wq);
+}
+
 /*
  * cpuset_change_task_nodemask - change task's mems_allowed and mempolicy
  * @tsk: the task to change

@@ -1097,7 +1131,8 @@ static void update_tasks_nodemask(struct
 		mpol_rebind_mm(mm, &cs->mems_allowed);
 		if (migrate)
 			cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems);
-		mmput(mm);
+		else
+			mmput(mm);
 	}
 	css_task_iter_end(&it);
 

@@ -1545,11 +1580,11 @@ static void cpuset_attach(struct cgroup_
 			 * @old_mems_allowed is the right nodesets that we
 			 * migrate mm from.
 			 */
-			if (is_memory_migrate(cs)) {
+			if (is_memory_migrate(cs))
 				cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
 						  &cpuset_attach_nodemask_to);
-			}
-			mmput(mm);
+			else
+				mmput(mm);
 		}
 	}
 

@@ -2359,6 +2394,9 @@ void __init cpuset_init_smp(void)
 	top_cpuset.effective_mems = node_states[N_MEMORY];
 
 	register_hotmemory_notifier(&cpuset_track_online_nodes_nb);
+
+	cpuset_migrate_mm_wq = alloc_ordered_workqueue("cpuset_migrate_mm", 0);
+	BUG_ON(!cpuset_migrate_mm_wq);
 }
 
 /**

Quoted 7 lines · Tejun Heo

Hello,

Thanks a lot for the report and detailed analysis. Can you please test whether the following patch fixes the issue?

Thanks.

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

[25331.763607] DEBUG_LOCKS_WARN_ON(lock->owner != current)
[25331.763630] ------------[ cut here ]------------
[25331.763634] WARNING: at kernel/locking/mutex-debug.c:80
[25331.763637] Modules linked in: nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc btrfs xor raid6_pq ghash_s390 prng ecb aes_s390 des_s390 des_generic sha512_s390 sha256_s390 sha1_s390 sha_common eadm_sch nfsd auth_rpcgss oid_registry nfs_acl lockd vhost_net tun vhost macvtap macvlan grace sunrpc dm_service_time dm_multipath dm_mod autofs4
[25331.763708] CPU: 56 PID: 114657 Comm: systemd-udevd Not tainted 4.4.0+ #91
[25331.763711] task: 000000fadc79de40 ti: 000000f95e7f8000 task.ti: 000000f95e7f8000
[25331.763715] Krnl PSW : 0404c00180000000 00000000001b7f32 (debug_mutex_unlock+0x16a/0x188)
[25331.763726]            R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 EA:3
Krnl GPRS: 0000004c00000037 000000fadc79de40 000000000000002b 0000000000000000
[25331.763732]            000000000028da3c 0000000000000000 000000f95e7fbf08 000000fab8e10df0
[25331.763735]            000000000000005c 000000facc0dc000 000000000000005c 000000000033e14a
[25331.763738]            0700000000000000 000000fab8e10df0 00000000001b7f2e 000000f95e7fbc80
[25331.763746] Krnl Code: 00000000001b7f22: c0200042784c	larl	%r2,a06fba
           00000000001b7f28: c0e50006ad50	brasl	%r14,28d9c8
          #00000000001b7f2e: a7f40001		brc	15,1b7f30
          >00000000001b7f32: a7f4ffe1		brc	15,1b7ef4
           00000000001b7f36: c03000429c9f	larl	%r3,a0b874
           00000000001b7f3c: c0200042783f	larl	%r2,a06fba
           00000000001b7f42: c0e50006ad43	brasl	%r14,28d9c8
           00000000001b7f48: a7f40001		brc	15,1b7f4a
[25331.763795] Call Trace:
[25331.763798] ([<00000000001b7f2e>] debug_mutex_unlock+0x166/0x188)
[25331.763804]  [<0000000000836a08>] __mutex_unlock_slowpath+0xa8/0x190
[25331.763808]  [<000000000033e14a>] seq_read+0x1c2/0x450
[25331.763813]  [<0000000000311e72>] __vfs_read+0x42/0x100

[25331.763818]  [<000000000031284e>] vfs_read+0x76/0x130
[25331.763821]  [<000000000031361e>] SyS_read+0x66/0xd8
[25331.763826]  [<000000000083af06>] system_call+0xd6/0x270
[25331.763829]  [<000003ffae1f19c8>] 0x3ffae1f19c8
[25331.763831] INFO: lockdep is turned off.
[25331.763833] Last Breaking-Event-Address:
[25331.763836]  [<00000000001b7f2e>] debug_mutex_unlock+0x166/0x188
[25331.763839] ---[ end trace 45177640eb39ef44 ]---

Quoted 7 lines

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

[25331.763607] DEBUG_LOCKS_WARN_ON(lock->owner != current)
[25331.763630] ------------[ cut here ]------------
[25331.763634] WARNING: at kernel/locking/mutex-debug.c:80

I restarted the test with panic_on_warn. Hopefully I can get a dump to check which mutex this was.

Quoted 10 lines · Christian Borntraeger

Quoted 7 lines

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

[25331.763607] DEBUG_LOCKS_WARN_ON(lock->owner != current)
[25331.763630] ------------[ cut here ]------------
[25331.763634] WARNING: at kernel/locking/mutex-debug.c:80

I restarted the test with panic_on_warn. Hopefully I can get a dump to check which mutex this was.

Hard to reproduce warnings like this tend to point towards memory corruption. Someone stepped on the mutex value and tickles the sanity check.

With lockdep and debugging enabled the mutex gets quite a bit bigger, so it gets more likely to be hit by 'random' corruption.

The locking in seq_read() seems rather straight forward.

Quoted 19 lines · Peter Zijlstra

Quoted 10 lines · Christian Borntraeger

Quoted 7 lines

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

[25331.763607] DEBUG_LOCKS_WARN_ON(lock->owner != current)
[25331.763630] ------------[ cut here ]------------
[25331.763634] WARNING: at kernel/locking/mutex-debug.c:80

I restarted the test with panic_on_warn. Hopefully I can get a dump to check which mutex this was.

Hard to reproduce warnings like this tend to point towards memory corruption. Someone stepped on the mutex value and tickles the sanity check.

With lockdep and debugging enabled the mutex gets quite a bit bigger, so it gets more likely to be hit by 'random' corruption.

Quoted 1 line

The locking in seq_read() seems rather straight forward.

I was able to reproduce. The dump shows a mutex that has an owner field, which does not exists as a task so this all looks fishy. The good thing is, that I can reproduce the issue within some hours. (exact same backtrace). Will add some more debug data to get a handle where we come from.

Quoted 20 lines · Christian Borntraeger

Quoted 19 lines · Peter Zijlstra

Quoted 10 lines · Christian Borntraeger

Quoted 7 lines

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

[25331.763607] DEBUG_LOCKS_WARN_ON(lock->owner != current)
[25331.763630] ------------[ cut here ]------------
[25331.763634] WARNING: at kernel/locking/mutex-debug.c:80

I restarted the test with panic_on_warn. Hopefully I can get a dump to check which mutex this was.

Hard to reproduce warnings like this tend to point towards memory corruption. Someone stepped on the mutex value and tickles the sanity check.

With lockdep and debugging enabled the mutex gets quite a bit bigger, so it gets more likely to be hit by 'random' corruption.

Quoted 6 lines

Quoted 1 line

The locking in seq_read() seems rather straight forward.

I was able to reproduce. The dump shows a mutex that has an owner field, which does not exists as a task so this all looks fishy. The good thing is, that I can reproduce the issue within some hours. (exact same backtrace). Will add some more debug data to get a handle where we come from.

Did the owner field show to something that still looks like a task_struct?

No, its not a task_struct. Activating some more debug information did indeed revealed several other issues (overwritten redzones etc). Unfortunately I only saw the broken things after the facts, so I do not know which code did that. When I disabled the cgroup controllers in libvirt I was no longer able to trigger the bugs. Still trying to narrow things down.

Hello,

Quoted 5 lines · Christian Borntraeger

No, its not a task_struct. Activating some more debug information did indeed revealed several other issues (overwritten redzones etc). Unfortunately I only saw the broken things after the facts, so I do not know which code did that. When I disabled the cgroup controllers in libvirt I was no longer able to trigger the bugs. Still trying to narrow things down.

Hmmm... that's worrying. CONFIG_DEBUG_PAGEALLOC sometimes can catch these sort of bugs red-handed. Might worth trying.

Quoted 11 lines · Tejun Heo

Hello,

Quoted 5 lines · Christian Borntraeger

No, its not a task_struct. Activating some more debug information did indeed revealed several other issues (overwritten redzones etc). Unfortunately I only saw the broken things after the facts, so I do not know which code did that. When I disabled the cgroup controllers in libvirt I was no longer able to trigger the bugs. Still trying to narrow things down.

Hmmm... that's worrying. CONFIG_DEBUG_PAGEALLOC sometimes can catch these sort of bugs red-handed. Might worth trying.

Christian, just to avoid that you get surprised like I did:
CONFIG_DEBUG_PAGEALLOC requires in the meantime an additional kernel parameter "debug_pagealloc=on" to be active.

That change was introduced a year ago, so it was probably only me who wasn't aware of that change :)

[  561.044066] Krnl PSW : 0704e00180000000 00000000001aa1ee (remove_entity_load_avg+0x1e/0x1b8)

[  561.044176] ([<00000000001ad750>] free_fair_sched_group+0x80/0xf8)
[  561.044181]  [<0000000000192656>] free_sched_group+0x2e/0x58
[  561.044187]  [<00000000001ded82>] rcu_process_callbacks+0x3fa/0x928

Quoted 5 lines · Christian Borntraeger

[  561.044066] Krnl PSW : 0704e00180000000 00000000001aa1ee (remove_entity_load_avg+0x1e/0x1b8)

[  561.044176] ([<00000000001ad750>] free_fair_sched_group+0x80/0xf8)
[  561.044181]  [<0000000000192656>] free_sched_group+0x2e/0x58
[  561.044187]  [<00000000001ded82>] rcu_process_callbacks+0x3fa/0x928

Urgh,.. lemme stare at that.

TJ, is css_offline guaranteed to be called in hierarchical order? I

got properly lost in the whole cgroup destroy code. There's endless workqueues and rcu callbacks there.

So the current place in free_fair_sched_group() is far too late to be calling remove_entity_load_avg(). But I'm not sure where I should put it, it needs to be in a place where we know the group is going to die but its parent is guaranteed to still exist.

Would offline be that place?

Quoted 6 lines

So the current place in free_fair_sched_group() is far too late to be calling remove_entity_load_avg(). But I'm not sure where I should put it, it needs to be in a place where we know the group is going to die but its parent is guaranteed to still exist.

Would offline be that place?

Hmmm... css_free would be with the following patch.

Quoted 7 lines · Tejun Heo

Quoted 6 lines

So the current place in free_fair_sched_group() is far too late to be calling remove_entity_load_avg(). But I'm not sure where I should put it, it needs to be in a place where we know the group is going to die but its parent is guaranteed to still exist.

Would offline be that place?

Quoted 1 line

Hmmm... css_free would be with the following patch.

I thought a bit more about this and I think the right thing to do here is making both css_offline and css_free follow the ancestry order. I'll post a patch to do that soon. offline is called at the head of destruction when the css is made invisble and draining of existing refs starts. free at the end of that process. Tree ordering shouldn't be where the two differ.

Quoted 6 lines

I thought a bit more about this and I think the right thing to do here is making both css_offline and css_free follow the ancestry order. I'll post a patch to do that soon. offline is called at the head of destruction when the css is made invisble and draining of existing refs starts. free at the end of that process. Tree ordering shouldn't be where the two differ.

OK, that would be good. Meanwhile the above seems to suggest that css_offline is already hierarchical?

I get the feeling the way sched uses the css_{offline,release,free} is sub-optimal. cpu_cgrp_subsys::css_free := sched_destroy_group() does a call_rcu, whereas if I read the comment with css_free_work_fn() correctly, this is already after a grace-period, so yet another doesn't make sense.

Hello,

Quoted 1 line · Peter Zijlstra

TJ, is css_offline guaranteed to be called in hierarchical order? I

No, they aren't. The ancestors of a css are guaranteed to stay around until css_free is called on the css and that's the only ordering guarantee.

Quoted 2 lines

got properly lost in the whole cgroup destroy code. There's endless workqueues and rcu callbacks there.

Yeah, it's hairy. I wondered about adding support for bouncing to workqueue in both percpu_ref and rcu which would make things easier to follow. Not sure how often this pattern happens tho.

Quoted 3 lines

Yeah, it's hairy. I wondered about adding support for bouncing to workqueue in both percpu_ref and rcu which would make things easier to follow. Not sure how often this pattern happens tho.

This came up recently offlist for call_rcu(), so that a call to (say) call_rcu_schedule_work() would do a schedule_work() after a grace period elapsed, invoking the function passed in to call_rcu_schedule_work(). There are several existing cases that do this, so special-casing it seems worthwhile. Perhaps something vaguely similar would work for percpu_ref.

FYI, my use case was also related to percpu-ref. The percpu ref API is unfortunately really hard to use and will almost always involve a work queue due to the complex interaction between percpu_ref_kill and percpu_ref_exit. One thing that would help a lot of callers would

be a percpu_ref_exit_sync that kills the ref and waits for all references to go away synchronously.

Quoted 4 lines · Christoph Hellwig

FYI, my use case was also related to percpu-ref. The percpu ref API is unfortunately really hard to use and will almost always involve a work queue due to the complex interaction between percpu_ref_kill and percpu_ref_exit. One thing that would help a lot of callers would

That's interesting. Can you please elaborate on how kill and exit interact to make things complex?

Quoted 2 lines

be a percpu_ref_exit_sync that kills the ref and waits for all references to go away synchronously.

That shouldn't be difficult to implement. One minor concern is that it's almost guaranteed that there will be cases where the synchronicity is exposed to userland. Anyways, can you please describe the use case?

Quoted 2 lines

That's interesting. Can you please elaborate on how kill and exit interact to make things complex?

That we need to first call kill to tear down the reference, then we get a release callback which is in the calling context of the last percpu_ref_put, but will need to call percpu_ref_exit from process context again. This means if any percpu_ref_put is from non-process context

we will always need a work_struct or similar to schedule the final percpu_ref_exit. Except when..

Quoted 7 lines

Quoted 2 lines

be a percpu_ref_exit_sync that kills the ref and waits for all references to go away synchronously.

That shouldn't be difficult to implement. One minor concern is that it's almost guaranteed that there will be cases where the synchronicity is exposed to userland. Anyways, can you please describe the use case?

We use this completion scheme where the percpu_ref_exit is done from the same context as the percpu_ref_kill which previously waits for the last reference drop. But for these cases exposing the synchronicity to the caller (including userland) actually is intentional.

My use case is a new storage target, broadly similar to the SCSI target, which happens to exhibit the same behavior. In that case we only want to return from the teardown function when all I/O on a 'queue' of sorts has finished, for example during module removal.

Hmmm... why do you need to call percpu_ref_exit() from process context? All it does is freeing the percpu counter and resetting the state, both of which can be done from any context.

Quoted 5 lines · Christian Borntraeger

[  561.044066] Krnl PSW : 0704e00180000000 00000000001aa1ee (remove_entity_load_avg+0x1e/0x1b8)

[  561.044176] ([<00000000001ad750>] free_fair_sched_group+0x80/0xf8)
[  561.044181]  [<0000000000192656>] free_sched_group+0x2e/0x58
[  561.044187]  [<00000000001ded82>] rcu_process_callbacks+0x3fa/0x928

Urgh,.. lemme stare at that.

Quoted 7 lines · Peter Zijlstra

Quoted 5 lines · Christian Borntraeger

[  561.044066] Krnl PSW : 0704e00180000000 00000000001aa1ee (remove_entity_load_avg+0x1e/0x1b8)

[  561.044176] ([<00000000001ad750>] free_fair_sched_group+0x80/0xf8)
[  561.044181]  [<0000000000192656>] free_sched_group+0x2e/0x58
[  561.044187]  [<00000000001ded82>] rcu_process_callbacks+0x3fa/0x928

Quoted 1 line

Urgh,.. lemme stare at that.

Christian, can you test with the remove_entity_load_avg() call removed from free_fair_sched_group() ?

It will slightly mess up accounting, but should be non fatal and avoids this current issue.

With Tejuns "cpuset: make mm migration asynchronous" and this hack

kernel/sched/fair.c +0 -0

index cfdc0e6..0847bab 100644

kernel/sched/fair.c +2 -2

@@ -8099,8 +8099,8 @@ void free_fair_sched_group(struct task_group *tg)
                if (tg->cfs_rq)
                        kfree(tg->cfs_rq[i]);
                if (tg->se) {
-                       if (tg->se[i])
-                               remove_entity_load_avg(tg->se[i]);
+//                     if (tg->se[i])
+//                             remove_entity_load_avg(tg->se[i]);
                        kfree(tg->se[i]);
                }
        }

things look good now on the scheduler/cgroup front. Thank you for your quick responses and answers.

Quoted 5 lines · Tejun Heo

Hello,

Thanks a lot for the report and detailed analysis. Can you please test whether the following patch fixes the issue?

Quoted 2 lines

Thanks.

Yes, the deadlock is gone and the system is still running. After some time I had the following WARN in the logs, though. Not sure yet if that is related.

If "cpuset.memory_migrate" is set, when a process is moved from one cpuset to another with a different memory node mask, pages in used by the process are migrated to the new set of nodes. This was performed synchronously in the ->attach() callback, which is synchronized against process management. Recently, the synchronization was changed from per-process rwsem to global percpu rwsem for simplicity and optimization.

Combined with the synchronous mm migration, this led to deadlocks because mm migration could schedule a work item which may in turn try to create a new worker blocking on the process management lock held from cgroup process migration path.

This heavy an operation shouldn't be performed synchronously from that deep inside cgroup migration in the first place. This patch punts the actual migration to an ordered workqueue and updates cgroup process migration and cpuset config update paths to flush the workqueue after all locks are released. This way, the operations still seem synchronous to userland without entangling mm migration with process management synchronization. CPU hotplug can also invoke mm migration but there's no reason for it to wait for mm migrations and thus doesn't synchronize against their completions.

Signed-off-by Tejun Heo <[email protected]>

Reported-and-tested-by Christian Borntraeger <[email protected]>

Cc [email protected] # v4.4+

---
 include/linux/cpuset.h |    6 ++++
 kernel/cgroup.c        |    2 +
 kernel/cpuset.c        |   71 +++++++++++++++++++++++++++++++++----------------
 3 files changed, 57 insertions(+), 22 deletions(-)

include/linux/cpuset.h +6 -0

index 85a868c..fea160e 100644
@@ -137,6 +137,8 @@ static inline void set_mems_allowed(nodemask_t nodemask)
 	task_unlock(current);
 }
 
+extern void cpuset_post_attach_flush(void);
+
 #else /* !CONFIG_CPUSETS */
 
 static inline bool cpusets_enabled(void) { return false; }

@@ -243,6 +245,10 @@ static inline bool read_mems_allowed_retry(unsigned int seq)
 	return false;
 }
 
+static inline void cpuset_post_attach_flush(void)
+{
+}
+
 #endif /* !CONFIG_CPUSETS */
 
 #endif /* _LINUX_CPUSET_H */

kernel/cgroup.c +2 -0

index c03a640..88abd4d 100644
@@ -58,6 +58,7 @@
 #include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/atomic.h>
+#include <linux/cpuset.h>
 #include <net/sock.h>
 
 /*

@@ -2739,6 +2740,7 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
 out_unlock_threadgroup:
 	percpu_up_write(&cgroup_threadgroup_rwsem);
 	cgroup_kn_unlock(of->kn);
+	cpuset_post_attach_flush();
 	return ret ?: nbytes;
 }
 

kernel/cpuset.c +49 -22

index 3e945fc..41989ab 100644
@@ -287,6 +287,8 @@ static struct cpuset top_cpuset = {
 static DEFINE_MUTEX(cpuset_mutex);
 static DEFINE_SPINLOCK(callback_lock);
 
+static struct workqueue_struct *cpuset_migrate_mm_wq;
+
 /*
  * CPU / memory hotplug is handled asynchronously.
  */

@@ -972,31 +974,51 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 }
 
 /*
- * cpuset_migrate_mm
- *
- *    Migrate memory region from one set of nodes to another.
- *
- *    Temporarilly set tasks mems_allowed to target nodes of migration,
- *    so that the migration code can allocate pages on these nodes.
- *
- *    While the mm_struct we are migrating is typically from some
- *    other task, the task_struct mems_allowed that we are hacking
- *    is for our current task, which must allocate new pages for that
- *    migrating memory region.
+ * Migrate memory region from one set of nodes to another.  This is
+ * performed asynchronously as it can be called from process migration path
+ * holding locks involved in process management.  All mm migrations are
+ * performed in the queued order and can be waited for by flushing
+ * cpuset_migrate_mm_wq.
  */
 
+struct cpuset_migrate_mm_work {
+	struct work_struct	work;
+	struct mm_struct	*mm;
+	nodemask_t		from;
+	nodemask_t		to;
+};
+
+static void cpuset_migrate_mm_workfn(struct work_struct *work)
+{
+	struct cpuset_migrate_mm_work *mwork =
+		container_of(work, struct cpuset_migrate_mm_work, work);
+
+	/* on a wq worker, no need to worry about %current's mems_allowed */
+	do_migrate_pages(mwork->mm, &mwork->from, &mwork->to, MPOL_MF_MOVE_ALL);
+	mmput(mwork->mm);
+	kfree(mwork);
+}
+
 static void cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from,
 							const nodemask_t *to)
 {
-	struct task_struct *tsk = current;
-
-	tsk->mems_allowed = *to;
+	struct cpuset_migrate_mm_work *mwork;
 
-	do_migrate_pages(mm, from, to, MPOL_MF_MOVE_ALL);
+	mwork = kzalloc(sizeof(*mwork), GFP_KERNEL);
+	if (mwork) {
+		mwork->mm = mm;
+		mwork->from = *from;
+		mwork->to = *to;
+		INIT_WORK(&mwork->work, cpuset_migrate_mm_workfn);
+		queue_work(cpuset_migrate_mm_wq, &mwork->work);
+	} else {
+		mmput(mm);
+	}
+}
 
-	rcu_read_lock();
-	guarantee_online_mems(task_cs(tsk), &tsk->mems_allowed);
-	rcu_read_unlock();
+void cpuset_post_attach_flush(void)
+{
+	flush_workqueue(cpuset_migrate_mm_wq);
 }
 
 /*

@@ -1097,7 +1119,8 @@ static void update_tasks_nodemask(struct cpuset *cs)
 		mpol_rebind_mm(mm, &cs->mems_allowed);
 		if (migrate)
 			cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems);
-		mmput(mm);
+		else
+			mmput(mm);
 	}
 	css_task_iter_end(&it);
 

@@ -1545,11 +1568,11 @@ static void cpuset_attach(struct cgroup_taskset *tset)
 			 * @old_mems_allowed is the right nodesets that we
 			 * migrate mm from.
 			 */
-			if (is_memory_migrate(cs)) {
+			if (is_memory_migrate(cs))
 				cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
 						  &cpuset_attach_nodemask_to);
-			}
-			mmput(mm);
+			else
+				mmput(mm);
 		}
 	}
 

@@ -1714,6 +1737,7 @@ static ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
 	mutex_unlock(&cpuset_mutex);
 	kernfs_unbreak_active_protection(of->kn);
 	css_put(&cs->css);
+	flush_workqueue(cpuset_migrate_mm_wq);
 	return retval ?: nbytes;
 }
 

@@ -2359,6 +2383,9 @@ void __init cpuset_init_smp(void)
 	top_cpuset.effective_mems = node_states[N_MEMORY];
 
 	register_hotmemory_notifier(&cpuset_track_online_nodes_nb);
+
+	cpuset_migrate_mm_wq = alloc_ordered_workqueue("cpuset_migrate_mm", 0);
+	BUG_ON(!cpuset_migrate_mm_wq);
 }
 
 /**

Hmmm I just realized that this patch slightly differs from the one that I tested. Do we need a retest?

Hello, Christian.

Quoted 2 lines · Christian Borntraeger

Hmmm I just realized that this patch slightly differs from the one that I tested. Do we need a retest?

It should be fine but I'd appreciate if you can test it again.

Quoted 7 lines · Tejun Heo

Hello, Christian.

Quoted 2 lines · Christian Borntraeger

Hmmm I just realized that this patch slightly differs from the one that I tested. Do we need a retest?

It should be fine but I'd appreciate if you can test it again.

I did restart the test after I wrote the mail. The latest version from this mail thread is still fine as far as I can tell.

If "cpuset.memory_migrate" is set, when a process is moved from one cpuset to another with a different memory node mask, pages in used by the process are migrated to the new set of nodes. This was performed synchronously in the ->attach() callback, which is synchronized against process management. Recently, the synchronization was changed from per-process rwsem to global percpu rwsem for simplicity and optimization.

Combined with the synchronous mm migration, this led to deadlocks because mm migration could schedule a work item which may in turn try to create a new worker blocking on the process management lock held from cgroup process migration path.

This heavy an operation shouldn't be performed synchronously from that deep inside cgroup migration in the first place. This patch punts the actual migration to an ordered workqueue and updates cgroup process migration and cpuset config update paths to flush the workqueue after all locks are released. This way, the operations still seem synchronous to userland without entangling mm migration with process management synchronization. CPU hotplug can also invoke mm migration but there's no reason for it to wait for mm migrations and thus doesn't synchronize against their completions.

Signed-off-by Tejun Heo <[email protected]>

Reported-and-tested-by Christian Borntraeger <[email protected]>

Cc [email protected] # v4.4+

There are three subsystem callbacks in css shutdown path - css_offline(), css_released() and css_free(). Except for css_released(), cgroup core didn't use to guarantee the order of invocation. css_offline() or css_free() could be called on a parent css before its children. This behavior is unexpected and led to use-after-free in cpu controller.

This patch updates offline path so that a parent css is never offlined before its children. Each css keeps online_cnt which reaches zero iff itself and all its children are offline and offline_css() is invoked only after online_cnt reaches zero.

This fixes the reported cpu controller malfunction. The next patch will update css_free() handling.

Quoted 12 lines · Tejun Heo

There are three subsystem callbacks in css shutdown path - css_offline(), css_released() and css_free(). Except for css_released(), cgroup core didn't use to guarantee the order of invocation. css_offline() or css_free() could be called on a parent css before its children. This behavior is unexpected and led to use-after-free in cpu controller.

This patch updates offline path so that a parent css is never offlined before its children. Each css keeps online_cnt which reaches zero iff itself and all its children are offline and offline_css() is invoked only after online_cnt reaches zero.

Quoted 2 lines

This fixes the reported cpu controller malfunction. The next patch will update css_free() handling.

No, I need to fix the cpu controller too, because the offending code sits off of css_free() (the next patch), but also does a call_rcu() in between, which also doesn't guarantee order.

Quoted 16 lines · Peter Zijlstra

Quoted 14 lines · Tejun Heo

There are three subsystem callbacks in css shutdown path - css_offline(), css_released() and css_free(). Except for css_released(), cgroup core didn't use to guarantee the order of invocation. css_offline() or css_free() could be called on a parent css before its children. This behavior is unexpected and led to use-after-free in cpu controller.

This patch updates offline path so that a parent css is never offlined before its children. Each css keeps online_cnt which reaches zero iff itself and all its children are offline and offline_css() is invoked only after online_cnt reaches zero.

This fixes the reported cpu controller malfunction. The next patch will update css_free() handling.

Quoted 3 lines

No, I need to fix the cpu controller too, because the offending code sits off of css_free() (the next patch), but also does a call_rcu() in between, which also doesn't guarantee order.

Ah, I see. Christian, can you please apply all three patches and see whether the problem gets fixed? Once verified, I'll update the patch description and repost.