diff mbox series

[v1.2,8/8] xfs: test for premature ENOSPC with large cow delalloc extents

Message ID 20240307232255.GG1927156@frogsfrogsfrogs (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Darrick J. Wong March 7, 2024, 11:22 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

On a higly fragmented filesystem a Direct IO write can fail with -ENOSPC error
even though the filesystem has sufficient number of free blocks.

This occurs if the file offset range on which the write operation is being
performed has a delalloc extent in the cow fork and this delalloc extent
begins much before the Direct IO range.

In such a scenario, xfs_reflink_allocate_cow() invokes xfs_bmapi_write() to
allocate the blocks mapped by the delalloc extent. The extent thus allocated
may not cover the beginning of file offset range on which the Direct IO write
was issued. Hence xfs_reflink_allocate_cow() ends up returning -ENOSPC.

This test addresses this issue.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v1.1: address some missing bits and remove extraneous code
v1.2: fix cow fork dumping screwing up golden output
---
 common/rc          |   14 ++++++++
 tests/xfs/1923     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/1923.out |    8 +++++
 3 files changed, 108 insertions(+)
 create mode 100755 tests/xfs/1923
 create mode 100644 tests/xfs/1923.out

Comments

Zorro Lang March 10, 2024, 9:17 a.m. UTC | #1 
On Thu, Mar 07, 2024 at 03:22:55PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> On a higly fragmented filesystem a Direct IO write can fail with -ENOSPC error
> even though the filesystem has sufficient number of free blocks.
> 
> This occurs if the file offset range on which the write operation is being
> performed has a delalloc extent in the cow fork and this delalloc extent
> begins much before the Direct IO range.
> 
> In such a scenario, xfs_reflink_allocate_cow() invokes xfs_bmapi_write() to
> allocate the blocks mapped by the delalloc extent. The extent thus allocated
> may not cover the beginning of file offset range on which the Direct IO write
> was issued. Hence xfs_reflink_allocate_cow() ends up returning -ENOSPC.
> 
> This test addresses this issue.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v1.1: address some missing bits and remove extraneous code
> v1.2: fix cow fork dumping screwing up golden output

This version is good to me, I'll merge it.

Reviewed-by: Zorro Lang <zlang@redhat.com>

BTW, I only see this patch for [PATCH 8/8], but I didn't see the "later" patch
for [PATCH 6/8], just to make sure if I missed something :)

Thanks,
Zorro

