| Message ID | 40c169580bd42e96faf11c7ce8805399dd0a180c.1722429630.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs/287: wait for subvolume deletion to complete | expand |
On Wed, Jul 31, 2024 at 01:41:24PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > The test deletes the subvolume and then immediately calls the logical > resolve ioctl to confirm the extent is not referenced by the subvolume > anymore. This however may often fail because the subvolume delete only > makes the subvolume not accessible to user space anymore, but the actual > deletion of the subvolume tree, and all its data references, happens in > the background in the cleaner kthread running in kernel space. > > So if by the time we do the query the cleaner kthread has not yet deleted > the subvolume tree, the test fails like this: > > $ ./check btrfs/287 > FSTYP -- btrfs > PLATFORM -- Linux/x86_64 debian0 5.14.0-btrfs-next-22 #1 SMP Tue Jul 30 16:31:55 WEST 2024 > MKFS_OPTIONS -- /dev/sdc > MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1 > > btrfs/287 0s ... - output mismatch (see /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad) > --- tests/btrfs/287.out 2024-07-30 17:40:49.037599612 +0100 > +++ /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad 2024-07-31 13:06:28.275728352 +0100 > @@ -82,12 +82,18 @@ > inode 257 offset 20971520 snap2 > inode 257 offset 12582912 snap2 > inode 257 offset 4194304 snap2 > +inode 257 offset 20971520 snap1 > +inode 257 offset 12582912 snap1 > +inode 257 offset 4194304 snap1 > inode 257 offset 20971520 root 5 > ... > (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/287.out /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad' to see the entire diff) > > HINT: You _MAY_ be missing kernel fix: > 0cad8f14d70c btrfs: fix backref walking not returning all inode refs > > Fix this by using the "subvolume sync" command to wait for the subvolume > to be deleted by the cleaner kthread before doing logical resolve queries. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: David Sterba <dsterba@suse.com>
在 2024/7/31 22:11, fdmanana@kernel.org 写道: > From: Filipe Manana <fdmanana@suse.com> > > The test deletes the subvolume and then immediately calls the logical > resolve ioctl to confirm the extent is not referenced by the subvolume > anymore. This however may often fail because the subvolume delete only > makes the subvolume not accessible to user space anymore, but the actual > deletion of the subvolume tree, and all its data references, happens in > the background in the cleaner kthread running in kernel space. > > So if by the time we do the query the cleaner kthread has not yet deleted > the subvolume tree, the test fails like this: > > $ ./check btrfs/287 > FSTYP -- btrfs > PLATFORM -- Linux/x86_64 debian0 5.14.0-btrfs-next-22 #1 SMP Tue Jul 30 16:31:55 WEST 2024 > MKFS_OPTIONS -- /dev/sdc > MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1 > > btrfs/287 0s ... - output mismatch (see /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad) > --- tests/btrfs/287.out 2024-07-30 17:40:49.037599612 +0100 > +++ /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad 2024-07-31 13:06:28.275728352 +0100 > @@ -82,12 +82,18 @@ > inode 257 offset 20971520 snap2 > inode 257 offset 12582912 snap2 > inode 257 offset 4194304 snap2 > +inode 257 offset 20971520 snap1 > +inode 257 offset 12582912 snap1 > +inode 257 offset 4194304 snap1 > inode 257 offset 20971520 root 5 > ... > (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/287.out /home/fdmanana/git/hub/xfstests/results//btrfs/287.out.bad' to see the entire diff) > > HINT: You _MAY_ be missing kernel fix: > 0cad8f14d70c btrfs: fix backref walking not returning all inode refs > > Fix this by using the "subvolume sync" command to wait for the subvolume > to be deleted by the cleaner kthread before doing logical resolve queries. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > tests/btrfs/287 | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/tests/btrfs/287 b/tests/btrfs/287 > index d6c04ea8..e88f4e0a 100755 > --- a/tests/btrfs/287 > +++ b/tests/btrfs/287 > @@ -147,6 +147,27 @@ query_logical_ino -o $second_extent_bytenr | filter_snapshot_ids > # Now delete the first snapshot and repeat the last 2 queries. > $BTRFS_UTIL_PROG subvolume delete -C $SCRATCH_MNT/snap1 | _filter_btrfs_subvol_delete > > +# Remount with a commit interval of 1s so that we wake up the cleaner kthread > +# (which does the actual subvolume tree deletion) and the transaction used by > +# the cleaner kthread to delete the tree gets committed sooner and we wait less > +# in the subvolume sync command below. > +_scratch_remount commit=1 > + > +# The subvolume delete only made the subvolume not accessible anymore, but its > +# tree and references to data extents are only deleted when the cleaner kthread > +# runs, so wait for the cleaner to finish. It isn't enough to pass -C/-c (commit > +# transaction) because the cleaner may run only after the transaction commit. > +# Most of the time we don't need this because the transaction kthread wakes up > +# the cleaner kthread, which deletes the subvolume before we query the extents > +# below, as this is a very small filesystem and therefore very fast. > +# Nevertheless it's racy and on slower machines it may fail often as well as on > +# kernels without the reworked async transaction commit (kernel commit > +# fdfbf020664b ("btrfs: rework async transaction committing"), which landed in > +# kernel 5.17) which changes timing and substantially increases the chances that > +# the cleaner already ran and deleted the subvolume tree by the time we do the > +# queries below. > +$BTRFS_UTIL_PROG subvolume sync $SCRATCH_MNT >> $seqres.full > + > # Query the second extent with an offset of 0, should return file offsets 12M > # and 20M for the default subvolume (root 5) and file offsets 4M, 12M and 20M > # for the second snapshot root.
diff --git a/tests/btrfs/287 b/tests/btrfs/287 index d6c04ea8..e88f4e0a 100755 --- a/tests/btrfs/287 +++ b/tests/btrfs/287 @@ -147,6 +147,27 @@ query_logical_ino -o $second_extent_bytenr | filter_snapshot_ids # Now delete the first snapshot and repeat the last 2 queries. $BTRFS_UTIL_PROG subvolume delete -C $SCRATCH_MNT/snap1 | _filter_btrfs_subvol_delete +# Remount with a commit interval of 1s so that we wake up the cleaner kthread +# (which does the actual subvolume tree deletion) and the transaction used by +# the cleaner kthread to delete the tree gets committed sooner and we wait less +# in the subvolume sync command below. +_scratch_remount commit=1 + +# The subvolume delete only made the subvolume not accessible anymore, but its +# tree and references to data extents are only deleted when the cleaner kthread +# runs, so wait for the cleaner to finish. It isn't enough to pass -C/-c (commit +# transaction) because the cleaner may run only after the transaction commit. +# Most of the time we don't need this because the transaction kthread wakes up +# the cleaner kthread, which deletes the subvolume before we query the extents +# below, as this is a very small filesystem and therefore very fast. +# Nevertheless it's racy and on slower machines it may fail often as well as on +# kernels without the reworked async transaction commit (kernel commit +# fdfbf020664b ("btrfs: rework async transaction committing"), which landed in +# kernel 5.17) which changes timing and substantially increases the chances that +# the cleaner already ran and deleted the subvolume tree by the time we do the +# queries below. +$BTRFS_UTIL_PROG subvolume sync $SCRATCH_MNT >> $seqres.full + # Query the second extent with an offset of 0, should return file offsets 12M # and 20M for the default subvolume (root 5) and file offsets 4M, 12M and 20M # for the second snapshot root.