[PATCH] fstests: generic/098 update test for truncating a file into the middle of a hole

[PATCH] fstests: generic/098 update test for truncating a file into the middle of a hole

[Liu Bo]

This updates generic/098 by adding a sync option, i.e. 'sync' after the second
write, and with btrfs's NO_HOLES, we could still get wrong isize after remount.

This gets fixed by the patch

'Btrfs: fix truncate down when no_holes feature is enabled'

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 tests/generic/098     | 57 ++++++++++++++++++++++++++++++++-------------------
 tests/generic/098.out | 10 +++++++++
 2 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/tests/generic/098 b/tests/generic/098
index 838bb5d..3b89939 100755
--- a/tests/generic/098
+++ b/tests/generic/098
@@ -64,27 +64,42 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 _scratch_mount
 
-# Create our test file with some data and durably persist it.
-$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
-sync
-
-# Append some data to the file, increasing its size, and leave a hole between
-# the old size and the start offset if the following write. So our file gets
-# a hole in the range [128Kb, 256Kb[.
-$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
-
-# Now truncate our file to a smaller size that is in the middle of the hole we
-# previously created. On most truncate implementations the data we appended
-# before gets discarded from memory (with truncate_setsize()) and never ends
-# up being written to disk.
-$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
-
-_scratch_cycle_mount
-
-# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
-# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
-echo "File content after remount:"
-od -t x1 $SCRATCH_MNT/foo
+workout()
+{
+	NEED_SYNC=$1
+
+	# Create our test file with some data and durably persist it.
+	$XFS_IO_PROG -t -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
+	sync
+
+	# Append some data to the file, increasing its size, and leave a hole between
+	# the old size and the start offset if the following write. So our file gets
+	# a hole in the range [128Kb, 256Kb[.
+	$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
+
+	if [ $NEED_SYNC -eq 1 ]; then
+		sync
+	fi
+
+	# Now truncate our file to a smaller size that is in the middle of the hole we
+	# previously created.
+	# If we don't flush dirty page cache above, on most truncate
+	# implementations the data we appended before gets discarded from
+	# memory (with truncate_setsize()) and never ends up being written to
+	# disk.
+	$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
+
+	_scratch_cycle_mount
+
+	# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
+	# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
+	echo "File content after remount:"
+	od -t x1 $SCRATCH_MNT/foo
+}
+
+workout 0
+# flush after each write
+workout 1
 
 status=0
 exit
diff --git a/tests/generic/098.out b/tests/generic/098.out
index 37415ee..f87f046 100644
--- a/tests/generic/098.out
+++ b/tests/generic/098.out
@@ -9,3 +9,13 @@ File content after remount:
 0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *
 0500000
+wrote 131072/131072 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 32768/32768 bytes at offset 262144
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+File content after remount:
+0000000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
+*
+0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+*
+0500000
-- 
2.5.0

Re: [PATCH] fstests: generic/098 update test for truncating a file into the middle of a hole

[Eryu Guan]

On Fri, Nov 11, 2016 at 02:30:04PM -0800, Liu Bo wrote:
> This updates generic/098 by adding a sync option, i.e. 'sync' after the second
> write, and with btrfs's NO_HOLES, we could still get wrong isize after remount.
> 
> This gets fixed by the patch
> 
> 'Btrfs: fix truncate down when no_holes feature is enabled'
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Looks good to me, just some nitpicks inline :)