> ---
>  common/rc          |   14 ++++++++
>  tests/xfs/1923     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/1923.out |    8 +++++
>  3 files changed, 108 insertions(+)
>  create mode 100755 tests/xfs/1923
>  create mode 100644 tests/xfs/1923.out
> 
> diff --git a/common/rc b/common/rc
> index 50dde313b8..9f54ab1e77 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1883,6 +1883,20 @@ _require_scratch_delalloc()
>  	_scratch_unmount
>  }
>  
> +# Require test fs supports delay allocation.
> +_require_test_delalloc()
> +{
> +	_require_command "$FILEFRAG_PROG" filefrag
> +
> +	rm -f $TEST_DIR/testy
> +	$XFS_IO_PROG -f -c 'pwrite 0 64k' $TEST_DIR/testy &> /dev/null
> +	$FILEFRAG_PROG -v $TEST_DIR/testy 2>&1 | grep -q delalloc
> +	res=$?
> +	rm -f $TEST_DIR/testy
> +	test $res -eq 0 || \
> +		_notrun "test requires delayed allocation buffered writes"
> +}
> +
>  # this test needs a test partition - check we're ok & mount it
>  #
>  _require_test()
> diff --git a/tests/xfs/1923 b/tests/xfs/1923
> new file mode 100755
> index 0000000000..4ad3dfa764
> --- /dev/null
> +++ b/tests/xfs/1923
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022-2024 Oracle.  All Rights Reserved.
> +#
> +# FS QA Test No. 1923
> +#
> +# This is a regression test for "xfs: Fix false ENOSPC when performing direct
> +# write on a delalloc extent in cow fork".  If there is a lot of free space but
> +# it is very fragmented, it's possible that a very large delalloc reservation
> +# could be created in the CoW fork by a buffered write.  If a directio write
> +# tries to convert the delalloc reservation to a real extent, it's possible
> +# that the allocation will succeed but fail to convert even the first block of
> +# the directio write range.  In this case, XFS will return ENOSPC even though
> +# all it needed to do was to keep converting until the allocator returns ENOSPC
> +# or the first block of the direct write got some space.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick clone
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $file1 $file2 $fragmentedfile
> +}
> +
> +# Import common functions.
> +. ./common/reflink
> +. ./common/inject
> +
> +# real QA test starts here
> +_fixed_by_kernel_commit d62113303d69 \
> +	"xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork"
> +
> +# Modify as appropriate.
> +_supported_fs xfs
> +_require_test_program "punch-alternating"
> +_require_test_reflink
> +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
> +_require_test_delalloc
> +
> +file1=$TEST_DIR/file1.$seq
> +file2=$TEST_DIR/file2.$seq
> +fragmentedfile=$TEST_DIR/fragmentedfile.$seq
> +
> +rm -f $file1 $file2 $fragmentedfile
> +
> +# COW operates on pages, so we must not perform operations in units smaller
> +# than a page.
> +blksz=$(_get_file_block_size $TEST_DIR)
> +pagesz=$(_get_page_size)
> +if (( $blksz < $pagesz )); then
> +	blksz=$pagesz
> +fi
> +
> +echo "Create source file"
> +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 256))" $file1 >> $seqres.full
> +
> +sync
> +
> +echo "Create Reflinked file"
> +_cp_reflink $file1 $file2 >> $seqres.full
> +
> +echo "Set cowextsize"
> +$XFS_IO_PROG -c "cowextsize $((blksz * 128))" -c stat $file1 >> $seqres.full
> +
> +echo "Fragment FS"
> +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 512))" $fragmentedfile >> $seqres.full
> +sync
> +$here/src/punch-alternating $fragmentedfile
> +
> +echo "Allocate block sized extent from now onwards"
> +_test_inject_error bmap_alloc_minlen_extent 1
> +
> +echo "Create big delalloc extent in CoW fork"
> +$XFS_IO_PROG -c "pwrite 0 $blksz" $file1 >> $seqres.full
> +
> +sync
> +
> +$XFS_IO_PROG -c 'bmap -elpv' -c 'bmap -celpv' $file1 &>> $seqres.full
> +
> +echo "Direct I/O write at offset 3FSB"
> +$XFS_IO_PROG -d -c "pwrite $((blksz * 3)) $((blksz * 2))" $file1 >> $seqres.full
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/1923.out b/tests/xfs/1923.out
> new file mode 100644
> index 0000000000..a0553cf3ee
> --- /dev/null
> +++ b/tests/xfs/1923.out
> @@ -0,0 +1,8 @@
> +QA output created by 1923
> +Create source file
> +Create Reflinked file
> +Set cowextsize
> +Fragment FS
> +Allocate block sized extent from now onwards
> +Create big delalloc extent in CoW fork
> +Direct I/O write at offset 3FSB
>
Darrick J. Wong March 10, 2024, 4:26 p.m. UTC | #2 
On Sun, Mar 10, 2024 at 05:17:34PM +0800, Zorro Lang wrote:
> On Thu, Mar 07, 2024 at 03:22:55PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > On a higly fragmented filesystem a Direct IO write can fail with -ENOSPC error
> > even though the filesystem has sufficient number of free blocks.
> > 
> > This occurs if the file offset range on which the write operation is being
> > performed has a delalloc extent in the cow fork and this delalloc extent
> > begins much before the Direct IO range.
> > 
> > In such a scenario, xfs_reflink_allocate_cow() invokes xfs_bmapi_write() to
> > allocate the blocks mapped by the delalloc extent. The extent thus allocated
> > may not cover the beginning of file offset range on which the Direct IO write
> > was issued. Hence xfs_reflink_allocate_cow() ends up returning -ENOSPC.
> > 
> > This test addresses this issue.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > v1.1: address some missing bits and remove extraneous code
> > v1.2: fix cow fork dumping screwing up golden output
> 
> This version is good to me, I'll merge it.
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> BTW, I only see this patch for [PATCH 8/8], but I didn't see the "later" patch
> for [PATCH 6/8], just to make sure if I missed something :)

