On Tue, Nov 08, 2022 at 07:52:14AM -0800, Darrick J. Wong wrote:
> On Tue, Nov 08, 2022 at 02:02:44PM +0800, Zorro Lang wrote:
> > On Mon, Nov 07, 2022 at 08:41:47AM -0800, Darrick J. Wong wrote:
> > > On Sat, Nov 05, 2022 at 11:23:24PM +0800, Zorro Lang wrote:
> > > > An old issue might cause on-disk inode sizes are logged prematurely
> > > > via the free eofblocks path on file close. Then fs shutdown might
> > > > leave NULL files but their di_size > 0.
> > > >
> > > > Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> > > > ---
> > > >
> > > > V2 replace "fiemap" with "stat" command, to check if a file has extents.
> > > > That helps this case more common.
> > > >
> > > > Thanks,
> > > > Zorro
> > > >
> > > > tests/generic/999 | 42 ++++++++++++++++++++++++++++++++++++++++++
> > > > tests/generic/999.out | 5 +++++
> > > > 2 files changed, 47 insertions(+)
> > > > create mode 100755 tests/generic/999
> > > > create mode 100644 tests/generic/999.out
> > > >
> > > > diff --git a/tests/generic/999 b/tests/generic/999
> > > > new file mode 100755
> > > > index 00000000..8b4596e0
> > > > --- /dev/null
> > > > +++ b/tests/generic/999
> > >
> > > Ugh sorry ^^^^^^^ I didn't notice this part and wrote my previous
> > > response thinking this was an xfs-only test...
> >
> > Oh, my bad, I forgot to change the "_supported_fs xfs" to "generic".
> >
> > >
> > > > @@ -0,0 +1,42 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2022 Red Hat, Inc. All Rights Reserved.
> > > > +#
> > > > +# FS QA Test No. 999
> > > > +#
> > > > +# Test an issue in the truncate codepath where on-disk inode sizes are logged
> > > > +# prematurely via the free eofblocks path on file close.
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto quick shutdown
> > > > +
> > > > +# real QA test starts here
> > > > +_supported_fs xfs
> > > > +_require_scratch
> > > > +_require_scratch_shutdown
> > > > +_scratch_mkfs > $seqres.full 2>&1
> > > > +_scratch_mount
> > > > +
> > > > +echo "Create many small files with one extent at least"
> > > > +for ((i=0; i<10000; i++));do
> > > > + $XFS_IO_PROG -f -c "pwrite 0 4k" $SCRATCH_MNT/file.$i >/dev/null 2>&1
> > > > +done
> > > > +
> > > > +echo "Shutdown the fs suddently"
> > > > +_scratch_shutdown
> > > > +
> > > > +echo "Cycle mount"
> > > > +_scratch_cycle_mount
> > > > +
> > > > +echo "Check file's (di_size > 0) extents"
> > > > +for f in $(find $SCRATCH_MNT -type f -size +0);do
> > > > + # Check if the file has any extent
> > > > + if [ "$(stat -c "%b" $f)" = "0" ];then
> > > > + echo " - $f get no extents, but its di_size > 0"
> > > > + break
> > > > + fi
> > > > +done
> > >
> > > ...so whereas I was trying to suggest that you could use the GETFSXATTR
> > > ioctl to return the extent count:
> > >
> > > $XFS_IO_PROG -c stat $f | grep fsxattr.nextents | awk '{print $3}'
> > >
> > > But that won't work outside of XFS. To make this generic, I think you
> > > have to do something like:
> > >
> > > $FILEFRAG_PROG -v $f | wc -l
> >
> > I'm wondering if we must check extent is 0, how about check allocated block = 0?
> > I tried [ "$(stat -c "%b" $f)" = "0" ], it fails on old kernel without this bug
> > fix, and test passed on new kernel. Does that make sense for you?
>
> What if an LSM (e.g. selinux) attach enough security attrs to push the
> xattr data into an external block? I don't think that happens often for
Oh! You're right, I forgot the ablocks. Due to fstests always use
"-o context=system_u:object_r:root_t:s0" to mount, I thought there's not
selinux labels to take xattr space by default, so ablocks generally is 0 (I think:).
But I agree with you, "$(stat -c "%b" $f)" can be affected by ablocks, so
filefrag might be a better solution.
Thanks,
Zorro
> XFS, but I think it will for ext3 with 128byte inodes.
>
> --D
>
> > Thanks,
> > Zorro
> >
> > >
> > > to see if there are any extents.
> > >
> > > --D
> > >
> > > > +
> > > > +# success, all done
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/generic/999.out b/tests/generic/999.out
> > > > new file mode 100644
> > > > index 00000000..50008783
> > > > --- /dev/null
> > > > +++ b/tests/generic/999.out
> > > > @@ -0,0 +1,5 @@
> > > > +QA output created by 999
> > > > +Create many small files with one extent at least
> > > > +Shutdown the fs suddently
> > > > +Cycle mount
> > > > +Check file's (di_size > 0) extents
> > > > --
> > > > 2.31.1
> > > >
> > >
> >
>
