Skip to content

Commit 90422cf

Browse files
jlintonarmintel-lab-lkp
jlintonarm
authored and
intel-lab-lkp
committed
mm/memcg: Possible cgroup migrate/signal deadlock
With CONFIG_MEMCG_KMEM and CONFIG_PROVE_LOCKING enabled (fedora rawhide kernel), running a simple podman test tosses a circular locking dependency warning. The podman container in question simpy contains the echo command and the libc/ld-linux needed to run it. The warning can be duplicated with just a single `podman build --network host --layers=false -t localhost/echo .` command, although the exact sequence that triggers the warning needs the task state to be changing the frozen state as well. So, its easier to duplicate with a slightly longer test case. I've attempted to trigger the actual deadlock with some standalone code and been unsuccessful, but looking at the code it appears to be a legitimate deadlock if a signal is being sent to the process from another thread while the task is migrating between cgroups. Attached is a fix which I'm confident fixes the problem, but I'm not really that confident in the fix since I don't fully understand all the possible states in the cgroup code. The fix avoids the deadlock by shifting the objcg->list manipulation to another spinlock and then using list_del_rcu in obj_cgroup_release. There is a bit more information in the actual BZ https://bugzilla.redhat.com/show_bug.cgi?id=2033016 including a shell script with the podman test/etc. (this machine has a new 5.17 splat before in the network driver) [ 87.700526] ====================================================== [ 87.706693] WARNING: possible circular locking dependency detected [ 87.712861] 5.17.0-rc2+ torvalds#171 Tainted: G W [ 87.718161] ------------------------------------------------------ [ 87.724328] podman/2371 is trying to acquire lock: [ 87.729107] ffffa723507a2838 (css_set_lock){..-.}-{2:2}, at: _raw_spin_lock_irqsave+0x1c/0x30 [ 87.737636] [ 87.737636] but task is already holding lock: [ 87.743456] ffff577e760d88d8 (&sighand->siglock){....}-{2:2}, at: _raw_spin_lock_irqsave+0x1c/0x30 [ 87.752409] [ 87.752409] which lock already depends on the new lock. [ 87.752409] [ 87.760572] [ 87.760572] the existing dependency chain (in reverse order) is: [ 87.768041] [ 87.768041] -> #1 (&sighand->siglock){....}-{2:2}: [ 87.774301] __lock_acquire+0x444/0x9c0 [ 87.778650] lock_acquire.part.0+0xa8/0x1b0 [ 87.783343] lock_acquire+0x68/0x8c [ 87.787342] __raw_spin_lock_irqsave+0x8c/0x140 [ 87.792383] _raw_spin_lock_irqsave+0x1c/0x30 [ 87.797249] __lock_task_sighand+0xa4/0x1d0 [ 87.801944] cgroup_freeze_task+0x28/0x80 [ 87.806465] cgroup_freezer_migrate_task+0x78/0xe0 [ 87.811765] cgroup_migrate_execute+0x33c/0x494 [ 87.816807] cgroup_update_dfl_csses+0x210/0x230 [ 87.821935] cgroup_subtree_control_write+0x41c/0x440 [ 87.827497] cgroup_file_write+0x90/0x260 [ 87.832016] kernfs_fop_write_iter+0x13c/0x1d0 [ 87.836971] new_sync_write+0xdc/0x15c [ 87.841230] vfs_write+0x1cc/0x220 [ 87.845141] ksys_write+0x64/0xec [ 87.848966] __arm64_sys_write+0x28/0x34 [ 87.853398] invoke_syscall+0x50/0x120 [ 87.857657] el0_svc_common.constprop.0+0x68/0x124 [ 87.862957] do_el0_svc+0x34/0x9c [ 87.866781] el0_svc+0x5c/0x19c [ 87.870433] el0t_64_sync_handler+0xa4/0x130 [ 87.875213] el0t_64_sync+0x1a4/0x1a8 [ 87.879385] [ 87.879385] -> #0 (css_set_lock){..-.}-{2:2}: [ 87.885210] check_prev_add+0xac/0x68c [ 87.889469] validate_chain+0x3fc/0x590 [ 87.893815] __lock_acquire+0x444/0x9c0 [ 87.898160] lock_acquire.part.0+0xa8/0x1b0 [ 87.902853] lock_acquire+0x68/0x8c [ 87.906851] __raw_spin_lock_irqsave+0x8c/0x140 [ 87.911892] _raw_spin_lock_irqsave+0x1c/0x30 [ 87.916758] obj_cgroup_release+0x4c/0xd0 [ 87.921280] percpu_ref_put_many.constprop.0+0x11c/0x130 [ 87.927102] drain_obj_stock+0x88/0xdc [ 87.931360] refill_obj_stock+0x8c/0x1e0 [ 87.935792] obj_cgroup_charge+0x100/0x1cc [ 87.940398] kmem_cache_alloc+0xb8/0x354 [ 87.944829] __sigqueue_alloc+0x164/0x340 [ 87.949350] __send_signal+0x248/0x560 [ 87.953610] send_signal+0x1c0/0x340 [ 87.957695] do_send_specific+0x1ac/0x1d0 [ 87.962214] do_tkill+0x84/0xa0 [ 87.965864] __arm64_sys_tgkill+0x38/0x50 [ 87.970383] invoke_syscall+0x50/0x120 [ 87.974642] el0_svc_common.constprop.0+0x68/0x124 [ 87.979941] do_el0_svc+0x34/0x9c [ 87.983765] el0_svc+0x5c/0x19c [ 87.987416] el0t_64_sync_handler+0xa4/0x130 [ 87.992196] el0t_64_sync+0x1a4/0x1a8 [ 87.996367] [ 87.996367] other info that might help us debug this: [ 87.996367] [ 88.004356] Possible unsafe locking scenario: [ 88.004356] [ 88.010263] CPU0 CPU1 [ 88.014780] ---- ---- [ 88.019297] lock(&sighand->siglock); [ 88.023036] lock(css_set_lock); [ 88.028858] lock(&sighand->siglock); [ 88.035114] lock(css_set_lock); [ 88.038418] [ 88.038418] *** DEADLOCK *** [ 88.038418] [ 88.044324] 3 locks held by podman/2371: [ 88.048235] #0: ffffa7235072d1c8 (rcu_read_lock){....}-{1:2}, at: do_send_specific+0x8/0x1d0 [ 88.056756] #1: ffff577e760d88d8 (&sighand->siglock){....}-{2:2}, at: _raw_spin_lock_irqsave+0x1c/0x30 [ 88.066144] #2: ffffa7235072d1c8 (rcu_read_lock){....}-{1:2}, at: percpu_ref_put_many.constprop.0+0x0/0x130 [ 88.075967] [ 88.075967] stack backtrace: [ 88.080312] CPU: 6 PID: 2371 Comm: podman Tainted: G W 5.17.0-rc2+ torvalds#171 [ 88.088217] Hardware name: SolidRun Ltd. SolidRun CEX7 Platform, BIOS EDK II Aug 9 2021 [ 88.096294] Call trace: [ 88.098728] dump_backtrace+0xf8/0x130 [ 88.102466] show_stack+0x24/0x80 [ 88.105770] dump_stack_lvl+0x8c/0xb8 [ 88.109423] dump_stack+0x18/0x34 [ 88.112727] print_circular_bug+0x1f8/0x200 [ 88.116899] check_noncircular+0x104/0x130 [ 88.120984] check_prev_add+0xac/0x68c [ 88.124722] validate_chain+0x3fc/0x590 [ 88.128547] __lock_acquire+0x444/0x9c0 [ 88.132371] lock_acquire.part.0+0xa8/0x1b0 [ 88.136543] lock_acquire+0x68/0x8c [ 88.140021] __raw_spin_lock_irqsave+0x8c/0x140 [ 88.144541] _raw_spin_lock_irqsave+0x1c/0x30 [ 88.148886] obj_cgroup_release+0x4c/0xd0 [ 88.152886] percpu_ref_put_many.constprop.0+0x11c/0x130 [ 88.158187] drain_obj_stock+0x88/0xdc [ 88.161924] refill_obj_stock+0x8c/0x1e0 [ 88.165836] obj_cgroup_charge+0x100/0x1cc [ 88.169920] kmem_cache_alloc+0xb8/0x354 [ 88.173832] __sigqueue_alloc+0x164/0x340 [ 88.177831] __send_signal+0x248/0x560 [ 88.181569] send_signal+0x1c0/0x340 Signed-off-by: Jeremy Linton <[email protected]> Cc: Johannes Weiner <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Vladimir Davydov <[email protected]> Cc: Andrew Morton <[email protected]>
1 parent b828014 commit 90422cf

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

