Re: [syzbot] [xfs?] possible deadlock in xfs_fs_dirty_inode

["Darrick J. Wong" <djwong@kernel.org>]

On Sat, Apr 27, 2024 at 07:22:21AM +1000, Dave Chinner wrote:
> On Fri, Apr 26, 2024 at 09:30:08AM -0700, Darrick J. Wong wrote:
> > On Thu, Apr 25, 2024 at 10:15:29PM -0700, syzbot wrote:
> > > Hello,
> > > 
> > > syzbot found the following issue on:
> > > 
> > > HEAD commit:    3b68086599f8 Merge tag 'sched_urgent_for_v6.9_rc5' of git:..
> > > git tree:       upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=158206bb180000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f47e5e015c177e57
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=1619d847a7b9ba3a9137
> > > compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> > > 
> > > Unfortunately, I don't have any reproducer for this issue yet.
> > > 
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/caa90b55d476/disk-3b680865.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/17940f1c5e8f/vmlinux-3b680865.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/b03bd6929a1c/bzImage-3b680865.xz
> > > 
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+1619d847a7b9ba3a9137@syzkaller.appspotmail.com
> > > 
> > > ======================================================
> > > WARNING: possible circular locking dependency detected
> > > 6.9.0-rc4-syzkaller-00274-g3b68086599f8 #0 Not tainted
> > > ------------------------------------------------------
> > > kswapd0/81 is trying to acquire lock:
> > > ffff8881a895a610 (sb_internal#3){.+.+}-{0:0}, at: xfs_fs_dirty_inode+0x158/0x250 fs/xfs/xfs_super.c:689
> > > 
> > > but task is already holding lock:
> > > ffffffff8e428e80 (fs_reclaim){+.+.}-{0:0}, at: balance_pgdat mm/vmscan.c:6782 [inline]
> > > ffffffff8e428e80 (fs_reclaim){+.+.}-{0:0}, at: kswapd+0xb20/0x30c0 mm/vmscan.c:7164
> > > 
> > > which lock already depends on the new lock.
> > > 
> > > 
> > > the existing dependency chain (in reverse order) is:
> > > 
> > > -> #2 (fs_reclaim){+.+.}-{0:0}:
> > >        lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5754
> > >        __fs_reclaim_acquire mm/page_alloc.c:3698 [inline]
> > >        fs_reclaim_acquire+0x88/0x140 mm/page_alloc.c:3712
> > >        might_alloc include/linux/sched/mm.h:312 [inline]
> > >        slab_pre_alloc_hook mm/slub.c:3746 [inline]
> > >        slab_alloc_node mm/slub.c:3827 [inline]
> > >        kmalloc_trace+0x47/0x360 mm/slub.c:3992
> > >        kmalloc include/linux/slab.h:628 [inline]
> > >        add_stack_record_to_list mm/page_owner.c:177 [inline]
> 
> There's the GFP_KERNEL allocation being warned about again.
> 
> > >        inc_stack_record_count mm/page_owner.c:219 [inline]
> > >        __set_page_owner+0x561/0x810 mm/page_owner.c:334
> > >        set_page_owner include/linux/page_owner.h:32 [inline]
> > >        post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1534
> > >        prep_new_page mm/page_alloc.c:1541 [inline]
> > >        get_page_from_freelist+0x3410/0x35b0 mm/page_alloc.c:3317
> > >        __alloc_pages+0x256/0x6c0 mm/page_alloc.c:4575
> > >        __alloc_pages_node include/linux/gfp.h:238 [inline]
> > >        alloc_pages_node include/linux/gfp.h:261 [inline]
> > >        alloc_slab_page+0x5f/0x160 mm/slub.c:2175
> > >        allocate_slab mm/slub.c:2338 [inline]
> > >        new_slab+0x84/0x2f0 mm/slub.c:2391
> > >        ___slab_alloc+0xc73/0x1260 mm/slub.c:3525
> > >        __slab_alloc mm/slub.c:3610 [inline]
> > >        __slab_alloc_node mm/slub.c:3663 [inline]
> > >        slab_alloc_node mm/slub.c:3835 [inline]
> > >        kmem_cache_alloc+0x252/0x340 mm/slub.c:3852
> > >        kmem_cache_zalloc include/linux/slab.h:739 [inline]
> > >        xfs_btree_alloc_cursor fs/xfs/libxfs/xfs_btree.h:679 [inline]
> > >        xfs_refcountbt_init_cursor+0x65/0x2a0 fs/xfs/libxfs/xfs_refcount_btree.c:367
> > >        xfs_reflink_find_shared fs/xfs/xfs_reflink.c:147 [inline]
> > >        xfs_reflink_trim_around_shared+0x53a/0x9d0 fs/xfs/xfs_reflink.c:194
> > >        xfs_buffered_write_iomap_begin+0xebf/0x1b40 fs/xfs/xfs_iomap.c:1062
> > 
> > Hm.  We've taken an ILOCK in xfs_buffered_write_iomap_begin, and now
> > we're allocating a btree cursor but we don't have PF_MEMALLOC_NOFS set,
> > nor do we pass GFP_NOFS.
> > 
> > Ah, because nothing in this code path sets PF_MEMALLOC_NOFS explicitly,
> > nor does it create a xfs_trans_alloc_empty, which would set that.  Prior
> > to the removal of kmem_alloc, I think we were much more aggressive about
> > GFP_NOFS usage.
> > 
> > Seeing as we're about to walk a btree, we probably want the empty
> > transaction to guard against btree cycle livelocks.
> 
> Nothing like that is needed or desired, this is a just a bug in the
> memory allocation tracking code...

Not needed because it doesn't address the root cause of these two syzbot
reports?  Or did you actually analyze the refcount btree code and
discover that there's no possibility of livelocking on btree cycles?

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
>