| Message ID | 161836230182.2754991.16864806174255630147.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fstests: random fixes | expand |
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue, Apr 13, 2021 at 06:05:01PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Provide a mkfs helper to format the dm thin device when external devices > are in use, and fix the dmthin mount helper to support them. This fixes > regressions in generic/347 and generic/500 when external logs are in > use. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > common/dmthin | 9 ++++++++- > tests/generic/223 | 3 +++ > tests/generic/347 | 2 +- > tests/generic/500 | 2 +- > 4 files changed, 13 insertions(+), 3 deletions(-) > > > diff --git a/common/dmthin b/common/dmthin > index c58c3948..3b1c7d45 100644 > --- a/common/dmthin > +++ b/common/dmthin > @@ -218,10 +218,17 @@ _dmthin_set_fail() > > _dmthin_mount_options() > { > - echo `_common_dev_mount_options $*` $DMTHIN_VOL_DEV $SCRATCH_MNT > + _scratch_options mount > + echo `_common_dev_mount_options $*` $SCRATCH_OPTIONS $DMTHIN_VOL_DEV $SCRATCH_MNT > } > > _dmthin_mount() > { > _mount -t $FSTYP `_dmthin_mount_options $*` > } > + > +_dmthin_mkfs() > +{ > + _scratch_options mkfs > + _mkfs_dev $SCRATCH_OPTIONS $@ $DMTHIN_VOL_DEV > +} > diff --git a/tests/generic/223 b/tests/generic/223 > index 1f85efe5..a5ace82f 100755 > --- a/tests/generic/223 > +++ b/tests/generic/223 > @@ -43,6 +43,9 @@ for SUNIT_K in 8 16 32 64 128; do > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1 > _scratch_mount > > + # Make sure everything is on the data device > + $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT What does this do for non-xfs filesystems? Do we need a FSTYP check and do chattr only on XFS? Thanks, Eryu > + > for SIZE_MULT in 1 2 8 64 256; do > let SIZE=$SIZE_MULT*$SUNIT_BYTES > > diff --git a/tests/generic/347 b/tests/generic/347 > index cbc5150a..e970ac10 100755 > --- a/tests/generic/347 > +++ b/tests/generic/347 > @@ -31,7 +31,7 @@ _setup_thin() > { > _dmthin_init $BACKING_SIZE $VIRTUAL_SIZE > _dmthin_set_queue > - _mkfs_dev $DMTHIN_VOL_DEV > + _dmthin_mkfs > _dmthin_mount > } > > diff --git a/tests/generic/500 b/tests/generic/500 > index 085ddbf3..5ab2f78c 100755 > --- a/tests/generic/500 > +++ b/tests/generic/500 > @@ -68,7 +68,7 @@ CLUSTER_SIZE=$((64 * 1024 / 512)) # 64K > > _dmthin_init $BACKING_SIZE $VIRTUAL_SIZE $CLUSTER_SIZE 0 > _dmthin_set_fail > -_mkfs_dev $DMTHIN_VOL_DEV > +_dmthin_mkfs > _dmthin_mount > > # There're two bugs at here, one is dm-thin bug, the other is filesystem
On Sun, Apr 18, 2021 at 08:25:48PM +0800, Eryu Guan wrote: > > diff --git a/tests/generic/223 b/tests/generic/223 > > index 1f85efe5..a5ace82f 100755 > > --- a/tests/generic/223 > > +++ b/tests/generic/223 > > @@ -43,6 +43,9 @@ for SUNIT_K in 8 16 32 64 128; do > > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1 > > _scratch_mount > > > > + # Make sure everything is on the data device > > + $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT > > What does this do for non-xfs filesystems? Do we need a FSTYP check and > do chattr only on XFS? This clears the FS_NOTAIL_FL flag, which prevents tail merging, on the root directory of the mounted scratch file system. That should be harmless on non-xfs file systems; in fact, the only file system that even uses NOTAIL_FL flag is reiserfs, and the NOTAIL_FL flag is not set by default on the root directory of a newly created reiserfs file system. However, by default reiserfs does not support the FS_IOC_{GET,SET}FLAGS ioctl unless the mount option "attrs" is given. Why, I have no idea: root@kvm-xfstests:~# mount -t reiserfs /vtmp/foo.img /mnt root@kvm-xfstests:~# xfs_io -c 'chattr -t' /mnt xfs_io: cannot get flags on /mnt: Inappropriate ioctl for device So it might be a good idea to redirect stderr for the xfs_io invocation to /dev/null, for those file systems which do not support the FS_IOC_{GET,SET}FLAGS ioctls. I also have no idea why this helps for xfs --- I would think it's a no-op, but I'm guessing there's some magical side-effect which is taking place when FS_IOC_SETFLAGS ioctl is processed? Maybe it would be worth a comment explaining what is going on --- and whether this is going to make any difference if the patch series which unifies FS_IOC_{GETSET}FLAGS handling is merged? - Ted
On Sun, Apr 18, 2021 at 10:44:08AM -0400, Theodore Ts'o wrote: > On Sun, Apr 18, 2021 at 08:25:48PM +0800, Eryu Guan wrote: > > > diff --git a/tests/generic/223 b/tests/generic/223 > > > index 1f85efe5..a5ace82f 100755 > > > --- a/tests/generic/223 > > > +++ b/tests/generic/223 > > > @@ -43,6 +43,9 @@ for SUNIT_K in 8 16 32 64 128; do > > > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1 > > > _scratch_mount > > > > > > + # Make sure everything is on the data device > > > + $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT > > > > What does this do for non-xfs filesystems? Do we need a FSTYP check and > > do chattr only on XFS? > > This clears the FS_NOTAIL_FL flag, which prevents tail merging, on the No, this is not the 'chattr' CLI program. This is the xfs_io 'chattr' command, and they have different attribute namespaces. See xfs_io for the definitions, but in this case: t inherit realtime flag (XFS_XFLAG_RTINHERIT) And so clearing that flag ensures that all newly created files are on the data device, as per the comment... > I also have no idea why this helps for xfs --- I would think it's a > no-op, Because it's not what you think it is. :) Cheers, Dave.
On Mon, Apr 19, 2021 at 12:04:42PM +1000, Dave Chinner wrote: > On Sun, Apr 18, 2021 at 10:44:08AM -0400, Theodore Ts'o wrote: > > On Sun, Apr 18, 2021 at 08:25:48PM +0800, Eryu Guan wrote: > > > > diff --git a/tests/generic/223 b/tests/generic/223 > > > > index 1f85efe5..a5ace82f 100755 > > > > --- a/tests/generic/223 > > > > +++ b/tests/generic/223 > > > > @@ -43,6 +43,9 @@ for SUNIT_K in 8 16 32 64 128; do > > > > _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1 > > > > _scratch_mount > > > > > > > > + # Make sure everything is on the data device > > > > + $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT > > > > > > What does this do for non-xfs filesystems? Do we need a FSTYP check and > > > do chattr only on XFS? Yes, I think so. Will fix. > > This clears the FS_NOTAIL_FL flag, which prevents tail merging, on the > > No, this is not the 'chattr' CLI program. This is the xfs_io > 'chattr' command, and they have different attribute namespaces. See > xfs_io for the definitions, but in this case: > > t inherit realtime flag (XFS_XFLAG_RTINHERIT) > > And so clearing that flag ensures that all newly created files > are on the data device, as per the comment... > > > I also have no idea why this helps for xfs --- I would think it's a > > no-op, > > Because it's not what you think it is. :) (Indeed.) --D > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/common/dmthin b/common/dmthin index c58c3948..3b1c7d45 100644 --- a/common/dmthin +++ b/common/dmthin @@ -218,10 +218,17 @@ _dmthin_set_fail() _dmthin_mount_options() { - echo `_common_dev_mount_options $*` $DMTHIN_VOL_DEV $SCRATCH_MNT + _scratch_options mount + echo `_common_dev_mount_options $*` $SCRATCH_OPTIONS $DMTHIN_VOL_DEV $SCRATCH_MNT } _dmthin_mount() { _mount -t $FSTYP `_dmthin_mount_options $*` } + +_dmthin_mkfs() +{ + _scratch_options mkfs + _mkfs_dev $SCRATCH_OPTIONS $@ $DMTHIN_VOL_DEV +} diff --git a/tests/generic/223 b/tests/generic/223 index 1f85efe5..a5ace82f 100755 --- a/tests/generic/223 +++ b/tests/generic/223 @@ -43,6 +43,9 @@ for SUNIT_K in 8 16 32 64 128; do _scratch_mkfs_geom $SUNIT_BYTES 4 $BLOCKSIZE >> $seqres.full 2>&1 _scratch_mount + # Make sure everything is on the data device + $XFS_IO_PROG -c 'chattr -t' $SCRATCH_MNT + for SIZE_MULT in 1 2 8 64 256; do let SIZE=$SIZE_MULT*$SUNIT_BYTES diff --git a/tests/generic/347 b/tests/generic/347 index cbc5150a..e970ac10 100755 --- a/tests/generic/347 +++ b/tests/generic/347 @@ -31,7 +31,7 @@ _setup_thin() { _dmthin_init $BACKING_SIZE $VIRTUAL_SIZE _dmthin_set_queue - _mkfs_dev $DMTHIN_VOL_DEV + _dmthin_mkfs _dmthin_mount } diff --git a/tests/generic/500 b/tests/generic/500 index 085ddbf3..5ab2f78c 100755 --- a/tests/generic/500 +++ b/tests/generic/500 @@ -68,7 +68,7 @@ CLUSTER_SIZE=$((64 * 1024 / 512)) # 64K _dmthin_init $BACKING_SIZE $VIRTUAL_SIZE $CLUSTER_SIZE 0 _dmthin_set_fail -_mkfs_dev $DMTHIN_VOL_DEV +_dmthin_mkfs _dmthin_mount # There're two bugs at here, one is dm-thin bug, the other is filesystem