Oh!  Yeah, on re-reading that thread, I remembered that Christoph said
he'd look into making xfs_ondisk.h check ioctl structure sizes like
xfs/122 currently does.

In the meantime, there weren't any changes other than your RVB tag.  If
you want, I can resend it with that added.

--D

> Thanks,
> Zorro
> 
> > ---
> >  common/rc          |   14 ++++++++
> >  tests/xfs/1923     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/1923.out |    8 +++++
> >  3 files changed, 108 insertions(+)
> >  create mode 100755 tests/xfs/1923
> >  create mode 100644 tests/xfs/1923.out
> > 
> > diff --git a/common/rc b/common/rc
> > index 50dde313b8..9f54ab1e77 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1883,6 +1883,20 @@ _require_scratch_delalloc()
> >  	_scratch_unmount
> >  }
> >  
> > +# Require test fs supports delay allocation.
> > +_require_test_delalloc()
> > +{
> > +	_require_command "$FILEFRAG_PROG" filefrag
> > +
> > +	rm -f $TEST_DIR/testy
> > +	$XFS_IO_PROG -f -c 'pwrite 0 64k' $TEST_DIR/testy &> /dev/null
> > +	$FILEFRAG_PROG -v $TEST_DIR/testy 2>&1 | grep -q delalloc
> > +	res=$?
> > +	rm -f $TEST_DIR/testy
> > +	test $res -eq 0 || \
> > +		_notrun "test requires delayed allocation buffered writes"
> > +}
> > +
> >  # this test needs a test partition - check we're ok & mount it
> >  #
> >  _require_test()
> > diff --git a/tests/xfs/1923 b/tests/xfs/1923
> > new file mode 100755
> > index 0000000000..4ad3dfa764
> > --- /dev/null
> > +++ b/tests/xfs/1923
> > @@ -0,0 +1,86 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022-2024 Oracle.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 1923
> > +#
> > +# This is a regression test for "xfs: Fix false ENOSPC when performing direct
> > +# write on a delalloc extent in cow fork".  If there is a lot of free space but
> > +# it is very fragmented, it's possible that a very large delalloc reservation
> > +# could be created in the CoW fork by a buffered write.  If a directio write
> > +# tries to convert the delalloc reservation to a real extent, it's possible
> > +# that the allocation will succeed but fail to convert even the first block of
> > +# the directio write range.  In this case, XFS will return ENOSPC even though
> > +# all it needed to do was to keep converting until the allocator returns ENOSPC
> > +# or the first block of the direct write got some space.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick clone
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $file1 $file2 $fragmentedfile
> > +}
> > +
> > +# Import common functions.
> > +. ./common/reflink
> > +. ./common/inject
> > +
> > +# real QA test starts here
> > +_fixed_by_kernel_commit d62113303d69 \
> > +	"xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork"
> > +
> > +# Modify as appropriate.
> > +_supported_fs xfs
> > +_require_test_program "punch-alternating"
> > +_require_test_reflink
> > +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
> > +_require_test_delalloc
> > +
> > +file1=$TEST_DIR/file1.$seq
> > +file2=$TEST_DIR/file2.$seq
> > +fragmentedfile=$TEST_DIR/fragmentedfile.$seq
> > +
> > +rm -f $file1 $file2 $fragmentedfile
> > +
> > +# COW operates on pages, so we must not perform operations in units smaller
> > +# than a page.
> > +blksz=$(_get_file_block_size $TEST_DIR)
> > +pagesz=$(_get_page_size)
> > +if (( $blksz < $pagesz )); then
> > +	blksz=$pagesz
> > +fi
> > +
> > +echo "Create source file"
> > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 256))" $file1 >> $seqres.full
> > +
> > +sync
> > +
> > +echo "Create Reflinked file"
> > +_cp_reflink $file1 $file2 >> $seqres.full
> > +
> > +echo "Set cowextsize"
> > +$XFS_IO_PROG -c "cowextsize $((blksz * 128))" -c stat $file1 >> $seqres.full
> > +
> > +echo "Fragment FS"
> > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 512))" $fragmentedfile >> $seqres.full
> > +sync
> > +$here/src/punch-alternating $fragmentedfile
> > +
> > +echo "Allocate block sized extent from now onwards"
> > +_test_inject_error bmap_alloc_minlen_extent 1
> > +
> > +echo "Create big delalloc extent in CoW fork"
> > +$XFS_IO_PROG -c "pwrite 0 $blksz" $file1 >> $seqres.full
> > +
> > +sync
> > +
> > +$XFS_IO_PROG -c 'bmap -elpv' -c 'bmap -celpv' $file1 &>> $seqres.full
> > +
> > +echo "Direct I/O write at offset 3FSB"
> > +$XFS_IO_PROG -d -c "pwrite $((blksz * 3)) $((blksz * 2))" $file1 >> $seqres.full
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/1923.out b/tests/xfs/1923.out
> > new file mode 100644
> > index 0000000000..a0553cf3ee
> > --- /dev/null
> > +++ b/tests/xfs/1923.out
> > @@ -0,0 +1,8 @@
> > +QA output created by 1923
> > +Create source file
> > +Create Reflinked file
> > +Set cowextsize
> > +Fragment FS
> > +Allocate block sized extent from now onwards
> > +Create big delalloc extent in CoW fork
> > +Direct I/O write at offset 3FSB
> > 
>
Zorro Lang March 11, 2024, 1:40 p.m. UTC | #3 
On Sun, Mar 10, 2024 at 09:26:54AM -0700, Darrick J. Wong wrote:
> On Sun, Mar 10, 2024 at 05:17:34PM +0800, Zorro Lang wrote:
> > On Thu, Mar 07, 2024 at 03:22:55PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > On a higly fragmented filesystem a Direct IO write can fail with -ENOSPC error
> > > even though the filesystem has sufficient number of free blocks.
> > > 
> > > This occurs if the file offset range on which the write operation is being
> > > performed has a delalloc extent in the cow fork and this delalloc extent
> > > begins much before the Direct IO range.
> > > 
> > > In such a scenario, xfs_reflink_allocate_cow() invokes xfs_bmapi_write() to
> > > allocate the blocks mapped by the delalloc extent. The extent thus allocated
> > > may not cover the beginning of file offset range on which the Direct IO write
> > > was issued. Hence xfs_reflink_allocate_cow() ends up returning -ENOSPC.
> > > 
> > > This test addresses this issue.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > > v1.1: address some missing bits and remove extraneous code
> > > v1.2: fix cow fork dumping screwing up golden output
> > 
> > This version is good to me, I'll merge it.
> > 
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > 
> > BTW, I only see this patch for [PATCH 8/8], but I didn't see the "later" patch
> > for [PATCH 6/8], just to make sure if I missed something :)
> 
> Oh!  Yeah, on re-reading that thread, I remembered that Christoph said
> he'd look into making xfs_ondisk.h check ioctl structure sizes like
> xfs/122 currently does.
> 
> In the meantime, there weren't any changes other than your RVB tag.  If
> you want, I can resend it with that added.

