Re: [PATCH 1/2] xfs/187: don't rely on FSCOUNTS for free space data

["Darrick J. Wong" <djwong@kernel.org>]

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
> > 
>