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

Hans Holmberg
2024-12-13T09:15:37Z

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:

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. */