Oh, if that patch don't need to be changed, I'll merge it directly, don't
need resending it :)

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > > ---
> > >  common/rc          |   14 ++++++++
> > >  tests/xfs/1923     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/xfs/1923.out |    8 +++++
> > >  3 files changed, 108 insertions(+)
> > >  create mode 100755 tests/xfs/1923
> > >  create mode 100644 tests/xfs/1923.out
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 50dde313b8..9f54ab1e77 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1883,6 +1883,20 @@ _require_scratch_delalloc()
> > >  	_scratch_unmount
> > >  }
> > >  
> > > +# Require test fs supports delay allocation.
> > > +_require_test_delalloc()
> > > +{
> > > +	_require_command "$FILEFRAG_PROG" filefrag
> > > +
> > > +	rm -f $TEST_DIR/testy
> > > +	$XFS_IO_PROG -f -c 'pwrite 0 64k' $TEST_DIR/testy &> /dev/null
> > > +	$FILEFRAG_PROG -v $TEST_DIR/testy 2>&1 | grep -q delalloc
> > > +	res=$?
> > > +	rm -f $TEST_DIR/testy
> > > +	test $res -eq 0 || \
> > > +		_notrun "test requires delayed allocation buffered writes"
> > > +}
> > > +
> > >  # this test needs a test partition - check we're ok & mount it
> > >  #
> > >  _require_test()
> > > diff --git a/tests/xfs/1923 b/tests/xfs/1923
> > > new file mode 100755
> > > index 0000000000..4ad3dfa764
> > > --- /dev/null
> > > +++ b/tests/xfs/1923
> > > @@ -0,0 +1,86 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022-2024 Oracle.  All Rights Reserved.
> > > +#
> > > +# FS QA Test No. 1923
> > > +#
> > > +# This is a regression test for "xfs: Fix false ENOSPC when performing direct
> > > +# write on a delalloc extent in cow fork".  If there is a lot of free space but
> > > +# it is very fragmented, it's possible that a very large delalloc reservation
> > > +# could be created in the CoW fork by a buffered write.  If a directio write
> > > +# tries to convert the delalloc reservation to a real extent, it's possible
> > > +# that the allocation will succeed but fail to convert even the first block of
> > > +# the directio write range.  In this case, XFS will return ENOSPC even though
> > > +# all it needed to do was to keep converting until the allocator returns ENOSPC
> > > +# or the first block of the direct write got some space.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick clone
> > > +
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $file1 $file2 $fragmentedfile
> > > +}
> > > +
> > > +# Import common functions.
> > > +. ./common/reflink
> > > +. ./common/inject
> > > +
> > > +# real QA test starts here
> > > +_fixed_by_kernel_commit d62113303d69 \
> > > +	"xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork"
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs xfs
> > > +_require_test_program "punch-alternating"
> > > +_require_test_reflink
> > > +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
> > > +_require_test_delalloc
> > > +
> > > +file1=$TEST_DIR/file1.$seq
> > > +file2=$TEST_DIR/file2.$seq
> > > +fragmentedfile=$TEST_DIR/fragmentedfile.$seq
> > > +
> > > +rm -f $file1 $file2 $fragmentedfile
> > > +
> > > +# COW operates on pages, so we must not perform operations in units smaller
> > > +# than a page.
> > > +blksz=$(_get_file_block_size $TEST_DIR)
> > > +pagesz=$(_get_page_size)
> > > +if (( $blksz < $pagesz )); then
> > > +	blksz=$pagesz
> > > +fi
> > > +
> > > +echo "Create source file"
> > > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 256))" $file1 >> $seqres.full
> > > +
> > > +sync
> > > +
> > > +echo "Create Reflinked file"
> > > +_cp_reflink $file1 $file2 >> $seqres.full
> > > +
> > > +echo "Set cowextsize"
> > > +$XFS_IO_PROG -c "cowextsize $((blksz * 128))" -c stat $file1 >> $seqres.full
> > > +
> > > +echo "Fragment FS"
> > > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 512))" $fragmentedfile >> $seqres.full
> > > +sync
> > > +$here/src/punch-alternating $fragmentedfile
> > > +
> > > +echo "Allocate block sized extent from now onwards"
> > > +_test_inject_error bmap_alloc_minlen_extent 1
> > > +
> > > +echo "Create big delalloc extent in CoW fork"
> > > +$XFS_IO_PROG -c "pwrite 0 $blksz" $file1 >> $seqres.full
> > > +
> > > +sync
> > > +
> > > +$XFS_IO_PROG -c 'bmap -elpv' -c 'bmap -celpv' $file1 &>> $seqres.full
> > > +
> > > +echo "Direct I/O write at offset 3FSB"
> > > +$XFS_IO_PROG -d -c "pwrite $((blksz * 3)) $((blksz * 2))" $file1 >> $seqres.full
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/xfs/1923.out b/tests/xfs/1923.out
> > > new file mode 100644
> > > index 0000000000..a0553cf3ee
> > > --- /dev/null
> > > +++ b/tests/xfs/1923.out
> > > @@ -0,0 +1,8 @@
> > > +QA output created by 1923
> > > +Create source file
> > > +Create Reflinked file
> > > +Set cowextsize
> > > +Fragment FS
> > > +Allocate block sized extent from now onwards
> > > +Create big delalloc extent in CoW fork
> > > +Direct I/O write at offset 3FSB
> > > 
> > 
>
Darrick J. Wong March 11, 2024, 3:04 p.m. UTC | #4 
On Mon, Mar 11, 2024 at 09:40:01PM +0800, Zorro Lang wrote:
> On Sun, Mar 10, 2024 at 09:26:54AM -0700, Darrick J. Wong wrote:
> > On Sun, Mar 10, 2024 at 05:17:34PM +0800, Zorro Lang wrote:
> > > On Thu, Mar 07, 2024 at 03:22:55PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > On a higly fragmented filesystem a Direct IO write can fail with -ENOSPC error
> > > > even though the filesystem has sufficient number of free blocks.
> > > > 
> > > > This occurs if the file offset range on which the write operation is being
> > > > performed has a delalloc extent in the cow fork and this delalloc extent
> > > > begins much before the Direct IO range.
> > > > 
> > > > In such a scenario, xfs_reflink_allocate_cow() invokes xfs_bmapi_write() to
> > > > allocate the blocks mapped by the delalloc extent. The extent thus allocated
> > > > may not cover the beginning of file offset range on which the Direct IO write
> > > > was issued. Hence xfs_reflink_allocate_cow() ends up returning -ENOSPC.
> > > > 
> > > > This test addresses this issue.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > > v1.1: address some missing bits and remove extraneous code
> > > > v1.2: fix cow fork dumping screwing up golden output
> > > 
> > > This version is good to me, I'll merge it.
> > > 
> > > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > > 
> > > BTW, I only see this patch for [PATCH 8/8], but I didn't see the "later" patch
> > > for [PATCH 6/8], just to make sure if I missed something :)
> > 
> > Oh!  Yeah, on re-reading that thread, I remembered that Christoph said
> > he'd look into making xfs_ondisk.h check ioctl structure sizes like
> > xfs/122 currently does.
> > 
> > In the meantime, there weren't any changes other than your RVB tag.  If
> > you want, I can resend it with that added.
> 
> Oh, if that patch don't need to be changed, I'll merge it directly, don't
> need resending it :)

