* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 [not found] <CAPM=9tyy5vubggbcj32bGpA_h6yDaBNM3QeJPySTzci-etfBZw@mail.gmail.com> @ 2020-05-21 23:13 ` Darrick J. Wong 2020-05-22 0:30 ` Dave Chinner 0 siblings, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2020-05-21 23:13 UTC (permalink / raw) To: Dave Airlie; +Cc: LKML, Linus Torvalds, xfs [cc linux-xfs] On Fri, May 22, 2020 at 08:21:50AM +1000, Dave Airlie wrote: > Hi, > > Just updated a rawhide VM to the Fedora 5.7.0-rc5 kernel, did some > package building, > > got the below trace, not sure if it's known and fixed or unknown. It's a known false-positive. An inode can't simultaneously be getting reclaimed due to zero refcount /and/ be the target of a getxattr call. Unfortunately, lockdep can't tell the difference, and it seems a little strange to set NOFS on the allocation (which increases the chances of a runtime error) just to quiet that down. --D > Dave. > > 725.862536] ====================================================== > [ 725.862564] WARNING: possible circular locking dependency detected > [ 725.862591] 5.7.0-0.rc5.20200515git1ae7efb38854.1.fc33.x86_64 #1 Not tainted > [ 725.862612] ------------------------------------------------------ > [ 725.862630] kswapd0/159 is trying to acquire lock: > [ 725.862645] ffff9b38d01a4470 (&xfs_nondir_ilock_class){++++}-{3:3}, > at: xfs_ilock+0xde/0x2c0 [xfs] > [ 725.862718] > but task is already holding lock: > [ 725.862735] ffffffffbbb8bd00 (fs_reclaim){+.+.}-{0:0}, at: > __fs_reclaim_acquire+0x5/0x30 > [ 725.862762] > which lock already depends on the new lock. > > [ 725.862785] > the existing dependency chain (in reverse order) is: > [ 725.862806] > -> #1 (fs_reclaim){+.+.}-{0:0}: > [ 725.862824] fs_reclaim_acquire+0x34/0x40 > [ 725.862839] __kmalloc+0x4f/0x270 > [ 725.862878] kmem_alloc+0x93/0x1d0 [xfs] > [ 725.862914] kmem_alloc_large+0x4c/0x130 [xfs] > [ 725.862945] xfs_attr_copy_value+0x74/0xa0 [xfs] > [ 725.862984] xfs_attr_get+0x9d/0xc0 [xfs] > [ 725.863021] xfs_get_acl+0xb6/0x200 [xfs] > [ 725.863036] get_acl+0x81/0x160 > [ 725.863052] posix_acl_xattr_get+0x3f/0xd0 > [ 725.863067] vfs_getxattr+0x148/0x170 > [ 725.863081] getxattr+0xa7/0x240 > [ 725.863093] path_getxattr+0x52/0x80 > [ 725.863111] do_syscall_64+0x5c/0xa0 > [ 725.863133] entry_SYSCALL_64_after_hwframe+0x49/0xb3 > [ 725.863149] > -> #0 (&xfs_nondir_ilock_class){++++}-{3:3}: > [ 725.863177] __lock_acquire+0x1257/0x20d0 > [ 725.863193] lock_acquire+0xb0/0x310 > [ 725.863207] down_write_nested+0x49/0x120 > [ 725.863242] xfs_ilock+0xde/0x2c0 [xfs] > [ 725.863277] xfs_reclaim_inode+0x3f/0x400 [xfs] > [ 725.863312] xfs_reclaim_inodes_ag+0x20b/0x410 [xfs] > [ 725.863351] xfs_reclaim_inodes_nr+0x31/0x40 [xfs] > [ 725.863368] super_cache_scan+0x190/0x1e0 > [ 725.863383] do_shrink_slab+0x184/0x420 > [ 725.863397] shrink_slab+0x182/0x290 > [ 725.863409] shrink_node+0x174/0x680 > [ 725.863927] balance_pgdat+0x2d0/0x5f0 > [ 725.864389] kswapd+0x21f/0x510 > [ 725.864836] kthread+0x131/0x150 > [ 725.865277] ret_from_fork+0x3a/0x50 > [ 725.865707] > other info that might help us debug this: > > [ 725.866953] Possible unsafe locking scenario: > > [ 725.867764] CPU0 CPU1 > [ 725.868161] ---- ---- > [ 725.868531] lock(fs_reclaim); > [ 725.868896] lock(&xfs_nondir_ilock_class); > [ 725.869276] lock(fs_reclaim); > [ 725.869633] lock(&xfs_nondir_ilock_class); > [ 725.869996] > *** DEADLOCK *** > > [ 725.871061] 4 locks held by kswapd0/159: > [ 725.871406] #0: ffffffffbbb8bd00 (fs_reclaim){+.+.}-{0:0}, at: > __fs_reclaim_acquire+0x5/0x30 > [ 725.871779] #1: ffffffffbbb7cef8 (shrinker_rwsem){++++}-{3:3}, at: > shrink_slab+0x115/0x290 > [ 725.872167] #2: ffff9b39f07a50e8 > (&type->s_umount_key#56){++++}-{3:3}, at: super_cache_scan+0x38/0x1e0 > [ 725.872560] #3: ffff9b39f077f258 > (&pag->pag_ici_reclaim_lock){+.+.}-{3:3}, at: > xfs_reclaim_inodes_ag+0x82/0x410 [xfs] > [ 725.873013] > stack backtrace: > [ 725.873811] CPU: 3 PID: 159 Comm: kswapd0 Not tainted > 5.7.0-0.rc5.20200515git1ae7efb38854.1.fc33.x86_64 #1 > [ 725.874249] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS ?-20180724_192412-buildhw-07.phx2.fedoraproject.org-1.fc29 > 04/01/2014 > [ 725.875158] Call Trace: > [ 725.875625] dump_stack+0x8b/0xc8 > [ 725.876090] check_noncircular+0x134/0x150 > [ 725.876547] __lock_acquire+0x1257/0x20d0 > [ 725.877019] lock_acquire+0xb0/0x310 > [ 725.877517] ? xfs_ilock+0xde/0x2c0 [xfs] > [ 725.877988] down_write_nested+0x49/0x120 > [ 725.878473] ? xfs_ilock+0xde/0x2c0 [xfs] > [ 725.878955] ? xfs_reclaim_inode+0x3f/0x400 [xfs] > [ 725.879448] xfs_ilock+0xde/0x2c0 [xfs] > [ 725.879925] xfs_reclaim_inode+0x3f/0x400 [xfs] > [ 725.880414] xfs_reclaim_inodes_ag+0x20b/0x410 [xfs] > [ 725.880876] ? sched_clock_cpu+0xc/0xb0 > [ 725.881343] ? mark_held_locks+0x2d/0x80 > [ 725.881798] ? _raw_spin_unlock_irqrestore+0x46/0x60 > [ 725.882268] ? lockdep_hardirqs_on+0x11e/0x1b0 > [ 725.882734] ? try_to_wake_up+0x249/0x820 > [ 725.883234] xfs_reclaim_inodes_nr+0x31/0x40 [xfs] > [ 725.883700] super_cache_scan+0x190/0x1e0 > [ 725.884180] do_shrink_slab+0x184/0x420 > [ 725.884653] shrink_slab+0x182/0x290 > [ 725.885129] shrink_node+0x174/0x680 > [ 725.885596] balance_pgdat+0x2d0/0x5f0 > [ 725.886074] kswapd+0x21f/0x510 > [ 725.886540] ? finish_wait+0x90/0x90 > [ 725.887013] ? balance_pgdat+0x5f0/0x5f0 > [ 725.887477] kthread+0x131/0x150 > [ 725.887937] ? __kthread_bind_mask+0x60/0x60 > [ 725.888410] ret_from_fork+0x3a/0x50 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 2020-05-21 23:13 ` lockdep trace with xfs + mm in it from 5.7.0-rc5 Darrick J. Wong @ 2020-05-22 0:30 ` Dave Chinner 2020-05-22 20:43 ` Darrick J. Wong 2020-05-22 20:51 ` Darrick J. Wong 0 siblings, 2 replies; 6+ messages in thread From: Dave Chinner @ 2020-05-22 0:30 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Dave Airlie, LKML, Linus Torvalds, xfs On Thu, May 21, 2020 at 04:13:12PM -0700, Darrick J. Wong wrote: > [cc linux-xfs] > > On Fri, May 22, 2020 at 08:21:50AM +1000, Dave Airlie wrote: > > Hi, > > > > Just updated a rawhide VM to the Fedora 5.7.0-rc5 kernel, did some > > package building, > > > > got the below trace, not sure if it's known and fixed or unknown. > > It's a known false-positive. An inode can't simultaneously be getting > reclaimed due to zero refcount /and/ be the target of a getxattr call. > Unfortunately, lockdep can't tell the difference, and it seems a little > strange to set NOFS on the allocation (which increases the chances of a > runtime error) just to quiet that down. __GFP_NOLOCKDEP is the intended flag to telling memory allocation that lockdep is stupid. However, it seems that the patches that were in progress some months ago to convert XFS to kmalloc interfaces and using GFP flags directly stalled - being able to mark locations like this with __GFP_NOLOCKDEP was one of the main reasons for getting rid of all the internal XFS memory allocation wrappers... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 2020-05-22 0:30 ` Dave Chinner @ 2020-05-22 20:43 ` Darrick J. Wong 2020-05-23 23:35 ` Dave Chinner 2020-05-22 20:51 ` Darrick J. Wong 1 sibling, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2020-05-22 20:43 UTC (permalink / raw) To: Dave Chinner; +Cc: Dave Airlie, LKML, Linus Torvalds, xfs On Fri, May 22, 2020 at 10:30:27AM +1000, Dave Chinner wrote: > On Thu, May 21, 2020 at 04:13:12PM -0700, Darrick J. Wong wrote: > > [cc linux-xfs] > > > > On Fri, May 22, 2020 at 08:21:50AM +1000, Dave Airlie wrote: > > > Hi, > > > > > > Just updated a rawhide VM to the Fedora 5.7.0-rc5 kernel, did some > > > package building, > > > > > > got the below trace, not sure if it's known and fixed or unknown. > > > > It's a known false-positive. An inode can't simultaneously be getting > > reclaimed due to zero refcount /and/ be the target of a getxattr call. > > Unfortunately, lockdep can't tell the difference, and it seems a little > > strange to set NOFS on the allocation (which increases the chances of a > > runtime error) just to quiet that down. > > __GFP_NOLOCKDEP is the intended flag to telling memory allocation > that lockdep is stupid. > > However, it seems that the patches that were in progress some months > ago to convert XFS to kmalloc interfaces and using GFP flags > directly stalled - being able to mark locations like this with > __GFP_NOLOCKDEP was one of the main reasons for getting rid of all > the internal XFS memory allocation wrappers... Question is, should I spend time adding a GFP_NOLOCKDEP bandaid to XFS or would my time be better spent reviewing your async inode reclaim series to make this go away for real? (Dang, now that I phrase it that way, Imma go read that series.) --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 2020-05-22 20:43 ` Darrick J. Wong @ 2020-05-23 23:35 ` Dave Chinner 0 siblings, 0 replies; 6+ messages in thread From: Dave Chinner @ 2020-05-23 23:35 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Dave Airlie, LKML, Linus Torvalds, xfs On Fri, May 22, 2020 at 01:43:08PM -0700, Darrick J. Wong wrote: > On Fri, May 22, 2020 at 10:30:27AM +1000, Dave Chinner wrote: > > On Thu, May 21, 2020 at 04:13:12PM -0700, Darrick J. Wong wrote: > > > [cc linux-xfs] > > > > > > On Fri, May 22, 2020 at 08:21:50AM +1000, Dave Airlie wrote: > > > > Hi, > > > > > > > > Just updated a rawhide VM to the Fedora 5.7.0-rc5 kernel, did some > > > > package building, > > > > > > > > got the below trace, not sure if it's known and fixed or unknown. > > > > > > It's a known false-positive. An inode can't simultaneously be getting > > > reclaimed due to zero refcount /and/ be the target of a getxattr call. > > > Unfortunately, lockdep can't tell the difference, and it seems a little > > > strange to set NOFS on the allocation (which increases the chances of a > > > runtime error) just to quiet that down. > > > > __GFP_NOLOCKDEP is the intended flag to telling memory allocation > > that lockdep is stupid. > > > > However, it seems that the patches that were in progress some months > > ago to convert XFS to kmalloc interfaces and using GFP flags > > directly stalled - being able to mark locations like this with > > __GFP_NOLOCKDEP was one of the main reasons for getting rid of all > > the internal XFS memory allocation wrappers... > > Question is, should I spend time adding a GFP_NOLOCKDEP bandaid to XFS > or would my time be better spent reviewing your async inode reclaim > series to make this go away for real? Heh. I started to write that async reclaim would make this go away, but then I realised it won't because we still do an XFS_ILOCK_EXCL call in xfs_inode_reclaim() right at the end to synchronise with anything that was blocked in the ILOCK during a lockless lookup waiting for reclaim to drop the lock after setting ip->i_ino = 0. So that patchset doesn't make the lockdep issues go away. I still need to work out if we can get rid of that ILOCK cycling in xfs_reclaim_inode() by changing the lockless lookup code, but that's a separate problem... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 2020-05-22 0:30 ` Dave Chinner 2020-05-22 20:43 ` Darrick J. Wong @ 2020-05-22 20:51 ` Darrick J. Wong 2020-05-25 5:00 ` Dave Airlie 1 sibling, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2020-05-22 20:51 UTC (permalink / raw) To: Dave Airlie, Dave Chinner; +Cc: LKML, Linus Torvalds, xfs OTOH, it didn't take that long to whip up a patch. Dave, does this fix your problem? --D --- xfs: more lockdep whackamole with kmem_alloc* Dave Airlie reported the following lockdep complaint: > ====================================================== > WARNING: possible circular locking dependency detected > 5.7.0-0.rc5.20200515git1ae7efb38854.1.fc33.x86_64 #1 Not tainted > ------------------------------------------------------ > kswapd0/159 is trying to acquire lock: > ffff9b38d01a4470 (&xfs_nondir_ilock_class){++++}-{3:3}, > at: xfs_ilock+0xde/0x2c0 [xfs] > > but task is already holding lock: > ffffffffbbb8bd00 (fs_reclaim){+.+.}-{0:0}, at: > __fs_reclaim_acquire+0x5/0x30 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (fs_reclaim){+.+.}-{0:0}: > fs_reclaim_acquire+0x34/0x40 > __kmalloc+0x4f/0x270 > kmem_alloc+0x93/0x1d0 [xfs] > kmem_alloc_large+0x4c/0x130 [xfs] > xfs_attr_copy_value+0x74/0xa0 [xfs] > xfs_attr_get+0x9d/0xc0 [xfs] > xfs_get_acl+0xb6/0x200 [xfs] > get_acl+0x81/0x160 > posix_acl_xattr_get+0x3f/0xd0 > vfs_getxattr+0x148/0x170 > getxattr+0xa7/0x240 > path_getxattr+0x52/0x80 > do_syscall_64+0x5c/0xa0 > entry_SYSCALL_64_after_hwframe+0x49/0xb3 > > -> #0 (&xfs_nondir_ilock_class){++++}-{3:3}: > __lock_acquire+0x1257/0x20d0 > lock_acquire+0xb0/0x310 > down_write_nested+0x49/0x120 > xfs_ilock+0xde/0x2c0 [xfs] > xfs_reclaim_inode+0x3f/0x400 [xfs] > xfs_reclaim_inodes_ag+0x20b/0x410 [xfs] > xfs_reclaim_inodes_nr+0x31/0x40 [xfs] > super_cache_scan+0x190/0x1e0 > do_shrink_slab+0x184/0x420 > shrink_slab+0x182/0x290 > shrink_node+0x174/0x680 > balance_pgdat+0x2d0/0x5f0 > kswapd+0x21f/0x510 > kthread+0x131/0x150 > ret_from_fork+0x3a/0x50 > > other info that might help us debug this: > > Possible unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(fs_reclaim); > lock(&xfs_nondir_ilock_class); > lock(fs_reclaim); > lock(&xfs_nondir_ilock_class); > > *** DEADLOCK *** > > 4 locks held by kswapd0/159: > #0: ffffffffbbb8bd00 (fs_reclaim){+.+.}-{0:0}, at: > __fs_reclaim_acquire+0x5/0x30 > #1: ffffffffbbb7cef8 (shrinker_rwsem){++++}-{3:3}, at: > shrink_slab+0x115/0x290 > #2: ffff9b39f07a50e8 > (&type->s_umount_key#56){++++}-{3:3}, at: super_cache_scan+0x38/0x1e0 > #3: ffff9b39f077f258 > (&pag->pag_ici_reclaim_lock){+.+.}-{3:3}, at: > xfs_reclaim_inodes_ag+0x82/0x410 [xfs] This is a known false positive because inodes cannot simultaneously be getting reclaimed and the target of a getxattr operation, but lockdep doesn't know that. We can (selectively) shut up lockdep until either it gets smarter or we change inode reclaim not to require the ILOCK by applying a stupid GFP_NOLOCKDEP bandaid. Reported-by: Dave Airlie <airlied@gmail.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- fs/xfs/kmem.h | 6 +++++- fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h index 6143117770e9..11623489b769 100644 --- a/fs/xfs/kmem.h +++ b/fs/xfs/kmem.h @@ -19,6 +19,7 @@ typedef unsigned __bitwise xfs_km_flags_t; #define KM_NOFS ((__force xfs_km_flags_t)0x0004u) #define KM_MAYFAIL ((__force xfs_km_flags_t)0x0008u) #define KM_ZERO ((__force xfs_km_flags_t)0x0010u) +#define KM_NOLOCKDEP ((__force xfs_km_flags_t)0x0020u) /* * We use a special process flag to avoid recursive callbacks into @@ -30,7 +31,7 @@ kmem_flags_convert(xfs_km_flags_t flags) { gfp_t lflags; - BUG_ON(flags & ~(KM_NOFS|KM_MAYFAIL|KM_ZERO)); + BUG_ON(flags & ~(KM_NOFS | KM_MAYFAIL | KM_ZERO | KM_NOLOCKDEP)); lflags = GFP_KERNEL | __GFP_NOWARN; if (flags & KM_NOFS) @@ -49,6 +50,9 @@ kmem_flags_convert(xfs_km_flags_t flags) if (flags & KM_ZERO) lflags |= __GFP_ZERO; + if (flags & KM_NOLOCKDEP) + lflags |= __GFP_NOLOCKDEP; + return lflags; } diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 224462b8bf08..55e184722b81 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -489,7 +489,7 @@ xfs_attr_copy_value( } if (!args->value) { - args->value = kmem_alloc_large(valuelen, 0); + args->value = kmem_alloc_large(valuelen, KM_NOLOCKDEP); if (!args->value) return -ENOMEM; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: lockdep trace with xfs + mm in it from 5.7.0-rc5 2020-05-22 20:51 ` Darrick J. Wong @ 2020-05-25 5:00 ` Dave Airlie 0 siblings, 0 replies; 6+ messages in thread From: Dave Airlie @ 2020-05-25 5:00 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Dave Chinner, LKML, Linus Torvalds, xfs On Sat, 23 May 2020 at 06:51, Darrick J. Wong <darrick.wong@oracle.com> wrote: > > OTOH, it didn't take that long to whip up a patch. > > Dave, does this fix your problem? I reproduced with 5.7.0-rc7, and tried this patch on top in the same VM doing the same thing. with this patch I no longer see the lockdep trace. Tested-by: Dave Airlie <airlied@gmail.com> Thanks, Dave. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-25 5:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAPM=9tyy5vubggbcj32bGpA_h6yDaBNM3QeJPySTzci-etfBZw@mail.gmail.com>
2020-05-21 23:13 ` lockdep trace with xfs + mm in it from 5.7.0-rc5 Darrick J. Wong
2020-05-22 0:30 ` Dave Chinner
2020-05-22 20:43 ` Darrick J. Wong
2020-05-23 23:35 ` Dave Chinner
2020-05-22 20:51 ` Darrick J. Wong
2020-05-25 5:00 ` Dave Airlie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).