> ---
>  tests/generic/098     | 57 ++++++++++++++++++++++++++++++++-------------------
>  tests/generic/098.out | 10 +++++++++
>  2 files changed, 46 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/generic/098 b/tests/generic/098
> index 838bb5d..3b89939 100755
> --- a/tests/generic/098
> +++ b/tests/generic/098
> @@ -64,27 +64,42 @@ rm -f $seqres.full
>  _scratch_mkfs >>$seqres.full 2>&1
>  _scratch_mount
>  
> -# Create our test file with some data and durably persist it.
> -$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
> -sync
> -
> -# Append some data to the file, increasing its size, and leave a hole between
> -# the old size and the start offset if the following write. So our file gets
> -# a hole in the range [128Kb, 256Kb[.
> -$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
> -
> -# Now truncate our file to a smaller size that is in the middle of the hole we
> -# previously created. On most truncate implementations the data we appended
> -# before gets discarded from memory (with truncate_setsize()) and never ends
> -# up being written to disk.
> -$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
> -
> -_scratch_cycle_mount
> -
> -# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
> -# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
> -echo "File content after remount:"
> -od -t x1 $SCRATCH_MNT/foo
> +workout()
> +{
> +	NEED_SYNC=$1

Use "local" to declare this var, and in lower case. Usually we use upper
case for global variables.

> +
> +	# Create our test file with some data and durably persist it.
> +	$XFS_IO_PROG -t -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
> +	sync
> +
> +	# Append some data to the file, increasing its size, and leave a hole between
> +	# the old size and the start offset if the following write. So our file gets
> +	# a hole in the range [128Kb, 256Kb[.
> +	$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
> +
> +	if [ $NEED_SYNC -eq 1 ]; then
> +		sync
> +	fi

Good to see some comments to explain why we need this to test
with/without sync case.

Thanks,
Eryu

> +
> +	# Now truncate our file to a smaller size that is in the middle of the hole we
> +	# previously created.
> +	# If we don't flush dirty page cache above, on most truncate
> +	# implementations the data we appended before gets discarded from
> +	# memory (with truncate_setsize()) and never ends up being written to
> +	# disk.
> +	$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
> +
> +	_scratch_cycle_mount
> +
> +	# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
> +	# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
> +	echo "File content after remount:"
> +	od -t x1 $SCRATCH_MNT/foo
> +}
> +
> +workout 0
> +# flush after each write
> +workout 1
>  
>  status=0
>  exit
> diff --git a/tests/generic/098.out b/tests/generic/098.out
> index 37415ee..f87f046 100644
> --- a/tests/generic/098.out
> +++ b/tests/generic/098.out
> @@ -9,3 +9,13 @@ File content after remount:
>  0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  *
>  0500000
> +wrote 131072/131072 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 32768/32768 bytes at offset 262144
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +File content after remount:
> +0000000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> +*
> +0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +*
> +0500000
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fstests: generic/098 update test for truncating a file into the middle of a hole

[Liu Bo]

On Tue, Nov 15, 2016 at 02:53:12PM +0800, Eryu Guan wrote:
> On Fri, Nov 11, 2016 at 02:30:04PM -0800, Liu Bo wrote:
> > This updates generic/098 by adding a sync option, i.e. 'sync' after the second
> > write, and with btrfs's NO_HOLES, we could still get wrong isize after remount.
> > 
> > This gets fixed by the patch
> > 
> > 'Btrfs: fix truncate down when no_holes feature is enabled'
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> 
> Looks good to me, just some nitpicks inline :)
> 
> > ---
> >  tests/generic/098     | 57 ++++++++++++++++++++++++++++++++-------------------
> >  tests/generic/098.out | 10 +++++++++
> >  2 files changed, 46 insertions(+), 21 deletions(-)
> > 
> > diff --git a/tests/generic/098 b/tests/generic/098
> > index 838bb5d..3b89939 100755
> > --- a/tests/generic/098
> > +++ b/tests/generic/098
> > @@ -64,27 +64,42 @@ rm -f $seqres.full
> >  _scratch_mkfs >>$seqres.full 2>&1
> >  _scratch_mount
> >  
> > -# Create our test file with some data and durably persist it.
> > -$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
> > -sync
> > -
> > -# Append some data to the file, increasing its size, and leave a hole between
> > -# the old size and the start offset if the following write. So our file gets
> > -# a hole in the range [128Kb, 256Kb[.
> > -$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
> > -
> > -# Now truncate our file to a smaller size that is in the middle of the hole we
> > -# previously created. On most truncate implementations the data we appended
> > -# before gets discarded from memory (with truncate_setsize()) and never ends
> > -# up being written to disk.
> > -$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
> > -
> > -_scratch_cycle_mount
> > -
> > -# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
> > -# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
> > -echo "File content after remount:"
> > -od -t x1 $SCRATCH_MNT/foo
> > +workout()
> > +{
> > +	NEED_SYNC=$1
> 
> Use "local" to declare this var, and in lower case. Usually we use upper
> case for global variables.

OK.

> 
> > +
> > +	# Create our test file with some data and durably persist it.
> > +	$XFS_IO_PROG -t -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
> > +	sync
> > +
> > +	# Append some data to the file, increasing its size, and leave a hole between
> > +	# the old size and the start offset if the following write. So our file gets
> > +	# a hole in the range [128Kb, 256Kb[.
> > +	$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
> > +
> > +	if [ $NEED_SYNC -eq 1 ]; then
> > +		sync
> > +	fi
> 
> Good to see some comments to explain why we need this to test
> with/without sync case.

Sure, will fix in v2.

Thanks,

-liubo
> 
> Thanks,
> Eryu
> 
> > +
> > +	# Now truncate our file to a smaller size that is in the middle of the hole we
> > +	# previously created.
> > +	# If we don't flush dirty page cache above, on most truncate
> > +	# implementations the data we appended before gets discarded from
> > +	# memory (with truncate_setsize()) and never ends up being written to
> > +	# disk.
> > +	$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
> > +
> > +	_scratch_cycle_mount
> > +
> > +	# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
> > +	# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
> > +	echo "File content after remount:"
> > +	od -t x1 $SCRATCH_MNT/foo
> > +}
> > +
> > +workout 0
> > +# flush after each write
> > +workout 1
> >  
> >  status=0
> >  exit
> > diff --git a/tests/generic/098.out b/tests/generic/098.out
> > index 37415ee..f87f046 100644
> > --- a/tests/generic/098.out
> > +++ b/tests/generic/098.out
> > @@ -9,3 +9,13 @@ File content after remount:
> >  0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  *
> >  0500000
> > +wrote 131072/131072 bytes at offset 0
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +wrote 32768/32768 bytes at offset 262144
> > +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > +File content after remount:
> > +0000000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
> > +*
> > +0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +*
> > +0500000
> > -- 
> > 2.5.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] fstests: generic/098 update test for truncating a file into the middle of a hole