That's correct -- the patch itself doesn't need changes, but the test
itself may get removed some day.

--D

> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > > ---
> > > >  common/rc          |   14 ++++++++
> > > >  tests/xfs/1923     |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/xfs/1923.out |    8 +++++
> > > >  3 files changed, 108 insertions(+)
> > > >  create mode 100755 tests/xfs/1923
> > > >  create mode 100644 tests/xfs/1923.out
> > > > 
> > > > diff --git a/common/rc b/common/rc
> > > > index 50dde313b8..9f54ab1e77 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -1883,6 +1883,20 @@ _require_scratch_delalloc()
> > > >  	_scratch_unmount
> > > >  }
> > > >  
> > > > +# Require test fs supports delay allocation.
> > > > +_require_test_delalloc()
> > > > +{
> > > > +	_require_command "$FILEFRAG_PROG" filefrag
> > > > +
> > > > +	rm -f $TEST_DIR/testy
> > > > +	$XFS_IO_PROG -f -c 'pwrite 0 64k' $TEST_DIR/testy &> /dev/null
> > > > +	$FILEFRAG_PROG -v $TEST_DIR/testy 2>&1 | grep -q delalloc
> > > > +	res=$?
> > > > +	rm -f $TEST_DIR/testy
> > > > +	test $res -eq 0 || \
> > > > +		_notrun "test requires delayed allocation buffered writes"
> > > > +}
> > > > +
> > > >  # this test needs a test partition - check we're ok & mount it
> > > >  #
> > > >  _require_test()
> > > > diff --git a/tests/xfs/1923 b/tests/xfs/1923
> > > > new file mode 100755
> > > > index 0000000000..4ad3dfa764
> > > > --- /dev/null
> > > > +++ b/tests/xfs/1923
> > > > @@ -0,0 +1,86 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2022-2024 Oracle.  All Rights Reserved.
> > > > +#
> > > > +# FS QA Test No. 1923
> > > > +#
> > > > +# This is a regression test for "xfs: Fix false ENOSPC when performing direct
> > > > +# write on a delalloc extent in cow fork".  If there is a lot of free space but
> > > > +# it is very fragmented, it's possible that a very large delalloc reservation
> > > > +# could be created in the CoW fork by a buffered write.  If a directio write
> > > > +# tries to convert the delalloc reservation to a real extent, it's possible
> > > > +# that the allocation will succeed but fail to convert even the first block of
> > > > +# the directio write range.  In this case, XFS will return ENOSPC even though
> > > > +# all it needed to do was to keep converting until the allocator returns ENOSPC
> > > > +# or the first block of the direct write got some space.
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto quick clone
> > > > +
> > > > +_cleanup()
> > > > +{
> > > > +	cd /
> > > > +	rm -f $file1 $file2 $fragmentedfile
> > > > +}
> > > > +
> > > > +# Import common functions.
> > > > +. ./common/reflink
> > > > +. ./common/inject
> > > > +
> > > > +# real QA test starts here
> > > > +_fixed_by_kernel_commit d62113303d69 \
> > > > +	"xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork"
> > > > +
> > > > +# Modify as appropriate.
> > > > +_supported_fs xfs
> > > > +_require_test_program "punch-alternating"
> > > > +_require_test_reflink
> > > > +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
> > > > +_require_test_delalloc
> > > > +
> > > > +file1=$TEST_DIR/file1.$seq
> > > > +file2=$TEST_DIR/file2.$seq
> > > > +fragmentedfile=$TEST_DIR/fragmentedfile.$seq
> > > > +
> > > > +rm -f $file1 $file2 $fragmentedfile
> > > > +
> > > > +# COW operates on pages, so we must not perform operations in units smaller
> > > > +# than a page.
> > > > +blksz=$(_get_file_block_size $TEST_DIR)
> > > > +pagesz=$(_get_page_size)
> > > > +if (( $blksz < $pagesz )); then
> > > > +	blksz=$pagesz
> > > > +fi
> > > > +
> > > > +echo "Create source file"
> > > > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 256))" $file1 >> $seqres.full
> > > > +
> > > > +sync
> > > > +
> > > > +echo "Create Reflinked file"
> > > > +_cp_reflink $file1 $file2 >> $seqres.full
> > > > +
> > > > +echo "Set cowextsize"
> > > > +$XFS_IO_PROG -c "cowextsize $((blksz * 128))" -c stat $file1 >> $seqres.full
> > > > +
> > > > +echo "Fragment FS"
> > > > +$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 512))" $fragmentedfile >> $seqres.full
> > > > +sync
> > > > +$here/src/punch-alternating $fragmentedfile
> > > > +
> > > > +echo "Allocate block sized extent from now onwards"
> > > > +_test_inject_error bmap_alloc_minlen_extent 1
> > > > +
> > > > +echo "Create big delalloc extent in CoW fork"
> > > > +$XFS_IO_PROG -c "pwrite 0 $blksz" $file1 >> $seqres.full
> > > > +
> > > > +sync
> > > > +
> > > > +$XFS_IO_PROG -c 'bmap -elpv' -c 'bmap -celpv' $file1 &>> $seqres.full
> > > > +
> > > > +echo "Direct I/O write at offset 3FSB"
> > > > +$XFS_IO_PROG -d -c "pwrite $((blksz * 3)) $((blksz * 2))" $file1 >> $seqres.full
> > > > +
> > > > +# success, all done
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/xfs/1923.out b/tests/xfs/1923.out
> > > > new file mode 100644
> > > > index 0000000000..a0553cf3ee
> > > > --- /dev/null
> > > > +++ b/tests/xfs/1923.out
> > > > @@ -0,0 +1,8 @@
> > > > +QA output created by 1923
> > > > +Create source file
> > > > +Create Reflinked file
> > > > +Set cowextsize
> > > > +Fragment FS
> > > > +Allocate block sized extent from now onwards
> > > > +Create big delalloc extent in CoW fork
> > > > +Direct I/O write at offset 3FSB
> > > > 
> > > 
> > 
> 
>
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 50dde313b8..9f54ab1e77 100644
--- a/common/rc
+++ b/common/rc
@@ -1883,6 +1883,20 @@  _require_scratch_delalloc()
 	_scratch_unmount
 }
 
