* [PATCHSET 0/2] fstests: random fixes @ 2022-04-11 22:54 Darrick J. Wong 2022-04-11 22:54 ` [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data Darrick J. Wong 2022-04-11 22:54 ` [PATCH 2/2] xfs/507: add test to auto group Darrick J. Wong 0 siblings, 2 replies; 7+ messages in thread From: Darrick J. Wong @ 2022-04-11 22:54 UTC (permalink / raw) To: djwong, guaneryu, zlang; +Cc: linux-xfs, fstests, guan Hi all, Here's the usual batch of odd fixes for fstests. 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=random-fixes xfsprogs git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes fstests git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes --- tests/xfs/187 | 2 +- tests/xfs/507 | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data 2022-04-11 22:54 [PATCHSET 0/2] fstests: random fixes Darrick J. Wong @ 2022-04-11 22:54 ` Darrick J. Wong 2022-04-12 8:47 ` Zorro Lang 2022-04-11 22:54 ` [PATCH 2/2] xfs/507: add test to auto group Darrick J. Wong 1 sibling, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2022-04-11 22:54 UTC (permalink / raw) To: djwong, guaneryu, zlang; +Cc: linux-xfs, fstests, guan From: Darrick J. Wong <djwong@kernel.org> Currently, this test relies on the XFS_IOC_FSCOUNTS ioctl to return accurate free space information. It doesn't. Convert it to use statfs, which uses the accurate versions of the percpu counters. Obviously, this only becomes a problem when we convert the free rtx count to use (sloppier) percpu counters instead of the (more precise and previously buggy) ondisk superblock counts. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- tests/xfs/187 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/xfs/187 b/tests/xfs/187 index 1929e566..a9dfb30a 100755 --- a/tests/xfs/187 +++ b/tests/xfs/187 @@ -135,7 +135,7 @@ punch_off=$((bigfile_sz - frag_sz)) $here/src/punch-alternating $SCRATCH_MNT/bigfile -o $((punch_off / fsbsize)) -i $((rtextsize_blks * 2)) -s $rtextsize_blks # Make sure we have some free rtextents. -free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep counts.freertx | awk '{print $3}') +free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep statfs.f_bavail | awk '{print $3}') if [ $free_rtx -eq 0 ]; then echo "Expected fragmented free rt space, found none." fi ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data 2022-04-11 22:54 ` [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data Darrick J. Wong @ 2022-04-12 8:47 ` Zorro Lang 2022-04-12 17:11 ` Darrick J. Wong 0 siblings, 1 reply; 7+ messages in thread From: Zorro Lang @ 2022-04-12 8:47 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, fstests On Mon, Apr 11, 2022 at 03:54:22PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Currently, this test relies on the XFS_IOC_FSCOUNTS ioctl to return > accurate free space information. It doesn't. Convert it to use statfs, > which uses the accurate versions of the percpu counters. Obviously, > this only becomes a problem when we convert the free rtx count to use > (sloppier) percpu counters instead of the (more precise and previously > buggy) ondisk superblock counts. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > tests/xfs/187 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/tests/xfs/187 b/tests/xfs/187 > index 1929e566..a9dfb30a 100755 > --- a/tests/xfs/187 > +++ b/tests/xfs/187 > @@ -135,7 +135,7 @@ punch_off=$((bigfile_sz - frag_sz)) > $here/src/punch-alternating $SCRATCH_MNT/bigfile -o $((punch_off / fsbsize)) -i $((rtextsize_blks * 2)) -s $rtextsize_blks > > # Make sure we have some free rtextents. > -free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep counts.freertx | awk '{print $3}') > +free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep statfs.f_bavail | awk '{print $3}') Do you mean the "cnt->freertx = mp->m_sb.sb_frextents" in xfs_fs_counts() isn't right? Thanks, Zorro > if [ $free_rtx -eq 0 ]; then > echo "Expected fragmented free rt space, found none." > fi > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data 2022-04-12 8:47 ` Zorro Lang @ 2022-04-12 17:11 ` Darrick J. Wong 2022-04-13 18:23 ` Zorro Lang 0 siblings, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2022-04-12 17:11 UTC (permalink / raw) To: linux-xfs, fstests On Tue, Apr 12, 2022 at 04:47:16PM +0800, Zorro Lang wrote: > On Mon, Apr 11, 2022 at 03:54:22PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Currently, this test relies on the XFS_IOC_FSCOUNTS ioctl to return > > accurate free space information. It doesn't. Convert it to use statfs, > > which uses the accurate versions of the percpu counters. Obviously, > > this only becomes a problem when we convert the free rtx count to use > > (sloppier) percpu counters instead of the (more precise and previously > > buggy) ondisk superblock counts. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > tests/xfs/187 | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/xfs/187 b/tests/xfs/187 > > index 1929e566..a9dfb30a 100755 > > --- a/tests/xfs/187 > > +++ b/tests/xfs/187 > > @@ -135,7 +135,7 @@ punch_off=$((bigfile_sz - frag_sz)) > > $here/src/punch-alternating $SCRATCH_MNT/bigfile -o $((punch_off / fsbsize)) -i $((rtextsize_blks * 2)) -s $rtextsize_blks > > > > # Make sure we have some free rtextents. > > -free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep counts.freertx | awk '{print $3}') > > +free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep statfs.f_bavail | awk '{print $3}') > > Do you mean the "cnt->freertx = mp->m_sb.sb_frextents" in xfs_fs_counts() isn't > right? Correct -- prior to the patches introduced here: https://lore.kernel.org/linux-xfs/164961485474.70555.18228016043917319266.stgit@magnolia/T/#t The kernel would account actual ondisk rt extent usage *and* in-memory transaction reservations in mp->m_sb.sb_frextents, which meant that one thread calling xfs_log_sb racing with another thread allocating space on the rt volume would write the wrong sb_frextents value to disk, which corrupts the superblock counters. The fix for that is to separate the two uses into separate counters -- now mp->m_sb.sb_frextents tracks the ondisk usage, and mp->m_frextents also includes tx reservations. m_frextents is a percpu counter, which means that we won't be able to rely on it for a precise accounting after the series is merged. Hence the switch to statfs, which does use the slow-but-accurate percpu_counter_sum method. --D > Thanks, > Zorro > > > if [ $free_rtx -eq 0 ]; then > > echo "Expected fragmented free rt space, found none." > > fi > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data 2022-04-12 17:11 ` Darrick J. Wong @ 2022-04-13 18:23 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2022-04-13 18:23 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, fstests On Tue, Apr 12, 2022 at 10:11:56AM -0700, Darrick J. Wong wrote: > On Tue, Apr 12, 2022 at 04:47:16PM +0800, Zorro Lang wrote: > > On Mon, Apr 11, 2022 at 03:54:22PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Currently, this test relies on the XFS_IOC_FSCOUNTS ioctl to return > > > accurate free space information. It doesn't. Convert it to use statfs, > > > which uses the accurate versions of the percpu counters. Obviously, > > > this only becomes a problem when we convert the free rtx count to use > > > (sloppier) percpu counters instead of the (more precise and previously > > > buggy) ondisk superblock counts. > > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > tests/xfs/187 | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > diff --git a/tests/xfs/187 b/tests/xfs/187 > > > index 1929e566..a9dfb30a 100755 > > > --- a/tests/xfs/187 > > > +++ b/tests/xfs/187 > > > @@ -135,7 +135,7 @@ punch_off=$((bigfile_sz - frag_sz)) > > > $here/src/punch-alternating $SCRATCH_MNT/bigfile -o $((punch_off / fsbsize)) -i $((rtextsize_blks * 2)) -s $rtextsize_blks > > > > > > # Make sure we have some free rtextents. > > > -free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep counts.freertx | awk '{print $3}') > > > +free_rtx=$($XFS_IO_PROG -c 'statfs' $SCRATCH_MNT | grep statfs.f_bavail | awk '{print $3}') > > > > Do you mean the "cnt->freertx = mp->m_sb.sb_frextents" in xfs_fs_counts() isn't > > right? > > Correct -- prior to the patches introduced here: > https://lore.kernel.org/linux-xfs/164961485474.70555.18228016043917319266.stgit@magnolia/T/#t > > The kernel would account actual ondisk rt extent usage *and* in-memory > transaction reservations in mp->m_sb.sb_frextents, which meant that one > thread calling xfs_log_sb racing with another thread allocating space on > the rt volume would write the wrong sb_frextents value to disk, which > corrupts the superblock counters. > > The fix for that is to separate the two uses into separate counters -- > now mp->m_sb.sb_frextents tracks the ondisk usage, and mp->m_frextents > also includes tx reservations. m_frextents is a percpu counter, which > means that we won't be able to rely on it for a precise accounting after > the series is merged. Hence the switch to statfs, which does use the > slow-but-accurate percpu_counter_sum method. Thanks for your detailed explanation, so it's about this patch: [PATCH 3/3] xfs: use a separate frextents counter for rt extent reservations It's good to me, it would be better if we can add more content (as above) into commit log. Reviewed-by: Zorro Lang <zlang@redhat.com> > > --D > > > Thanks, > > Zorro > > > > > if [ $free_rtx -eq 0 ]; then > > > echo "Expected fragmented free rt space, found none." > > > fi > > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] xfs/507: add test to auto group 2022-04-11 22:54 [PATCHSET 0/2] fstests: random fixes Darrick J. Wong 2022-04-11 22:54 ` [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data Darrick J. Wong @ 2022-04-11 22:54 ` Darrick J. Wong 2022-04-12 8:47 ` Zorro Lang 1 sibling, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2022-04-11 22:54 UTC (permalink / raw) To: djwong, guaneryu, zlang; +Cc: linux-xfs, fstests, guan From: Darrick J. Wong <djwong@kernel.org> Add this regression test to the auto group now that it's been quite a while since the fix patches went upstream. Signed-off-by: Darrick J. Wong <djwong@kernel.org> --- tests/xfs/507 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/xfs/507 b/tests/xfs/507 index aa3d8eeb..b9c9ab29 100755 --- a/tests/xfs/507 +++ b/tests/xfs/507 @@ -4,13 +4,17 @@ # # FS QA Test No. 507 # +# Regression test for kernel commit: +# +# 394aafdc15da ("xfs: widen inode delalloc block counter to 64-bits") +# # Try to overflow i_delayed_blks by setting the largest cowextsize hint # possible, creating a sparse file with a single byte every cowextsize bytes, # reflinking it, and retouching every written byte to see if we can create # enough speculative COW reservations to overflow i_delayed_blks. # . ./common/preamble -_begin_fstest clone +_begin_fstest auto clone _register_cleanup "_cleanup" BUS ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] xfs/507: add test to auto group 2022-04-11 22:54 ` [PATCH 2/2] xfs/507: add test to auto group Darrick J. Wong @ 2022-04-12 8:47 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2022-04-12 8:47 UTC (permalink / raw) To: Darrick J. Wong; +Cc: guaneryu, linux-xfs, fstests, guan On Mon, Apr 11, 2022 at 03:54:28PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Add this regression test to the auto group now that it's been quite a > while since the fix patches went upstream. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- Reviewed-by: Zorro Lang <zlang@redhat.com> > tests/xfs/507 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > > diff --git a/tests/xfs/507 b/tests/xfs/507 > index aa3d8eeb..b9c9ab29 100755 > --- a/tests/xfs/507 > +++ b/tests/xfs/507 > @@ -4,13 +4,17 @@ > # > # FS QA Test No. 507 > # > +# Regression test for kernel commit: > +# > +# 394aafdc15da ("xfs: widen inode delalloc block counter to 64-bits") > +# > # Try to overflow i_delayed_blks by setting the largest cowextsize hint > # possible, creating a sparse file with a single byte every cowextsize bytes, > # reflinking it, and retouching every written byte to see if we can create > # enough speculative COW reservations to overflow i_delayed_blks. > # > . ./common/preamble > -_begin_fstest clone > +_begin_fstest auto clone > > _register_cleanup "_cleanup" BUS > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-04-13 18:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-11 22:54 [PATCHSET 0/2] fstests: random fixes Darrick J. Wong 2022-04-11 22:54 ` [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data Darrick J. Wong 2022-04-12 8:47 ` Zorro Lang 2022-04-12 17:11 ` Darrick J. Wong 2022-04-13 18:23 ` Zorro Lang 2022-04-11 22:54 ` [PATCH 2/2] xfs/507: add test to auto group Darrick J. Wong 2022-04-12 8:47 ` Zorro Lang
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).