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

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

[Hans Holmberg]

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)) {
+		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

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

[Darrick J. Wong]

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
> 
> 

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

[Hans Holmberg]

On 25/03/2026 16:22, Darrick J. Wong wrote:
> 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?
> 

For freezes, we'll check the condition for starting gc when we've thawed,
and in general I don't think it's a big deal to miss a wakeup when writing
the attribute for rare corner cases. The new setting will take
effect as soon as a zone is filled or marked as partially used.

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

[Damien Le Moal]

On 2026/03/25 5:43, 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>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

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

[Dave Chinner]

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.

Isn't that a deadlock vector?

i.e. unmount takes s_umount, concurrently userspace writes a new
value to sysfs file. sysfs file write blocks on s_umount, unmount
blocks holding s_umount waiting for sysfs file reference count to go
to zero to destroy it?

-Dave.
-- 
Dave Chinner
dgc@kernel.org

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

[Christoph Hellwig]

On Thu, Mar 26, 2026 at 01:51:54PM +1100, Dave Chinner wrote:
> > - 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.
> 
> Isn't that a deadlock vector?
> 
> i.e. unmount takes s_umount, concurrently userspace writes a new
> value to sysfs file. sysfs file write blocks on s_umount, unmount
> blocks holding s_umount waiting for sysfs file reference count to go
> to zero to destroy it?

Given that the code does a trylock it should not.

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

[Christoph Hellwig]

On Wed, Mar 25, 2026 at 01:43:12PM +0100, Hans Holmberg wrote:
> - 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.

Independent of this patch we really should tear down sysfs early in
unmount, otherwise we'll run into issues with data structures torn
down first eventually.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>