Re: [PATCH v2] xfs: start gc on zonegc_low_space attribute updates

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

On Wed, Mar 25, 2026 at 01:43:12PM +0100, Hans Holmberg wrote:
> Start gc if the agressiveness of zone garbage collection is changed
> by the user (if the file system is not read only).
> 
> Without this change, the new setting will not be taken into account
> until the gc thread is woken up by e.g. a write.
> 
> Cc: <stable@vger.kernel.org> # v6.15
> Fixes: 845abeb1f06a8a ("xfs: add tunable threshold parameter for triggering zone GC")
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> ---
> 
> v2:
> - Added a new helper to wake up the gc thread in stead of unparking it,
>   which is required to make this work properly.
> - Added protection against races with unmounts as sysfs gets torn down
>   after the zone info struct is freed. This also avoids unneded
>   wakeups during remount.
> - Added fixes and stable cc tags as provided by Darrick.
> 
> v1: https://lore.kernel.org/linux-xfs/20260320130256.35225-1-hans.holmberg@wdc.com/
> 
>  fs/xfs/xfs_sysfs.c      |  7 ++++++-
>  fs/xfs/xfs_zone_alloc.h |  4 ++++
>  fs/xfs/xfs_zone_gc.c    | 17 +++++++++++++++++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 6c7909838234..4527119b2961 100644
> --- a/fs/xfs/xfs_sysfs.c
> +++ b/fs/xfs/xfs_sysfs.c
> @@ -14,6 +14,7 @@
>  #include "xfs_log_priv.h"
>  #include "xfs_mount.h"
>  #include "xfs_zones.h"
> +#include "xfs_zone_alloc.h"
>  
>  struct xfs_sysfs_attr {
>  	struct attribute attr;
> @@ -724,6 +725,7 @@ zonegc_low_space_store(
>  	const char		*buf,
>  	size_t			count)
>  {
> +	struct xfs_mount	*mp = zoned_to_mp(kobj);
>  	int			ret;
>  	unsigned int		val;
>  
> @@ -734,7 +736,10 @@ zonegc_low_space_store(
>  	if (val > 100)
>  		return -EINVAL;
>  
> -	zoned_to_mp(kobj)->m_zonegc_low_space = val;
> +	if (mp->m_zonegc_low_space != val) {
> +		mp->m_zonegc_low_space = val;
> +		xfs_zone_gc_wakeup(mp);
> +	}
>  
>  	return count;
>  }
> diff --git a/fs/xfs/xfs_zone_alloc.h b/fs/xfs/xfs_zone_alloc.h
> index 4db02816d0fd..8b2ef98c81ef 100644
> --- a/fs/xfs/xfs_zone_alloc.h
> +++ b/fs/xfs/xfs_zone_alloc.h
> @@ -51,6 +51,7 @@ int xfs_mount_zones(struct xfs_mount *mp);
>  void xfs_unmount_zones(struct xfs_mount *mp);
>  void xfs_zone_gc_start(struct xfs_mount *mp);
>  void xfs_zone_gc_stop(struct xfs_mount *mp);
> +void xfs_zone_gc_wakeup(struct xfs_mount *mp);
>  #else
>  static inline int xfs_mount_zones(struct xfs_mount *mp)
>  {
> @@ -65,6 +66,9 @@ static inline void xfs_zone_gc_start(struct xfs_mount *mp)
>  static inline void xfs_zone_gc_stop(struct xfs_mount *mp)
>  {
>  }
> +static inline void xfs_zone_gc_wakeup(struct xfs_mount *mp)
> +{
> +}
>  #endif /* CONFIG_XFS_RT */
>  
>  #endif /* _XFS_ZONE_ALLOC_H */
> diff --git a/fs/xfs/xfs_zone_gc.c b/fs/xfs/xfs_zone_gc.c
> index 0ff710fa0ee7..a8f71231f351 100644
> --- a/fs/xfs/xfs_zone_gc.c
> +++ b/fs/xfs/xfs_zone_gc.c
> @@ -1171,6 +1171,23 @@ xfs_zone_gc_stop(
>  		kthread_park(mp->m_zone_info->zi_gc_thread);
>  }
>  
> +void
> +xfs_zone_gc_wakeup(
> +	struct xfs_mount	*mp)
> +{
> +	struct super_block      *sb = mp->m_super;
> +
> +	/*
> +	 * If we are unmounting the file system we must not try to
> +	 * wake gc as m_zone_info might have been freed already.
> +	 */
> +	if (down_read_trylock(&sb->s_umount)) {

s_umount can be held exclusively for other things (e.g. freeze attempts)
which means that you might miss a wakeup here even though the filesystem
isn't unmounting.

That's probably benign here and in the other place (inode flush) that
does this, but it's worth asking -- will it matter that there's a slight
chance that someone adjusts the value and we fail to wake up the gc
thread?

--D

> +		if (!xfs_is_readonly(mp))
> +			wake_up_process(mp->m_zone_info->zi_gc_thread);
> +		up_read(&sb->s_umount);
> +	}
> +}
> +
>  int
>  xfs_zone_gc_mount(
>  	struct xfs_mount	*mp)
> -- 
> 2.34.1
> 
>