diff mbox series

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

Message ID 20241211085636.1380516-11-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/43] xfs: constify feature checks | expand

Commit Message

Christoph Hellwig Dec. 11, 2024, 8:54 a.m. UTC
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(-)

Comments

Darrick J. Wong Dec. 12, 2024, 9:38 p.m. UTC | #1
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?

Otherwise looks good to me.

--D

> +		}
>  
>  		/* Reserve AG blocks for future btree expansion. */
>  		error = xfs_fs_reserve_ag_blocks(mp);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index d92bce7bc184..73bc053fdd17 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -640,7 +640,8 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
>  }
>  
>  extern void	xfs_uuid_table_free(void);
> -extern uint64_t xfs_default_resblks(xfs_mount_t *mp);
> +uint64_t	xfs_default_resblks(struct xfs_mount *mp,
> +			enum xfs_free_counter ctr);
>  extern int	xfs_mountfs(xfs_mount_t *mp);
>  extern void	xfs_unmountfs(xfs_mount_t *);
>  
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 1960ee0aad45..f57c27940467 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -944,7 +944,7 @@ xfs_restore_resvblks(
>  			resblks = mp->m_resblks[i].save;
>  			mp->m_resblks[i].save = 0;
>  		} else
> -			resblks = xfs_default_resblks(mp);
> +			resblks = xfs_default_resblks(mp, i);
>  		xfs_reserve_blocks(mp, i, resblks);
>  	}
>  }
> -- 
> 2.45.2
> 
>
Hans Holmberg Dec. 13, 2024, 9:15 a.m. UTC | #2
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. */
Darrick J. Wong Dec. 15, 2024, 6:42 p.m. UTC | #3
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. */
>
diff mbox series

Patch

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.");
+		}
 
 		/* Reserve AG blocks for future btree expansion. */
 		error = xfs_fs_reserve_ag_blocks(mp);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index d92bce7bc184..73bc053fdd17 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -640,7 +640,8 @@  xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
 }
 
 extern void	xfs_uuid_table_free(void);
-extern uint64_t xfs_default_resblks(xfs_mount_t *mp);
+uint64_t	xfs_default_resblks(struct xfs_mount *mp,
+			enum xfs_free_counter ctr);
 extern int	xfs_mountfs(xfs_mount_t *mp);
 extern void	xfs_unmountfs(xfs_mount_t *);
 
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 1960ee0aad45..f57c27940467 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -944,7 +944,7 @@  xfs_restore_resvblks(
 			resblks = mp->m_resblks[i].save;
 			mp->m_resblks[i].save = 0;
 		} else
-			resblks = xfs_default_resblks(mp);
+			resblks = xfs_default_resblks(mp, i);
 		xfs_reserve_blocks(mp, i, resblks);
 	}
 }