On Wed, Dec 18, 2024 at 8:19 PM Qu Wenruo <quwenruo.btrfs@xxxxxxx> wrote: > > > > 在 2024/12/19 06:39, Darrick J. Wong 写道: > > On Wed, Dec 18, 2024 at 09:07:26AM +1030, Qu Wenruo wrote: > >> > >> > >> 在 2024/12/18 03:52, Darrick J. Wong 写道: > >>> On Tue, Dec 17, 2024 at 08:26:33AM +0000, Filipe Manana wrote: > >>>> On Tue, Dec 17, 2024 at 8:14 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > >>>>> > >>>>> On Wed, Dec 11, 2024 at 03:09:40PM +0000, fdmanana@xxxxxxxxxx wrote: > >>>>>> The test also fails sporadically on xfs and the bug was already reported > >>>>>> to the xfs mailing list: > >>>>>> > >>>>>> https://lore.kernel.org/linux-xfs/CAL3q7H7cURmnkJfUUx44HM3q=xKmqHb80eRdisErD_x8rU4+0Q@xxxxxxxxxxxxxx/ > >>>>>> > >>>>> > >>>>> This version still doesn't seem to have the fs freeze/unfreeze that Darrick > >>>>> asked for in that thread. > >>>> > >>>> I don't get it, what's the freeze/unfreeze for? Where should they be placed? > >>>> Is it some way to get around the bug on xfs? > >>> > >>> freeze kicks the background inode gc thread so that the unlinked clones > >>> actually get freed before the swapon call. A less bighammer idea might > >>> be to call XFS_IOC_FREE_EOFBLOCKS which also kicks the garbage > >>> collectors. > >> > >> I'm wondering why this GC things can not be done inside XFS' swapon call? > >> > >> So that we don't need some per-fs workaround in a generic test case. > > > > I suppose one could call xfs_inodegc_flush from within swapon with the > > swap file's i_rwsem held, but now we're blocking swapon while we wait > > for some unbounded number of probably unrelated unlinked inodes to be > > freed on the off chance that one of them shared blocks. > > > > A better answer might be to run FALLOC_FL_UNSHARE on the file, but now > > we're making swapon more complex and potentially issuing a lot of IO to > > make that happen. If you can convince the fsdevel/mm folks that swapon > > is supposed to try to correct things it doesn't like in the file mapping > > (instead of returning EINVAL or whatever it does now) then we could add > > that to the syscall definition. So how do we proceed from here? The btrfs fix was merged into Linus' tree sometime ago (v6.13-rc5), and I would hate to lose test coverage in fstests. Since the bug seems to be hard to fix on XFS, shall I make the test _notun on xfs, or move it tests/btrfs? Zorro? Thanks. > > Sorry that I'm no familiar with XFS to provide any help, but the swapon > call on btrfs is already very complex. > > It needs to verify every extent of that file is not shared, and block > the subvolume from being snapshotted. > (The extent shareness check iteslf may already cause quite some IO) > > So at least to me, a little more extra logic and IO shouldn't be a huge > blockage, since we're already doing exactly that since the btrfs > swapfile support. > > Thanks, > Qu > > > > --D > > > >> Thanks, > >> Qu > >>> > >>> --D > >>> > >> > >> > > >
