On Thu, Jul 10, 2025 at 5:08 PM Zorro Lang <zlang@xxxxxxxxxx> wrote:
>
> On Thu, Jul 10, 2025 at 12:47:30PM +0100, Filipe Manana wrote:
> > On Thu, Jul 10, 2025 at 8:29 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Jul 09, 2025 at 09:53:50AM +0100, fdmanana@xxxxxxxxxx wrote:
> > > > From: Filipe Manana <fdmanana@xxxxxxxx>
> > > >
> > > > Test that overwriting a file with mmap when the filesystem has no more
> > > > space available for data allocation works. The motivation here is to check
> > > > that NOCOW mode of a COW filesystem (such as btrfs) works as expected.
> > > >
> > > > This currently fails with btrfs but it's fixed by a kernel patch that has
> > > > the subject:
> > > >
> > > > btrfs: fix -ENOSPC mmap write failure on NOCOW files/extents
> > > >
> > > > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> > > > ---
> > > > tests/generic/211 | 58 +++++++++++++++++++++++++++++++++++++++++++
> > > > tests/generic/211.out | 6 +++++
> > > > 2 files changed, 64 insertions(+)
> > > > create mode 100755 tests/generic/211
> > > > create mode 100644 tests/generic/211.out
> > > >
> > > > diff --git a/tests/generic/211 b/tests/generic/211
> > > > new file mode 100755
> > > > index 00000000..c77508fe
> > > > --- /dev/null
> > > > +++ b/tests/generic/211
> > > > @@ -0,0 +1,58 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2025 SUSE Linux Products GmbH. All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 211
> > > > +#
> > > > +# Test that overwriting a file with mmap when the filesystem has no more space
> > > > +# available for data allocation works. The motivation here is to check that
> > > > +# NOCOW mode of a COW filesystem (such as btrfs) works as expected.
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto quick rw mmap
> > > > +
> > > > +. ./common/filter
> > > > +
> > > > +_require_scratch
> > > > +
> > > > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> > > > + "btrfs: fix -ENOSPC mmap write failure on NOCOW files/extents"
> > > > +
> > > > +# Use a 512M fs so that it's fast to fill it with data but not too small such
> > > > +# that on btrfs it results in a fs with mixed block groups - we want to have
> > > > +# dedicated block groups for data and metadata, so that after filling all the
> > > > +# data block groups we can do a NOCOW write with mmap (if we have enough free
> > > > +# metadata space available).
> > > > +fs_size=$(_small_fs_size_mb 512)
> > > > +_scratch_mkfs_sized $((fs_size * 1024 * 1024)) >>$seqres.full 2>&1 || \
> > > > + _fail "mkfs failed"
> > >
> > > _scratch_mkfs_sized calls _notrun if it fails:
> > >
> > > _scratch_mkfs_sized()
> > > {
> > > _try_scratch_mkfs_sized "$@" || _notrun "_scratch_mkfs_sized failed with ($*)"
> > > }
> > >
> > > So you can let it _notrun:
> > > _scratch_mkfs_sized $((fs_size * 1024 * 1024)) >>$seqres.full 2>&1
> > >
> > > or you'd like to _fail:
> > >
> > > _try_scratch_mkfs_sized $((fs_size * 1024 * 1024)) >>$seqres.full 2>&1 || \
> > > _fail "mkfs failed"
> >
> > The fail makes more sense to me - having the _scratch_mkfs_sized()
> > calling _notrun doesn't make sense to me at the moment.
>
> OK, so you can use _try_scratch_mkfs_sized.
>
> >
> > >
> > > > +_scratch_mount
> > > > +
> > > > +touch $SCRATCH_MNT/foobar
> > > > +
> > > > +# Set the file to NOCOW mode on btrfs, which must be done while the file is
> > > > +# empty, otherwise it fails.
> > > > +if [ $FSTYP == "btrfs" ]; then
> > > > + _require_chattr C
> > > > + $CHATTR_PROG +C $SCRATCH_MNT/foobar
> > > > +fi
> > > > +
> > > > +# Add initial data to the file we will later overwrite with mmap.
> > > > +$XFS_IO_PROG -c "pwrite -S 0xab 0 1M" $SCRATCH_MNT/foobar | _filter_xfs_io
> > > > +
> > > > +# Now fill all the remaining space with data.
> > > > +blksz=$(_get_block_size $SCRATCH_MNT)
> > > > +dd if=/dev/zero of=$SCRATCH_MNT/filler bs=$blksz >>$seqres.full 2>&1
> > >
> > > As this's a generic test case, I'm wondering if the common/populate:_fill_fs()
> > > helps?
> >
> > Because the intention is to exhaust data space and leave enough
> > metadata space free so that the NOCOW write can be done, as Qu already
> > replied before.
> > In btrfs we have space divided in two sections (block groups): one for
> > data and one for metadata.
> > Metadata is always COWed in btrfs by design, data can be NOCOWed
> > (chattr +C or prealloc extents for the first write) - as longs as
> > there's enough metadata space available (every data write requires
> > updating some metadata).
> >
> > If this is too specific or hard to comprehend, I can move the test
> > case into tests/btrfs/ (we have a few similar to this scenario iirc).
> > Let me know if I shall move it into tests/btrfs/.
>
> Thanks for the detailed explanation from you and Qu:) If so, maybe
> _get_file_block_size better than _get_block_size?
>
> I think we can keep this case in generic directory, it's a btrfs bug reproducer
> and it won't break other fs test. If someone fs hopes to change that "fill fs"
> part, we can use if...else... so maybe you can add more comments about why btrfs
> need this dd operation rather than _fill_fs things, to avoid others change this
> part in the future.
Ok, I'll update the comment above dd to be more detailed, about why dd
and not __populate_fill_fs().
Thanks.
>
> Thanks,
> Zorro
>
> >
> > Thanks.
> >
> > >
> > > Thanks,
> > > Zorro
> > >
> > > > +
> > > > +# Overwrite the file with a mmap write. Should succeed.
> > > > +$XFS_IO_PROG -c "mmap -w 0 1M" \
> > > > + -c "mwrite -S 0xcd 0 1M" \
> > > > + -c "munmap" \
> > > > + $SCRATCH_MNT/foobar
> > > > +
> > > > +# Cycle mount and dump the file's content. We expect to see the new data.
> > > > +_scratch_cycle_mount
> > > > +_hexdump $SCRATCH_MNT/foobar
> > > > +
> > > > +# success, all done
> > > > +_exit 0
> > > > diff --git a/tests/generic/211.out b/tests/generic/211.out
> > > > new file mode 100644
> > > > index 00000000..71cdf0f8
> > > > --- /dev/null
> > > > +++ b/tests/generic/211.out
> > > > @@ -0,0 +1,6 @@
> > > > +QA output created by 211
> > > > +wrote 1048576/1048576 bytes at offset 0
> > > > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > > +000000 cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd cd >................<
> > > > +*
> > > > +100000
> > > > --
> > > > 2.47.2
> > > >
> > > >
> > >
> >
>
