linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] xfs: a couple of realtime growfs fixes
@ 2020-10-08  3:56 Darrick J. Wong
  2020-10-08  3:56 ` [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08  3:56 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, chandanrlinux, sandeen

Hi all,

While I was taking a closer look at Chandan's earlier fixes for the
realtime growfs code, I realized that fstests doesn't actually have a
test case for growing a realtime volume.  I wrote one for testing rmap
expansions on the data device and kludged it to work for realtime, and
watched the kernel trip over a ton of assertions and fail xfs_scrub.

The two patches in this series fix the two problems that I found.  The
first is that inode inactivation will truncate the new bitmap blocks if
we don't set the VFS inode size; and the second is that we don't update
the secondary superblocks with the new rt geometry.

I'll cough up a test case after this patch.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=rt-growfs-fixes-5.10
---
 fs/xfs/xfs_rtalloc.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume
  2020-10-08  3:56 [PATCH 0/2] xfs: a couple of realtime growfs fixes Darrick J. Wong
@ 2020-10-08  3:56 ` Darrick J. Wong
  2020-10-08  6:56   ` Chandan Babu R
  2020-10-08  3:56 ` [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks Darrick J. Wong
  2020-10-08  3:59 ` [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume Darrick J. Wong
  2 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08  3:56 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, chandanrlinux, sandeen

From: Darrick J. Wong <darrick.wong@oracle.com>

The realtime bitmap and summary files are regular files that are hidden
away from the directory tree.  Since they're regular files, inode
inactivation will try to purge what it thinks are speculative
preallocations beyond the incore size of the file.  Unfortunately,
xfs_growfs_rt forgets to update the incore size when it resizes the
inodes, with the result that inactivating the rt inodes at unmount time
will cause their contents to be truncated.

Fix this by updating the incore size when we change the ondisk size as
part of updating the superblock.  Note that we don't do this when we're
allocating blocks to the rt inodes because we actually want those blocks
to get purged if the growfs fails.

This fixes corruption complaints from the online rtsummary checker when
running xfs/233.  Since that test requires rmap, one can also trigger
this by growing an rt volume, cycling the mount, and creating rt files.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_rtalloc.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 9d4e33d70d2a..1c3969807fb9 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1027,10 +1027,13 @@ xfs_growfs_rt(
 		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
 		xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
 		/*
-		 * Update the bitmap inode's size.
+		 * Update the bitmap inode's size ondisk and incore.  We need
+		 * to update the incore size so that inode inactivation won't
+		 * punch what it thinks are "posteof" blocks.
 		 */
 		mp->m_rbmip->i_d.di_size =
 			nsbp->sb_rbmblocks * nsbp->sb_blocksize;
+		i_size_write(VFS_I(mp->m_rbmip), mp->m_rbmip->i_d.di_size);
 		xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
 		/*
 		 * Get the summary inode into the transaction.
@@ -1038,9 +1041,12 @@ xfs_growfs_rt(
 		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
 		xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
 		/*
-		 * Update the summary inode's size.
+		 * Update the summary inode's size.  We need to update the
+		 * incore size so that inode inactivation won't punch what it
+		 * thinks are "posteof" blocks.
 		 */
 		mp->m_rsumip->i_d.di_size = nmp->m_rsumsize;
+		i_size_write(VFS_I(mp->m_rsumip), mp->m_rsumip->i_d.di_size);
 		xfs_trans_log_inode(tp, mp->m_rsumip, XFS_ILOG_CORE);
 		/*
 		 * Copy summary data from old to new sizes.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks
  2020-10-08  3:56 [PATCH 0/2] xfs: a couple of realtime growfs fixes Darrick J. Wong
  2020-10-08  3:56 ` [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume Darrick J. Wong
@ 2020-10-08  3:56 ` Darrick J. Wong
  2020-10-08  7:31   ` Chandan Babu R
  2020-10-08  3:59 ` [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume Darrick J. Wong
  2 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08  3:56 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs, chandanrlinux, sandeen

From: Darrick J. Wong <darrick.wong@oracle.com>

When we call growfs on the data device, we update the secondary
superblocks to reflect the updated filesystem geometry.  We need to do
this for growfs on the realtime volume too, because a future xfs_repair
run could try to fix the filesystem using a backup superblock.

This was observed by the online superblock scrubbers while running
xfs/233.  One can also trigger this by growing an rt volume, cycling the
mount, and creating new rt files.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_rtalloc.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 1c3969807fb9..5b2e68d9face 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -18,7 +18,7 @@
 #include "xfs_trans_space.h"
 #include "xfs_icache.h"
 #include "xfs_rtalloc.h"
-
+#include "xfs_sb.h"
 
 /*
  * Read and return the summary information for a given extent size,
@@ -1108,6 +1108,11 @@ xfs_growfs_rt(
 	 */
 	kmem_free(nmp);
 
+	/* Update secondary superblocks now the physical grow has completed */
+	error = xfs_update_secondary_sbs(mp);
+	if (error)
+		return error;
+
 	/*
 	 * If we had to allocate a new rsum_cache, we either need to free the
 	 * old one (if we succeeded) or free the new one and restore the old one


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume
  2020-10-08  3:56 [PATCH 0/2] xfs: a couple of realtime growfs fixes Darrick J. Wong
  2020-10-08  3:56 ` [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume Darrick J. Wong
  2020-10-08  3:56 ` [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks Darrick J. Wong
@ 2020-10-08  3:59 ` Darrick J. Wong
  2020-10-08  8:12   ` Chandan Babu R
  2 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08  3:59 UTC (permalink / raw)
  To: linux-xfs, chandanrlinux, sandeen; +Cc: fstests

From: Darrick J. Wong <darrick.wong@oracle.com>

Make sure that we can run growfs to expand the realtime volume without
it blowing up.  This is a regression test for the following patches:

xfs: Set xfs_buf type flag when growing summary/bitmap files
xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
xfs: fix realtime bitmap/summary file truncation when growing rt volume
xfs: make xfs_growfs_rt update secondary superblocks

Because the xfs maintainer realized that no, we have no tests for this
particular piece of functionality.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/916     |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/916.out |   10 +++++++
 tests/xfs/group   |    1 +
 3 files changed, 92 insertions(+)
 create mode 100755 tests/xfs/916
 create mode 100644 tests/xfs/916.out

diff --git a/tests/xfs/916 b/tests/xfs/916
new file mode 100755
index 00000000..c2484376
--- /dev/null
+++ b/tests/xfs/916
@@ -0,0 +1,81 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2020, Oracle and/or its affiliates.  All Rights Reserved.
+#
+# FS QA Test No. 916
+#
+# Tests xfs_growfs on the realtime volume to make sure none of it blows up.
+# This is a regression test for the following patches:
+#
+# xfs: Set xfs_buf type flag when growing summary/bitmap files
+# xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
+# xfs: fix realtime bitmap/summary file truncation when growing rt volume
+# xfs: make xfs_growfs_rt update secondary superblocks
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	_scratch_unmount >> $seqres.full 2>&1
+	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
+	rm -f $tmp.* $TEST_DIR/$seq.rtvol
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+# Note that we don't _require_realtime because we synthesize a rt volume
+# below.
+_require_scratch_nocheck
+_require_no_large_scratch_dev
+
+echo "Create fake rt volume"
+truncate -s 400m $TEST_DIR/$seq.rtvol
+rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
+
+echo "Format and mount 100m rt volume"
+export USE_EXTERNAL=yes
+export SCRATCH_RTDEV=$rtdev
+_scratch_mkfs -r size=100m > $seqres.full
+_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
+
+testdir=$SCRATCH_MNT/test-$seq
+mkdir $testdir
+
+echo "Check rt volume stats"
+$XFS_IO_PROG -c 'chattr +t' $testdir
+$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
+before=$(stat -f -c '%b' $testdir)
+
+echo "Create some files"
+_pwrite_byte 0x61 0 1m $testdir/original >> $seqres.full
+
+echo "Grow fs"
+$XFS_GROWFS_PROG $SCRATCH_MNT 2>&1 |  _filter_growfs >> $seqres.full
+_scratch_cycle_mount
+
+echo "Recheck 400m rt volume stats"
+$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
+after=$(stat -f -c '%b' $testdir)
+_within_tolerance "rt volume size" $after $((before * 4)) 5% -v
+
+echo "Create more copies to make sure the bitmap really works"
+cp -p $testdir/original $testdir/copy3
+
+echo "Check filesystem"
+_check_xfs_filesystem $SCRATCH_DEV none $rtdev
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/916.out b/tests/xfs/916.out
new file mode 100644
index 00000000..55f2356a
--- /dev/null
+++ b/tests/xfs/916.out
@@ -0,0 +1,10 @@
+QA output created by 916
+Create fake rt volume
+Format and mount 100m rt volume
+Check rt volume stats
+Create some files
+Grow fs
+Recheck 400m rt volume stats
+rt volume size is in range
+Create more copies to make sure the bitmap really works
+Check filesystem
diff --git a/tests/xfs/group b/tests/xfs/group
index ef375397..4e58b5cc 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -549,6 +549,7 @@
 910 auto quick inobtcount
 911 auto quick bigtime
 915 auto quick quota
+916 auto quick realtime growfs
 1202 auto quick swapext
 1208 auto quick swapext
 1500 dangerous_fuzzers dangerous_bothrepair

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume
  2020-10-08  3:56 ` [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume Darrick J. Wong
@ 2020-10-08  6:56   ` Chandan Babu R
  0 siblings, 0 replies; 9+ messages in thread
From: Chandan Babu R @ 2020-10-08  6:56 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, sandeen

On Thursday 8 October 2020 9:26:06 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The realtime bitmap and summary files are regular files that are hidden
> away from the directory tree.  Since they're regular files, inode
> inactivation will try to purge what it thinks are speculative
> preallocations beyond the incore size of the file.  Unfortunately,
> xfs_growfs_rt forgets to update the incore size when it resizes the
> inodes, with the result that inactivating the rt inodes at unmount time
> will cause their contents to be truncated.
> 
> Fix this by updating the incore size when we change the ondisk size as
> part of updating the superblock.  Note that we don't do this when we're
> allocating blocks to the rt inodes because we actually want those blocks
> to get purged if the growfs fails.
> 
> This fixes corruption complaints from the online rtsummary checker when
> running xfs/233.  Since that test requires rmap, one can also trigger
> this by growing an rt volume, cycling the mount, and creating rt files.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_rtalloc.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 9d4e33d70d2a..1c3969807fb9 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1027,10 +1027,13 @@ xfs_growfs_rt(
>  		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL);
>  		xfs_trans_ijoin(tp, mp->m_rbmip, XFS_ILOCK_EXCL);
>  		/*
> -		 * Update the bitmap inode's size.
> +		 * Update the bitmap inode's size ondisk and incore.  We need
> +		 * to update the incore size so that inode inactivation won't
> +		 * punch what it thinks are "posteof" blocks.
>  		 */
>  		mp->m_rbmip->i_d.di_size =
>  			nsbp->sb_rbmblocks * nsbp->sb_blocksize;
> +		i_size_write(VFS_I(mp->m_rbmip), mp->m_rbmip->i_d.di_size);
>  		xfs_trans_log_inode(tp, mp->m_rbmip, XFS_ILOG_CORE);
>  		/*
>  		 * Get the summary inode into the transaction.
> @@ -1038,9 +1041,12 @@ xfs_growfs_rt(
>  		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL);
>  		xfs_trans_ijoin(tp, mp->m_rsumip, XFS_ILOCK_EXCL);
>  		/*
> -		 * Update the summary inode's size.
> +		 * Update the summary inode's size.  We need to update the
> +		 * incore size so that inode inactivation won't punch what it
> +		 * thinks are "posteof" blocks.
>  		 */
>  		mp->m_rsumip->i_d.di_size = nmp->m_rsumsize;
> +		i_size_write(VFS_I(mp->m_rsumip), mp->m_rsumip->i_d.di_size);
>  		xfs_trans_log_inode(tp, mp->m_rsumip, XFS_ILOG_CORE);
>  		/*
>  		 * Copy summary data from old to new sizes.
> 
> 


-- 
chandan




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks
  2020-10-08  3:56 ` [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks Darrick J. Wong
@ 2020-10-08  7:31   ` Chandan Babu R
  2020-10-08 14:47     ` Darrick J. Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Chandan Babu R @ 2020-10-08  7:31 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, sandeen

On Thursday 8 October 2020 9:26:12 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we call growfs on the data device, we update the secondary
> superblocks to reflect the updated filesystem geometry.  We need to do
> this for growfs on the realtime volume too, because a future xfs_repair
> run could try to fix the filesystem using a backup superblock.
> 
> This was observed by the online superblock scrubbers while running
> xfs/233.  One can also trigger this by growing an rt volume, cycling the
> mount, and creating new rt files.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/xfs_rtalloc.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 1c3969807fb9..5b2e68d9face 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -18,7 +18,7 @@
>  #include "xfs_trans_space.h"
>  #include "xfs_icache.h"
>  #include "xfs_rtalloc.h"
> -
> +#include "xfs_sb.h"
>  
>  /*
>   * Read and return the summary information for a given extent size,
> @@ -1108,6 +1108,11 @@ xfs_growfs_rt(
>  	 */
>  	kmem_free(nmp);
>  
> +	/* Update secondary superblocks now the physical grow has completed */
> +	error = xfs_update_secondary_sbs(mp);
> +	if (error)
> +		return error;
> +

If any of the operations in the previous "for" loop causes "error" to be set
and the loop to be exited, the call to xfs_update_secondary_sbs() would
overwrite this error value. In the worst case it might set error to 0 and
hence return a success status to the caller when the growfs operation
had actually failed.

-- 
chandan




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume
  2020-10-08  3:59 ` [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume Darrick J. Wong
@ 2020-10-08  8:12   ` Chandan Babu R
  2020-10-08 15:02     ` Darrick J. Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Chandan Babu R @ 2020-10-08  8:12 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, sandeen, fstests, guaneryu

On Thursday 8 October 2020 9:29:57 AM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure that we can run growfs to expand the realtime volume without
> it blowing up.  This is a regression test for the following patches:
> 
> xfs: Set xfs_buf type flag when growing summary/bitmap files
> xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> xfs: fix realtime bitmap/summary file truncation when growing rt volume
> xfs: make xfs_growfs_rt update secondary superblocks
> 
> Because the xfs maintainer realized that no, we have no tests for this
> particular piece of functionality.

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

Eryu,
We could ignore https://www.spinics.net/lists/linux-xfs/msg44948.html since
the current patch is a superset of the test scenarios verified there.

> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/916     |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/916.out |   10 +++++++
>  tests/xfs/group   |    1 +
>  3 files changed, 92 insertions(+)
>  create mode 100755 tests/xfs/916
>  create mode 100644 tests/xfs/916.out
> 
> diff --git a/tests/xfs/916 b/tests/xfs/916
> new file mode 100755
> index 00000000..c2484376
> --- /dev/null
> +++ b/tests/xfs/916
> @@ -0,0 +1,81 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2020, Oracle and/or its affiliates.  All Rights Reserved.
> +#
> +# FS QA Test No. 916
> +#
> +# Tests xfs_growfs on the realtime volume to make sure none of it blows up.
> +# This is a regression test for the following patches:
> +#
> +# xfs: Set xfs_buf type flag when growing summary/bitmap files
> +# xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> +# xfs: fix realtime bitmap/summary file truncation when growing rt volume
> +# xfs: make xfs_growfs_rt update secondary superblocks
> +#
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1    # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	_scratch_unmount >> $seqres.full 2>&1
> +	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
> +	rm -f $tmp.* $TEST_DIR/$seq.rtvol
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs
> +# Note that we don't _require_realtime because we synthesize a rt volume
> +# below.
> +_require_scratch_nocheck
> +_require_no_large_scratch_dev
> +
> +echo "Create fake rt volume"
> +truncate -s 400m $TEST_DIR/$seq.rtvol
> +rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
> +
> +echo "Format and mount 100m rt volume"
> +export USE_EXTERNAL=yes
> +export SCRATCH_RTDEV=$rtdev
> +_scratch_mkfs -r size=100m > $seqres.full
> +_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
> +
> +testdir=$SCRATCH_MNT/test-$seq
> +mkdir $testdir
> +
> +echo "Check rt volume stats"
> +$XFS_IO_PROG -c 'chattr +t' $testdir
> +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> +before=$(stat -f -c '%b' $testdir)
> +
> +echo "Create some files"
> +_pwrite_byte 0x61 0 1m $testdir/original >> $seqres.full
> +
> +echo "Grow fs"
> +$XFS_GROWFS_PROG $SCRATCH_MNT 2>&1 |  _filter_growfs >> $seqres.full
> +_scratch_cycle_mount
> +
> +echo "Recheck 400m rt volume stats"
> +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> +after=$(stat -f -c '%b' $testdir)
> +_within_tolerance "rt volume size" $after $((before * 4)) 5% -v
> +
> +echo "Create more copies to make sure the bitmap really works"
> +cp -p $testdir/original $testdir/copy3
> +
> +echo "Check filesystem"
> +_check_xfs_filesystem $SCRATCH_DEV none $rtdev
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/916.out b/tests/xfs/916.out
> new file mode 100644
> index 00000000..55f2356a
> --- /dev/null
> +++ b/tests/xfs/916.out
> @@ -0,0 +1,10 @@
> +QA output created by 916
> +Create fake rt volume
> +Format and mount 100m rt volume
> +Check rt volume stats
> +Create some files
> +Grow fs
> +Recheck 400m rt volume stats
> +rt volume size is in range
> +Create more copies to make sure the bitmap really works
> +Check filesystem
> diff --git a/tests/xfs/group b/tests/xfs/group
> index ef375397..4e58b5cc 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -549,6 +549,7 @@
>  910 auto quick inobtcount
>  911 auto quick bigtime
>  915 auto quick quota
> +916 auto quick realtime growfs
>  1202 auto quick swapext
>  1208 auto quick swapext
>  1500 dangerous_fuzzers dangerous_bothrepair
> 


-- 
chandan




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks
  2020-10-08  7:31   ` Chandan Babu R
@ 2020-10-08 14:47     ` Darrick J. Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08 14:47 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: linux-xfs, sandeen

On Thu, Oct 08, 2020 at 01:01:59PM +0530, Chandan Babu R wrote:
> On Thursday 8 October 2020 9:26:12 AM IST Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > When we call growfs on the data device, we update the secondary
> > superblocks to reflect the updated filesystem geometry.  We need to do
> > this for growfs on the realtime volume too, because a future xfs_repair
> > run could try to fix the filesystem using a backup superblock.
> > 
> > This was observed by the online superblock scrubbers while running
> > xfs/233.  One can also trigger this by growing an rt volume, cycling the
> > mount, and creating new rt files.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/xfs_rtalloc.c |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 1c3969807fb9..5b2e68d9face 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> > @@ -18,7 +18,7 @@
> >  #include "xfs_trans_space.h"
> >  #include "xfs_icache.h"
> >  #include "xfs_rtalloc.h"
> > -
> > +#include "xfs_sb.h"
> >  
> >  /*
> >   * Read and return the summary information for a given extent size,
> > @@ -1108,6 +1108,11 @@ xfs_growfs_rt(
> >  	 */
> >  	kmem_free(nmp);
> >  
> > +	/* Update secondary superblocks now the physical grow has completed */
> > +	error = xfs_update_secondary_sbs(mp);
> > +	if (error)
> > +		return error;
> > +
> 
> If any of the operations in the previous "for" loop causes "error" to be set
> and the loop to be exited, the call to xfs_update_secondary_sbs() would
> overwrite this error value. In the worst case it might set error to 0 and
> hence return a success status to the caller when the growfs operation
> had actually failed.

Oops, good catch!

--D

> -- 
> chandan
> 
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume
  2020-10-08  8:12   ` Chandan Babu R
@ 2020-10-08 15:02     ` Darrick J. Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2020-10-08 15:02 UTC (permalink / raw)
  To: Chandan Babu R; +Cc: linux-xfs, sandeen, fstests, guaneryu

On Thu, Oct 08, 2020 at 01:42:47PM +0530, Chandan Babu R wrote:
> On Thursday 8 October 2020 9:29:57 AM IST Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Make sure that we can run growfs to expand the realtime volume without
> > it blowing up.  This is a regression test for the following patches:
> > 
> > xfs: Set xfs_buf type flag when growing summary/bitmap files
> > xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> > xfs: fix realtime bitmap/summary file truncation when growing rt volume
> > xfs: make xfs_growfs_rt update secondary superblocks
> > 
> > Because the xfs maintainer realized that no, we have no tests for this
> > particular piece of functionality.
> 
> Looks good to me.
> 
> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
> 
> Eryu,
> We could ignore https://www.spinics.net/lists/linux-xfs/msg44948.html since
> the current patch is a superset of the test scenarios verified there.

Ofc now I discover that there's yet another bug in that we don't
annotate grabbing the rt bitmap and summary files, which causes lockdep
to barf, so I'll rev the whole series with that fix and update this test.

--D

> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/916     |   81 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/916.out |   10 +++++++
> >  tests/xfs/group   |    1 +
> >  3 files changed, 92 insertions(+)
> >  create mode 100755 tests/xfs/916
> >  create mode 100644 tests/xfs/916.out
> > 
> > diff --git a/tests/xfs/916 b/tests/xfs/916
> > new file mode 100755
> > index 00000000..c2484376
> > --- /dev/null
> > +++ b/tests/xfs/916
> > @@ -0,0 +1,81 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (c) 2020, Oracle and/or its affiliates.  All Rights Reserved.
> > +#
> > +# FS QA Test No. 916
> > +#
> > +# Tests xfs_growfs on the realtime volume to make sure none of it blows up.
> > +# This is a regression test for the following patches:
> > +#
> > +# xfs: Set xfs_buf type flag when growing summary/bitmap files
> > +# xfs: Set xfs_buf's b_ops member when zeroing bitmap/summary files
> > +# xfs: fix realtime bitmap/summary file truncation when growing rt volume
> > +# xfs: make xfs_growfs_rt update secondary superblocks
> > +#
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1    # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	_scratch_unmount >> $seqres.full 2>&1
> > +	test -e "$rtdev" && losetup -d $rtdev >> $seqres.full 2>&1
> > +	rm -f $tmp.* $TEST_DIR/$seq.rtvol
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +# Note that we don't _require_realtime because we synthesize a rt volume
> > +# below.
> > +_require_scratch_nocheck
> > +_require_no_large_scratch_dev
> > +
> > +echo "Create fake rt volume"
> > +truncate -s 400m $TEST_DIR/$seq.rtvol
> > +rtdev=$(_create_loop_device $TEST_DIR/$seq.rtvol)
> > +
> > +echo "Format and mount 100m rt volume"
> > +export USE_EXTERNAL=yes
> > +export SCRATCH_RTDEV=$rtdev
> > +_scratch_mkfs -r size=100m > $seqres.full
> > +_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
> > +
> > +testdir=$SCRATCH_MNT/test-$seq
> > +mkdir $testdir
> > +
> > +echo "Check rt volume stats"
> > +$XFS_IO_PROG -c 'chattr +t' $testdir
> > +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> > +before=$(stat -f -c '%b' $testdir)
> > +
> > +echo "Create some files"
> > +_pwrite_byte 0x61 0 1m $testdir/original >> $seqres.full
> > +
> > +echo "Grow fs"
> > +$XFS_GROWFS_PROG $SCRATCH_MNT 2>&1 |  _filter_growfs >> $seqres.full
> > +_scratch_cycle_mount
> > +
> > +echo "Recheck 400m rt volume stats"
> > +$XFS_INFO_PROG $SCRATCH_MNT >> $seqres.full
> > +after=$(stat -f -c '%b' $testdir)
> > +_within_tolerance "rt volume size" $after $((before * 4)) 5% -v
> > +
> > +echo "Create more copies to make sure the bitmap really works"
> > +cp -p $testdir/original $testdir/copy3
> > +
> > +echo "Check filesystem"
> > +_check_xfs_filesystem $SCRATCH_DEV none $rtdev
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/916.out b/tests/xfs/916.out
> > new file mode 100644
> > index 00000000..55f2356a
> > --- /dev/null
> > +++ b/tests/xfs/916.out
> > @@ -0,0 +1,10 @@
> > +QA output created by 916
> > +Create fake rt volume
> > +Format and mount 100m rt volume
> > +Check rt volume stats
> > +Create some files
> > +Grow fs
> > +Recheck 400m rt volume stats
> > +rt volume size is in range
> > +Create more copies to make sure the bitmap really works
> > +Check filesystem
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index ef375397..4e58b5cc 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -549,6 +549,7 @@
> >  910 auto quick inobtcount
> >  911 auto quick bigtime
> >  915 auto quick quota
> > +916 auto quick realtime growfs
> >  1202 auto quick swapext
> >  1208 auto quick swapext
> >  1500 dangerous_fuzzers dangerous_bothrepair
> > 
> 
> 
> -- 
> chandan
> 
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-10-08 15:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08  3:56 [PATCH 0/2] xfs: a couple of realtime growfs fixes Darrick J. Wong
2020-10-08  3:56 ` [PATCH 1/2] xfs: fix realtime bitmap/summary file truncation when growing rt volume Darrick J. Wong
2020-10-08  6:56   ` Chandan Babu R
2020-10-08  3:56 ` [PATCH 2/2] xfs: make xfs_growfs_rt update secondary superblocks Darrick J. Wong
2020-10-08  7:31   ` Chandan Babu R
2020-10-08 14:47     ` Darrick J. Wong
2020-10-08  3:59 ` [RFC PATCH 3/2] xfstest: test running growfs on the realtime volume Darrick J. Wong
2020-10-08  8:12   ` Chandan Babu R
2020-10-08 15:02     ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).