Re: [PATCH 21/24] xfs: factor out a xfs_growfs_check_rtgeom helper

Darrick J. Wong
2024-08-26T18:27:35Z

On Mon, Aug 26, 2024 at 12:06:58PM +1000, Dave Chinner wrote:
> On Thu, Aug 22, 2024 at 05:20:07PM -0700, Darrick J. Wong wrote:
> > From: Christoph Hellwig <hch@lst.de>
> >
> > Split the check that the rtsummary fits into the log into a separate
> > helper, and use xfs_growfs_rt_alloc_fake_mount to calculate the new RT
> > geometry.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > [djwong: avoid division for the 0-rtx growfs check]
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > fs/xfs/xfs_rtalloc.c | 43 +++++++++++++++++++++++++++++--------------
> > 1 file changed, 29 insertions(+), 14 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 61231b1dc4b79..78a3879ad6193 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> > @@ -1023,6 +1023,31 @@ xfs_growfs_rtg(
> > return error;
> > }
> >
> > +static int
> > +xfs_growfs_check_rtgeom(
> > + const struct xfs_mount *mp,
> > + xfs_rfsblock_t rblocks,
> > + xfs_extlen_t rextsize)
> > +{
> > + struct xfs_mount *nmp;
> > + int error = 0;
> > +
> > + nmp = xfs_growfs_rt_alloc_fake_mount(mp, rblocks, rextsize);
> > + if (!nmp)
> > + return -ENOMEM;
> > +
> > + /*
> > + * New summary size can't be more than half the size of the log. This
> > + * prevents us from getting a log overflow, since we'll log basically
> > + * the whole summary file at once.
> > + */
> > + if (nmp->m_rsumblocks > (mp->m_sb.sb_logblocks >> 1))
> > + error = -EINVAL;
>
> FWIW, the new size needs to be smaller than that, because the "half
> the log size" must to include all the log metadata needed to
> encapsulate that object. The grwofs transaction also logs inodes and
> the superblock, so that also takes away from the maximum size of
> the summary file....

<shrug> It's the same logic as what's there now, and there haven't been
any bug reports, have there? Though I suppose that's just a reduction
of what? One block for the rtbitmap, and (conservatively) two inodes
and a superblock?

n = nmp->m_rsumblocks + 1 + howmany(inodesize * 2, blocksize) + 1;
if (n > (logblocks / 2))
return -EINVAL;

--D

> -Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
>