Re: [PATCH 10/43] xfs: preserve RT reservations across remounts

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

On Fri, Dec 13, 2024 at 10:15:25AM +0100, Hans Holmberg wrote:
> On Thu, Dec 12, 2024 at 10:38 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Wed, Dec 11, 2024 at 09:54:35AM +0100, Christoph Hellwig wrote:
> > > From: Hans Holmberg <hans.holmberg@wdc.com>
> > >
> > > Introduce a reservation setting for rt devices so that zoned GC
> > > reservations are preserved over remount ro/rw cycles.
> > >
> > > Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  fs/xfs/xfs_mount.c | 22 +++++++++++++++-------
> > >  fs/xfs/xfs_mount.h |  3 ++-
> > >  fs/xfs/xfs_super.c |  2 +-
> > >  3 files changed, 18 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > > index 4174035b2ac9..db910ecc1ed4 100644
> > > --- a/fs/xfs/xfs_mount.c
> > > +++ b/fs/xfs/xfs_mount.c
> > > @@ -465,10 +465,15 @@ xfs_mount_reset_sbqflags(
> > >  }
> > >
> > >  uint64_t
> > > -xfs_default_resblks(xfs_mount_t *mp)
> > > +xfs_default_resblks(
> > > +     struct xfs_mount        *mp,
> > > +     enum xfs_free_counter   ctr)
> > >  {
> > >       uint64_t resblks;
> > >
> > > +     if (ctr == XC_FREE_RTEXTENTS)
> > > +             return 0;
> > > +
> > >       /*
> > >        * We default to 5% or 8192 fsbs of space reserved, whichever is
> > >        * smaller.  This is intended to cover concurrent allocation
> > > @@ -683,6 +688,7 @@ xfs_mountfs(
> > >       uint                    quotamount = 0;
> > >       uint                    quotaflags = 0;
> > >       int                     error = 0;
> > > +     int                     i;
> > >
> > >       xfs_sb_mount_common(mp, sbp);
> > >
> > > @@ -1051,18 +1057,20 @@ xfs_mountfs(
> > >        * privileged transactions. This is needed so that transaction
> > >        * space required for critical operations can dip into this pool
> > >        * when at ENOSPC. This is needed for operations like create with
> > > -      * attr, unwritten extent conversion at ENOSPC, etc. Data allocations
> > > -      * are not allowed to use this reserved space.
> > > +      * attr, unwritten extent conversion at ENOSPC, garbage collection
> > > +      * etc. Data allocations are not allowed to use this reserved space.
> > >        *
> > >        * This may drive us straight to ENOSPC on mount, but that implies
> > >        * we were already there on the last unmount. Warn if this occurs.
> > >        */
> > >       if (!xfs_is_readonly(mp)) {
> > > -             error = xfs_reserve_blocks(mp, XC_FREE_BLOCKS,
> > > -                             xfs_default_resblks(mp));
> > > -             if (error)
> > > -                     xfs_warn(mp,
> > > +             for (i = 0; i < XC_FREE_NR; i++) {
> > > +                     error = xfs_reserve_blocks(mp, i,
> > > +                                     xfs_default_resblks(mp, i));
> > > +                     if (error)
> > > +                             xfs_warn(mp,
> > >       "Unable to allocate reserve blocks. Continuing without reserve pool.");
> >
> > Should we be able to log *which* reserve block pool is out?
> 
> Yep, that should be useful I think. We could do something like this:

Yeah, that looks good to me.

--D

> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 20d564b3b564..6ef69d025f9a 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -674,6 +674,10 @@ xfs_rtbtree_compute_maxlevels(
>         mp->m_rtbtree_maxlevels = levels;
>  }
> 
> +static const char * const xfs_free_pool_name[XC_FREE_NR] = {
> +               "free blocks", "free rt extents", "available rt extents"
> +};
> +
>  /*
>   * This function does the following on an initial mount of a file system:
>   *     - reads the superblock from disk and init the mount struct
> @@ -1081,7 +1085,8 @@ xfs_mountfs(
>                                         xfs_default_resblks(mp, i));
>                         if (error)
>                                 xfs_warn(mp,
> -       "Unable to allocate reserve blocks. Continuing without reserve pool.");
> +"Unable to allocate reserve blocks. Continuing without reserve pool for %s.",
> +                               xfs_free_pool_name[i]);
>                 }
> 
>                 /* Reserve AG blocks for future btree expansion. */
>