‎mm/memcontrol.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ struct mem_cgroup *vmpressure_to_memcg(struct vmpressure *vmpr)
255255

256256
#ifdef CONFIG_MEMCG_KMEM
257257
extern spinlock_t css_set_lock;
258+
DEFINE_SPINLOCK(objcg_list_lock);
258259

259260
bool mem_cgroup_kmem_disabled(void)
260261
{
@@ -298,9 +299,9 @@ static void obj_cgroup_release(struct percpu_ref *ref)
298299
if (nr_pages)
299300
obj_cgroup_uncharge_pages(objcg, nr_pages);
300301

301-
spin_lock_irqsave(&css_set_lock, flags);
302-
list_del(&objcg->list);
303-
spin_unlock_irqrestore(&css_set_lock, flags);
302+
spin_lock_irqsave(&objcg_list_lock, flags);
303+
list_del_rcu(&objcg->list);
304+
spin_unlock_irqrestore(&objcg_list_lock, flags);
304305

305306
percpu_ref_exit(ref);
306307
kfree_rcu(objcg, rcu);
@@ -333,6 +334,7 @@ static void memcg_reparent_objcgs(struct mem_cgroup *memcg,
333334
objcg = rcu_replace_pointer(memcg->objcg, NULL, true);
334335

335336
spin_lock_irq(&css_set_lock);
337+
spin_lock(&objcg_list_lock);
336338

337339
/* 1) Ready to reparent active objcg. */
338340
list_add(&objcg->list, &memcg->objcg_list);
@@ -342,6 +344,7 @@ static void memcg_reparent_objcgs(struct mem_cgroup *memcg,
342344
/* 3) Move already reparented objcgs to the parent's list */
343345
list_splice(&memcg->objcg_list, &parent->objcg_list);
344346

347+
spin_unlock(&objcg_list_lock);
345348
spin_unlock_irq(&css_set_lock);
346349

347350
percpu_ref_kill(&objcg->refcnt);

0 commit comments

Comments
 (0)