* [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write
@ 2021-08-24 0:37 Darrick J. Wong
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Darrick J. Wong @ 2021-08-24 0:37 UTC (permalink / raw)
To: xfs
From: Darrick J. Wong <djwong@kernel.org>
While prototyping a free space defragmentation tool, I observed an
unexpected IO error while running a sequence of commands that can be
recreated by the following sequence of commands:
# xfs_io -f -c "pwrite -S 0x58 -b 10m 0 10m" file1
# cp --reflink=always file1 file2
# punch-alternating -o 1 file2
# xfs_io -c "funshare 0 10m" file2
fallocate: Input/output error
I then scraped this (abbreviated) stack trace from dmesg:
WARNING: CPU: 0 PID: 30788 at fs/iomap/buffered-io.c:577 iomap_write_begin+0x376/0x450
CPU: 0 PID: 30788 Comm: xfs_io Not tainted 5.14.0-rc6-xfsx #rc6 5ef57b62a900814b3e4d885c755e9014541c8732
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:iomap_write_begin+0x376/0x450
RSP: 0018:ffffc90000c0fc20 EFLAGS: 00010297
RAX: 0000000000000001 RBX: ffffc90000c0fd10 RCX: 0000000000001000
RDX: ffffc90000c0fc54 RSI: 000000000000000c RDI: 000000000000000c
RBP: ffff888005d5dbd8 R08: 0000000000102000 R09: ffffc90000c0fc50
R10: 0000000000b00000 R11: 0000000000101000 R12: ffffea0000336c40
R13: 0000000000001000 R14: ffffc90000c0fd10 R15: 0000000000101000
FS: 00007f4b8f62fe40(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000056361c554108 CR3: 000000000524e004 CR4: 00000000001706f0
Call Trace:
iomap_unshare_actor+0x95/0x140
iomap_apply+0xfa/0x300
iomap_file_unshare+0x44/0x60
xfs_reflink_unshare+0x50/0x140 [xfs 61947ea9b3a73e79d747dbc1b90205e7987e4195]
xfs_file_fallocate+0x27c/0x610 [xfs 61947ea9b3a73e79d747dbc1b90205e7987e4195]
vfs_fallocate+0x133/0x330
__x64_sys_fallocate+0x3e/0x70
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f4b8f79140a
Looking at the iomap tracepoints, I saw this:
iomap_iter: dev 8:64 ino 0x100 pos 0 length 0 flags WRITE|0x80 (0x81) ops xfs_buffered_write_iomap_ops caller iomap_file_unshare
iomap_iter_dstmap: dev 8:64 ino 0x100 bdev 8:64 addr -1 offset 0 length 131072 type DELALLOC flags SHARED
iomap_iter_srcmap: dev 8:64 ino 0x100 bdev 8:64 addr 147456 offset 0 length 4096 type MAPPED flags
iomap_iter: dev 8:64 ino 0x100 pos 0 length 4096 flags WRITE|0x80 (0x81) ops xfs_buffered_write_iomap_ops caller iomap_file_unshare
iomap_iter_dstmap: dev 8:64 ino 0x100 bdev 8:64 addr -1 offset 4096 length 4096 type DELALLOC flags SHARED
console: WARNING: CPU: 0 PID: 30788 at fs/iomap/buffered-io.c:577 iomap_write_begin+0x376/0x450
The first time funshare calls ->iomap_begin, xfs sees that the first
block is shared and creates a 128k delalloc reservation in the COW fork.
The delalloc reservation is returned as dstmap, and the shared block is
returned as srcmap. So far so good.
funshare calls ->iomap_begin to try the second block. This time there's
no srcmap (punch-alternating punched it out!) but we still have the
delalloc reservation in the COW fork. Therefore, we again return the
reservation as dstmap and the hole as srcmap. iomap_unshare_iter
incorrectly tries to unshare the hole, which __iomap_write_begin rejects
because shared regions must be fully written and therefore cannot
require zeroing.
Therefore, change the buffered write iomap_begin function not to set
IOMAP_F_SHARED when there isn't a source mapping to read from for the
unsharing.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_iomap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 7c69b124a475..abf6d60945ab 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -870,6 +870,7 @@ xfs_buffered_write_iomap_begin(
struct xfs_bmbt_irec imap, cmap;
struct xfs_iext_cursor icur, ccur;
xfs_fsblock_t prealloc_blocks = 0;
+ u16 cflags = 0;
bool eof = false, cow_eof = false, shared = false;
int allocfork = XFS_DATA_FORK;
int error = 0;
@@ -1061,6 +1062,7 @@ xfs_buffered_write_iomap_begin(
found_cow:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
if (imap.br_startoff <= offset_fsb) {
+ cflags = IOMAP_F_SHARED;
error = xfs_bmbt_to_iomap(ip, srcmap, &imap, 0);
if (error)
return error;
@@ -1068,7 +1070,7 @@ xfs_buffered_write_iomap_begin(
xfs_trim_extent(&cmap, offset_fsb,
imap.br_startoff - offset_fsb);
}
- return xfs_bmbt_to_iomap(ip, iomap, &cmap, IOMAP_F_SHARED);
+ return xfs_bmbt_to_iomap(ip, iomap, &cmap, cflags);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS
2021-08-24 0:37 [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Darrick J. Wong
@ 2021-08-24 0:38 ` Darrick J. Wong
2021-08-24 7:35 ` Christoph Hellwig
2021-08-24 9:33 ` Zorro Lang
2021-08-24 7:32 ` [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Christoph Hellwig
2021-08-24 11:03 ` Chandan Babu R
2 siblings, 2 replies; 7+ messages in thread
From: Darrick J. Wong @ 2021-08-24 0:38 UTC (permalink / raw)
To: xfs
From: Darrick J. Wong <djwong@kernel.org>
This is a regression test for "xfs: only set IOMAP_F_SHARED when
providing a srcmap to a write".
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
tests/generic/729 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/729.out | 2 +
2 files changed, 75 insertions(+)
create mode 100755 tests/generic/729
create mode 100644 tests/generic/729.out
diff --git a/tests/generic/729 b/tests/generic/729
new file mode 100755
index 00000000..269aed65
--- /dev/null
+++ b/tests/generic/729
@@ -0,0 +1,73 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 Oracle. All Rights Reserved.
+#
+# FS QA Test 729
+#
+# This is a regression test for "xfs: only set IOMAP_F_SHARED when providing a
+# srcmap to a write". If a user creates a sparse shared region in a file,
+# convinces XFS to create a copy-on-write delayed allocation reservation
+# spanning both the shared blocks and the holes, and then calls the fallocate
+# unshare command to unshare the entire sparse region, XFS incorrectly tells
+# iomap that the delalloc blocks for the holes are shared, which causes it to
+# error out while trying to unshare a hole.
+#
+. ./common/preamble
+_begin_fstest auto clone unshare
+
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ rm -r -f $tmp.* $TEST_DIR/$seq
+}
+
+# Import common functions.
+. ./common/reflink
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_require_cp_reflink
+_require_test_reflink
+_require_test_program "punch-alternating"
+_require_xfs_io_command "fpunch"
+_require_xfs_io_command "funshare"
+
+mkdir $TEST_DIR/$seq
+file1=$TEST_DIR/$seq/a
+file2=$TEST_DIR/$seq/b
+
+$XFS_IO_PROG -f -c "pwrite -S 0x58 -b 10m 0 10m" $file1 >> $seqres.full
+
+f1sum0="$(md5sum $file1 | _filter_test_dir)"
+
+_cp_reflink $file1 $file2
+$here/src/punch-alternating -o 1 $file2
+
+f2sum0="$(md5sum $file2 | _filter_test_dir)"
+
+# set cowextsize to the defaults (128k) to force delalloc cow preallocations
+test "$FSTYP" = "xfs" && $XFS_IO_PROG -c 'cowextsize 0' $file2
+$XFS_IO_PROG -c "funshare 0 10m" $file2
+
+f1sum1="$(md5sum $file1 | _filter_test_dir)"
+f2sum1="$(md5sum $file2 | _filter_test_dir)"
+
+test "${f1sum0}" = "${f1sum1}" || echo "file1 should not have changed"
+test "${f2sum0}" = "${f2sum1}" || echo "file2 should not have changed"
+
+_test_cycle_mount
+
+f1sum2="$(md5sum $file1 | _filter_test_dir)"
+f2sum2="$(md5sum $file2 | _filter_test_dir)"
+
+test "${f1sum2}" = "${f1sum1}" || echo "file1 should not have changed ondisk"
+test "${f2sum2}" = "${f2sum1}" || echo "file2 should not have changed ondisk"
+
+# success, all done
+echo Silence is golden
+status=0
+exit
diff --git a/tests/generic/729.out b/tests/generic/729.out
new file mode 100644
index 00000000..0f175ae2
--- /dev/null
+++ b/tests/generic/729.out
@@ -0,0 +1,2 @@
+QA output created by 729
+Silence is golden
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write
2021-08-24 0:37 [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Darrick J. Wong
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
@ 2021-08-24 7:32 ` Christoph Hellwig
2021-08-24 11:03 ` Chandan Babu R
2 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2021-08-24 7:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
This looks fine. But the cmap flag somehow tripped me up, and while
looking for a better name I just came up with the variant below that
avoids the need for the local variable entirely. Either version is
fine with me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index d8cd2583dedbf..66f138437b425 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -1064,11 +1064,11 @@ xfs_buffered_write_iomap_begin(
error = xfs_bmbt_to_iomap(ip, srcmap, &imap, 0);
if (error)
return error;
- } else {
- xfs_trim_extent(&cmap, offset_fsb,
- imap.br_startoff - offset_fsb);
+ return xfs_bmbt_to_iomap(ip, iomap, &cmap, IOMAP_F_SHARED);
}
- return xfs_bmbt_to_iomap(ip, iomap, &cmap, IOMAP_F_SHARED);
+
+ xfs_trim_extent(&cmap, offset_fsb, imap.br_startoff - offset_fsb);
+ return xfs_bmbt_to_iomap(ip, iomap, &cmap, 0);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
@ 2021-08-24 7:35 ` Christoph Hellwig
2021-08-24 9:33 ` Zorro Lang
1 sibling, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2021-08-24 7:35 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
2021-08-24 7:35 ` Christoph Hellwig
@ 2021-08-24 9:33 ` Zorro Lang
2021-08-24 15:17 ` Darrick J. Wong
1 sibling, 1 reply; 7+ messages in thread
From: Zorro Lang @ 2021-08-24 9:33 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs, fstests
On Mon, Aug 23, 2021 at 05:38:35PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> This is a regression test for "xfs: only set IOMAP_F_SHARED when
> providing a srcmap to a write".
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> tests/generic/729 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/729.out | 2 +
> 2 files changed, 75 insertions(+)
> create mode 100755 tests/generic/729
> create mode 100644 tests/generic/729.out
>
> diff --git a/tests/generic/729 b/tests/generic/729
> new file mode 100755
> index 00000000..269aed65
> --- /dev/null
> +++ b/tests/generic/729
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 Oracle. All Rights Reserved.
> +#
> +# FS QA Test 729
> +#
> +# This is a regression test for "xfs: only set IOMAP_F_SHARED when providing a
> +# srcmap to a write". If a user creates a sparse shared region in a file,
> +# convinces XFS to create a copy-on-write delayed allocation reservation
> +# spanning both the shared blocks and the holes, and then calls the fallocate
> +# unshare command to unshare the entire sparse region, XFS incorrectly tells
> +# iomap that the delalloc blocks for the holes are shared, which causes it to
> +# error out while trying to unshare a hole.
> +#
> +. ./common/preamble
> +_begin_fstest auto clone unshare
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> + cd /
> + rm -r -f $tmp.* $TEST_DIR/$seq
> +}
> +
> +# Import common functions.
> +. ./common/reflink
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_require_cp_reflink
> +_require_test_reflink
> +_require_test_program "punch-alternating"
> +_require_xfs_io_command "fpunch"
I didn't find "fpunch" in this case, but find "cowextsize". Did I
miss something?
Others looks good to me.
Reviewed-by: Zorro Lang <zlang@redhat.com>
cc fstests@vger.kernel.org, due to it's a patch to xfstests.
Thanks,
Zorro
> +_require_xfs_io_command "funshare"
> +
> +mkdir $TEST_DIR/$seq
> +file1=$TEST_DIR/$seq/a
> +file2=$TEST_DIR/$seq/b
> +
> +$XFS_IO_PROG -f -c "pwrite -S 0x58 -b 10m 0 10m" $file1 >> $seqres.full
> +
> +f1sum0="$(md5sum $file1 | _filter_test_dir)"
> +
> +_cp_reflink $file1 $file2
> +$here/src/punch-alternating -o 1 $file2
> +
> +f2sum0="$(md5sum $file2 | _filter_test_dir)"
> +
> +# set cowextsize to the defaults (128k) to force delalloc cow preallocations
> +test "$FSTYP" = "xfs" && $XFS_IO_PROG -c 'cowextsize 0' $file2
> +$XFS_IO_PROG -c "funshare 0 10m" $file2
> +
> +f1sum1="$(md5sum $file1 | _filter_test_dir)"
> +f2sum1="$(md5sum $file2 | _filter_test_dir)"
> +
> +test "${f1sum0}" = "${f1sum1}" || echo "file1 should not have changed"
> +test "${f2sum0}" = "${f2sum1}" || echo "file2 should not have changed"
> +
> +_test_cycle_mount
> +
> +f1sum2="$(md5sum $file1 | _filter_test_dir)"
> +f2sum2="$(md5sum $file2 | _filter_test_dir)"
> +
> +test "${f1sum2}" = "${f1sum1}" || echo "file1 should not have changed ondisk"
> +test "${f2sum2}" = "${f2sum1}" || echo "file2 should not have changed ondisk"
> +
> +# success, all done
> +echo Silence is golden
> +status=0
> +exit
> diff --git a/tests/generic/729.out b/tests/generic/729.out
> new file mode 100644
> index 00000000..0f175ae2
> --- /dev/null
> +++ b/tests/generic/729.out
> @@ -0,0 +1,2 @@
> +QA output created by 729
> +Silence is golden
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write
2021-08-24 0:37 [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Darrick J. Wong
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
2021-08-24 7:32 ` [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Christoph Hellwig
@ 2021-08-24 11:03 ` Chandan Babu R
2 siblings, 0 replies; 7+ messages in thread
From: Chandan Babu R @ 2021-08-24 11:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On 24 Aug 2021 at 06:07, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> While prototyping a free space defragmentation tool, I observed an
> unexpected IO error while running a sequence of commands that can be
> recreated by the following sequence of commands:
>
> # xfs_io -f -c "pwrite -S 0x58 -b 10m 0 10m" file1
> # cp --reflink=always file1 file2
> # punch-alternating -o 1 file2
> # xfs_io -c "funshare 0 10m" file2
> fallocate: Input/output error
>
> I then scraped this (abbreviated) stack trace from dmesg:
>
> WARNING: CPU: 0 PID: 30788 at fs/iomap/buffered-io.c:577 iomap_write_begin+0x376/0x450
> CPU: 0 PID: 30788 Comm: xfs_io Not tainted 5.14.0-rc6-xfsx #rc6 5ef57b62a900814b3e4d885c755e9014541c8732
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:iomap_write_begin+0x376/0x450
> RSP: 0018:ffffc90000c0fc20 EFLAGS: 00010297
> RAX: 0000000000000001 RBX: ffffc90000c0fd10 RCX: 0000000000001000
> RDX: ffffc90000c0fc54 RSI: 000000000000000c RDI: 000000000000000c
> RBP: ffff888005d5dbd8 R08: 0000000000102000 R09: ffffc90000c0fc50
> R10: 0000000000b00000 R11: 0000000000101000 R12: ffffea0000336c40
> R13: 0000000000001000 R14: ffffc90000c0fd10 R15: 0000000000101000
> FS: 00007f4b8f62fe40(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000056361c554108 CR3: 000000000524e004 CR4: 00000000001706f0
> Call Trace:
> iomap_unshare_actor+0x95/0x140
> iomap_apply+0xfa/0x300
> iomap_file_unshare+0x44/0x60
> xfs_reflink_unshare+0x50/0x140 [xfs 61947ea9b3a73e79d747dbc1b90205e7987e4195]
> xfs_file_fallocate+0x27c/0x610 [xfs 61947ea9b3a73e79d747dbc1b90205e7987e4195]
> vfs_fallocate+0x133/0x330
> __x64_sys_fallocate+0x3e/0x70
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f4b8f79140a
>
> Looking at the iomap tracepoints, I saw this:
>
> iomap_iter: dev 8:64 ino 0x100 pos 0 length 0 flags WRITE|0x80 (0x81) ops xfs_buffered_write_iomap_ops caller iomap_file_unshare
> iomap_iter_dstmap: dev 8:64 ino 0x100 bdev 8:64 addr -1 offset 0 length 131072 type DELALLOC flags SHARED
> iomap_iter_srcmap: dev 8:64 ino 0x100 bdev 8:64 addr 147456 offset 0 length 4096 type MAPPED flags
> iomap_iter: dev 8:64 ino 0x100 pos 0 length 4096 flags WRITE|0x80 (0x81) ops xfs_buffered_write_iomap_ops caller iomap_file_unshare
> iomap_iter_dstmap: dev 8:64 ino 0x100 bdev 8:64 addr -1 offset 4096 length 4096 type DELALLOC flags SHARED
> console: WARNING: CPU: 0 PID: 30788 at fs/iomap/buffered-io.c:577 iomap_write_begin+0x376/0x450
>
> The first time funshare calls ->iomap_begin, xfs sees that the first
> block is shared and creates a 128k delalloc reservation in the COW fork.
> The delalloc reservation is returned as dstmap, and the shared block is
> returned as srcmap. So far so good.
>
> funshare calls ->iomap_begin to try the second block. This time there's
> no srcmap (punch-alternating punched it out!) but we still have the
> delalloc reservation in the COW fork. Therefore, we again return the
> reservation as dstmap and the hole as srcmap. iomap_unshare_iter
> incorrectly tries to unshare the hole, which __iomap_write_begin rejects
> because shared regions must be fully written and therefore cannot
> require zeroing.
>
> Therefore, change the buffered write iomap_begin function not to set
> IOMAP_F_SHARED when there isn't a source mapping to read from for the
> unsharing.
Indeed, the hole following the 0th extent isn't a "shared extent" as was
conveyed by xfs_buffered_write_iomap_begin(). With this patch applied,
iomap_unshare_actor() returns early since the extent described by iomap isn't
shared.
I am fine with either your version of the fix or the fix authored by
Christoph.
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/xfs_iomap.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 7c69b124a475..abf6d60945ab 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -870,6 +870,7 @@ xfs_buffered_write_iomap_begin(
> struct xfs_bmbt_irec imap, cmap;
> struct xfs_iext_cursor icur, ccur;
> xfs_fsblock_t prealloc_blocks = 0;
> + u16 cflags = 0;
> bool eof = false, cow_eof = false, shared = false;
> int allocfork = XFS_DATA_FORK;
> int error = 0;
> @@ -1061,6 +1062,7 @@ xfs_buffered_write_iomap_begin(
> found_cow:
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> if (imap.br_startoff <= offset_fsb) {
> + cflags = IOMAP_F_SHARED;
> error = xfs_bmbt_to_iomap(ip, srcmap, &imap, 0);
> if (error)
> return error;
> @@ -1068,7 +1070,7 @@ xfs_buffered_write_iomap_begin(
> xfs_trim_extent(&cmap, offset_fsb,
> imap.br_startoff - offset_fsb);
> }
> - return xfs_bmbt_to_iomap(ip, iomap, &cmap, IOMAP_F_SHARED);
> + return xfs_bmbt_to_iomap(ip, iomap, &cmap, cflags);
>
> out_unlock:
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
--
chandan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS
2021-08-24 9:33 ` Zorro Lang
@ 2021-08-24 15:17 ` Darrick J. Wong
0 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2021-08-24 15:17 UTC (permalink / raw)
To: xfs, fstests
On Tue, Aug 24, 2021 at 05:33:28PM +0800, Zorro Lang wrote:
> On Mon, Aug 23, 2021 at 05:38:35PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > This is a regression test for "xfs: only set IOMAP_F_SHARED when
> > providing a srcmap to a write".
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > tests/generic/729 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/generic/729.out | 2 +
> > 2 files changed, 75 insertions(+)
> > create mode 100755 tests/generic/729
> > create mode 100644 tests/generic/729.out
> >
> > diff --git a/tests/generic/729 b/tests/generic/729
> > new file mode 100755
> > index 00000000..269aed65
> > --- /dev/null
> > +++ b/tests/generic/729
> > @@ -0,0 +1,73 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Oracle. All Rights Reserved.
> > +#
> > +# FS QA Test 729
> > +#
> > +# This is a regression test for "xfs: only set IOMAP_F_SHARED when providing a
> > +# srcmap to a write". If a user creates a sparse shared region in a file,
> > +# convinces XFS to create a copy-on-write delayed allocation reservation
> > +# spanning both the shared blocks and the holes, and then calls the fallocate
> > +# unshare command to unshare the entire sparse region, XFS incorrectly tells
> > +# iomap that the delalloc blocks for the holes are shared, which causes it to
> > +# error out while trying to unshare a hole.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto clone unshare
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > + cd /
> > + rm -r -f $tmp.* $TEST_DIR/$seq
> > +}
> > +
> > +# Import common functions.
> > +. ./common/reflink
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_require_cp_reflink
> > +_require_test_reflink
> > +_require_test_program "punch-alternating"
> > +_require_xfs_io_command "fpunch"
>
> I didn't find "fpunch" in this case,
It covers the punch activities in punch-alternating. I'll add a comment
pointing out that subtlety.
> but find "cowextsize". Did I miss something?
Oops. Yes, I'll add that too.
> Others looks good to me.
> Reviewed-by: Zorro Lang <zlang@redhat.com>
>
> cc fstests@vger.kernel.org, due to it's a patch to xfstests.
<nod> Thanks for reviewing.
--D
> Thanks,
> Zorro
>
> > +_require_xfs_io_command "funshare"
> > +
> > +mkdir $TEST_DIR/$seq
> > +file1=$TEST_DIR/$seq/a
> > +file2=$TEST_DIR/$seq/b
> > +
> > +$XFS_IO_PROG -f -c "pwrite -S 0x58 -b 10m 0 10m" $file1 >> $seqres.full
> > +
> > +f1sum0="$(md5sum $file1 | _filter_test_dir)"
> > +
> > +_cp_reflink $file1 $file2
> > +$here/src/punch-alternating -o 1 $file2
> > +
> > +f2sum0="$(md5sum $file2 | _filter_test_dir)"
> > +
> > +# set cowextsize to the defaults (128k) to force delalloc cow preallocations
> > +test "$FSTYP" = "xfs" && $XFS_IO_PROG -c 'cowextsize 0' $file2
> > +$XFS_IO_PROG -c "funshare 0 10m" $file2
> > +
> > +f1sum1="$(md5sum $file1 | _filter_test_dir)"
> > +f2sum1="$(md5sum $file2 | _filter_test_dir)"
> > +
> > +test "${f1sum0}" = "${f1sum1}" || echo "file1 should not have changed"
> > +test "${f2sum0}" = "${f2sum1}" || echo "file2 should not have changed"
> > +
> > +_test_cycle_mount
> > +
> > +f1sum2="$(md5sum $file1 | _filter_test_dir)"
> > +f2sum2="$(md5sum $file2 | _filter_test_dir)"
> > +
> > +test "${f1sum2}" = "${f1sum1}" || echo "file1 should not have changed ondisk"
> > +test "${f2sum2}" = "${f2sum1}" || echo "file2 should not have changed ondisk"
> > +
> > +# success, all done
> > +echo Silence is golden
> > +status=0
> > +exit
> > diff --git a/tests/generic/729.out b/tests/generic/729.out
> > new file mode 100644
> > index 00000000..0f175ae2
> > --- /dev/null
> > +++ b/tests/generic/729.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 729
> > +Silence is golden
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-24 15:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 0:37 [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Darrick J. Wong
2021-08-24 0:38 ` [RFC PATCH] generic: regression test for a FALLOC_FL_UNSHARE bug in XFS Darrick J. Wong
2021-08-24 7:35 ` Christoph Hellwig
2021-08-24 9:33 ` Zorro Lang
2021-08-24 15:17 ` Darrick J. Wong
2021-08-24 7:32 ` [PATCH] xfs: only set IOMAP_F_SHARED when providing a srcmap to a write Christoph Hellwig
2021-08-24 11:03 ` Chandan Babu R
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.