[Liu Bo]

This updates generic/098 by adding a sync option, i.e. 'sync' after the second
write, and with btrfs's NO_HOLES, we could still get wrong isize after remount.

This gets fixed by the patch

'Btrfs: fix truncate down when no_holes feature is enabled'

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
v2: use 'local' for local variable and add comments for 'sync' option.

 tests/generic/098     | 60 +++++++++++++++++++++++++++++++++------------------
 tests/generic/098.out | 10 +++++++++
 2 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/tests/generic/098 b/tests/generic/098
index 838bb5d..8ab0ad4 100755
--- a/tests/generic/098
+++ b/tests/generic/098
@@ -64,27 +64,45 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 _scratch_mount
 
-# Create our test file with some data and durably persist it.
-$XFS_IO_PROG -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
-sync
-
-# Append some data to the file, increasing its size, and leave a hole between
-# the old size and the start offset if the following write. So our file gets
-# a hole in the range [128Kb, 256Kb[.
-$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
-
-# Now truncate our file to a smaller size that is in the middle of the hole we
-# previously created. On most truncate implementations the data we appended
-# before gets discarded from memory (with truncate_setsize()) and never ends
-# up being written to disk.
-$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
-
-_scratch_cycle_mount
-
-# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
-# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
-echo "File content after remount:"
-od -t x1 $SCRATCH_MNT/foo
+workout()
+{
+	local need_sync=$1
+
+	# Create our test file with some data and durably persist it.
+	$XFS_IO_PROG -t -f -c "pwrite -S 0xaa 0 128K" $SCRATCH_MNT/foo | _filter_xfs_io
+	sync
+
+	# Append some data to the file, increasing its size, and leave a hole between
+	# the old size and the start offset if the following write. So our file gets
+	# a hole in the range [128Kb, 256Kb[.
+	$XFS_IO_PROG -c "pwrite -S 0xbb 256K 32K" $SCRATCH_MNT/foo | _filter_xfs_io
+
+	# This 'sync' is to flush file extent on disk and update on-disk inode size.
+	# This is required to trigger a bug in btrfs truncate where it updates on-disk
+	# inode size incorrectly.
+	if [ $need_sync -eq 1 ]; then
+		sync
+	fi
+
+	# Now truncate our file to a smaller size that is in the middle of the hole we
+	# previously created.
+	# If we don't flush dirty page cache above, on most truncate
+	# implementations the data we appended before gets discarded from
+	# memory (with truncate_setsize()) and never ends up being written to
+	# disk.
+	$XFS_IO_PROG -c "truncate 160K" $SCRATCH_MNT/foo
+
+	_scratch_cycle_mount
+
+	# We expect to see a file with a size of 160Kb, with the first 128Kb of data all
+	# having the value 0xaa and the remaining 32Kb of data all having the value 0x00
+	echo "File content after remount:"
+	od -t x1 $SCRATCH_MNT/foo
+}
+
+workout 0
+# flush after each write
+workout 1
 
 status=0
 exit
diff --git a/tests/generic/098.out b/tests/generic/098.out
index 37415ee..f87f046 100644
--- a/tests/generic/098.out
+++ b/tests/generic/098.out
@@ -9,3 +9,13 @@ File content after remount:
 0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 *
 0500000
+wrote 131072/131072 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 32768/32768 bytes at offset 262144
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+File content after remount:
+0000000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa
+*
+0400000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+*
+0500000
-- 
2.5.0