+# Require test fs supports delay allocation.
+_require_test_delalloc()
+{
+	_require_command "$FILEFRAG_PROG" filefrag
+
+	rm -f $TEST_DIR/testy
+	$XFS_IO_PROG -f -c 'pwrite 0 64k' $TEST_DIR/testy &> /dev/null
+	$FILEFRAG_PROG -v $TEST_DIR/testy 2>&1 | grep -q delalloc
+	res=$?
+	rm -f $TEST_DIR/testy
+	test $res -eq 0 || \
+		_notrun "test requires delayed allocation buffered writes"
+}
+
 # this test needs a test partition - check we're ok & mount it
 #
 _require_test()
diff --git a/tests/xfs/1923 b/tests/xfs/1923
new file mode 100755
index 0000000000..4ad3dfa764
--- /dev/null
+++ b/tests/xfs/1923
@@ -0,0 +1,86 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022-2024 Oracle.  All Rights Reserved.
+#
+# FS QA Test No. 1923
+#
+# This is a regression test for "xfs: Fix false ENOSPC when performing direct
+# write on a delalloc extent in cow fork".  If there is a lot of free space but
+# it is very fragmented, it's possible that a very large delalloc reservation
+# could be created in the CoW fork by a buffered write.  If a directio write
+# tries to convert the delalloc reservation to a real extent, it's possible
+# that the allocation will succeed but fail to convert even the first block of
+# the directio write range.  In this case, XFS will return ENOSPC even though
+# all it needed to do was to keep converting until the allocator returns ENOSPC
+# or the first block of the direct write got some space.
+#
+. ./common/preamble
+_begin_fstest auto quick clone
+
+_cleanup()
+{
+	cd /
+	rm -f $file1 $file2 $fragmentedfile
+}
+
+# Import common functions.
+. ./common/reflink
+. ./common/inject
+
+# real QA test starts here
+_fixed_by_kernel_commit d62113303d69 \
+	"xfs: Fix false ENOSPC when performing direct write on a delalloc extent in cow fork"
+
+# Modify as appropriate.
+_supported_fs xfs
+_require_test_program "punch-alternating"
+_require_test_reflink
+_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
+_require_test_delalloc
+
+file1=$TEST_DIR/file1.$seq
+file2=$TEST_DIR/file2.$seq
+fragmentedfile=$TEST_DIR/fragmentedfile.$seq
+
+rm -f $file1 $file2 $fragmentedfile
+
+# COW operates on pages, so we must not perform operations in units smaller
+# than a page.
+blksz=$(_get_file_block_size $TEST_DIR)
+pagesz=$(_get_page_size)
+if (( $blksz < $pagesz )); then
+	blksz=$pagesz
+fi
+
+echo "Create source file"
+$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 256))" $file1 >> $seqres.full
+
+sync
+
+echo "Create Reflinked file"
+_cp_reflink $file1 $file2 >> $seqres.full
+
+echo "Set cowextsize"
+$XFS_IO_PROG -c "cowextsize $((blksz * 128))" -c stat $file1 >> $seqres.full
+
+echo "Fragment FS"
+$XFS_IO_PROG -f -c "pwrite 0 $((blksz * 512))" $fragmentedfile >> $seqres.full
+sync
+$here/src/punch-alternating $fragmentedfile
+
+echo "Allocate block sized extent from now onwards"
+_test_inject_error bmap_alloc_minlen_extent 1
+
+echo "Create big delalloc extent in CoW fork"
+$XFS_IO_PROG -c "pwrite 0 $blksz" $file1 >> $seqres.full
+
+sync
+
+$XFS_IO_PROG -c 'bmap -elpv' -c 'bmap -celpv' $file1 &>> $seqres.full
+
+echo "Direct I/O write at offset 3FSB"
+$XFS_IO_PROG -d -c "pwrite $((blksz * 3)) $((blksz * 2))" $file1 >> $seqres.full
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/1923.out b/tests/xfs/1923.out
new file mode 100644
index 0000000000..a0553cf3ee
--- /dev/null
+++ b/tests/xfs/1923.out
@@ -0,0 +1,8 @@ 
+QA output created by 1923
+Create source file
+Create Reflinked file
+Set cowextsize
+Fragment FS
+Allocate block sized extent from now onwards
+Create big delalloc extent in CoW fork
+Direct I/O write at offset 3FSB