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

Dave Chinner
2024-04-26T21:22:25Z

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...

-Dave.