On Sun, Apr 22, 2018 at 03:07:59AM -0700, Nathan Chancellor wrote: > From: Greg Thelen <gthelen@xxxxxxxxxx> > > commit 2e898e4c0a3897ccd434adac5abb8330194f527b upstream. > > lock_page_memcg()/unlock_page_memcg() use spin_lock_irqsave/restore() if > the page's memcg is undergoing move accounting, which occurs when a > process leaves its memcg for a new one that has > memory.move_charge_at_immigrate set. > > unlocked_inode_to_wb_begin,end() use spin_lock_irq/spin_unlock_irq() if > the given inode is switching writeback domains. Switches occur when > enough writes are issued from a new domain. > > This existing pattern is thus suspicious: > lock_page_memcg(page); > unlocked_inode_to_wb_begin(inode, &locked); > ... > unlocked_inode_to_wb_end(inode, locked); > unlock_page_memcg(page); > > If both inode switch and process memcg migration are both in-flight then > unlocked_inode_to_wb_end() will unconditionally enable interrupts while > still holding the lock_page_memcg() irq spinlock. This suggests the > possibility of deadlock if an interrupt occurs before unlock_page_memcg(). > > truncate > __cancel_dirty_page > lock_page_memcg > unlocked_inode_to_wb_begin > unlocked_inode_to_wb_end > <interrupts mistakenly enabled> > <interrupt> > end_page_writeback > test_clear_page_writeback > lock_page_memcg > <deadlock> > unlock_page_memcg > > Due to configuration limitations this deadlock is not currently possible > because we don't mix cgroup writeback (a cgroupv2 feature) and > memory.move_charge_at_immigrate (a cgroupv1 feature). > > If the kernel is hacked to always claim inode switching and memcg > moving_account, then this script triggers lockup in less than a minute: > > cd /mnt/cgroup/memory > mkdir a b > echo 1 > a/memory.move_charge_at_immigrate > echo 1 > b/memory.move_charge_at_immigrate > ( > echo $BASHPID > a/cgroup.procs > while true; do > dd if=/dev/zero of=/mnt/big bs=1M count=256 > done > ) & > while true; do > sync > done & > sleep 1h & > SLEEP=$! > while true; do > echo $SLEEP > a/cgroup.procs > echo $SLEEP > b/cgroup.procs > done > > The deadlock does not seem possible, so it's debatable if there's any > reason to modify the kernel. I suggest we should to prevent future > surprises. And Wang Long said "this deadlock occurs three times in our > environment", so there's more reason to apply this, even to stable. > Stable 4.4 has minor conflicts applying this patch. For a clean 4.4 patch > see "[PATCH for-4.4] writeback: safer lock nesting" > https://lkml.org/lkml/2018/4/11/146 > > Wang Long said "this deadlock occurs three times in our environment" > > [gthelen@xxxxxxxxxx: v4] > Link: http://lkml.kernel.org/r/20180411084653.254724-1-gthelen@xxxxxxxxxx > [akpm@xxxxxxxxxxxxxxxxxxxx: comment tweaks, struct initialization simplification] > Change-Id: Ibb773e8045852978f6207074491d262f1b3fb613 > Link: http://lkml.kernel.org/r/20180410005908.167976-1-gthelen@xxxxxxxxxx > Fixes: 682aa8e1a6a1 ("writeback: implement unlocked_inode_to_wb transaction and use it for stat updates") > Signed-off-by: Greg Thelen <gthelen@xxxxxxxxxx> > Reported-by: Wang Long <wanglong19@xxxxxxxxxxx> > Acked-by: Wang Long <wanglong19@xxxxxxxxxxx> > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > Reviewed-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx> > Cc: Tejun Heo <tj@xxxxxxxxxx> > Cc: Nicholas Piggin <npiggin@xxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> [v4.2+] > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > [natechancellor: Applied to 4.4 based on Greg's backport on lkml.org] I need a working patch for 4.9.y, 4.14.y and 4.16.y first before I can apply this one :( thanks, greg k-h