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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>
>
> --D
>
> > Thanks,
> > Zorro
> >
> > > if [ $free_rtx -eq 0 ]; then
> > > echo "Expected fragmented free rt space, found none."
> > > fi
> > >
> >
>
