[PATCH] xfs: start gc on zonegc_low_space attribute updates

[PATCH] 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.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---

 fs/xfs/xfs_sysfs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
index 6c7909838234..ac1329944049 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,11 @@ 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;
+		if (!xfs_is_readonly(mp))
+			xfs_zone_gc_start(mp);
+	}
 
 	return count;
 }
-- 
2.34.1

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

[Darrick J. Wong]

On Fri, Mar 20, 2026 at 02:02:56PM +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.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>

This sounds like a bug fix, so

Cc: <stable@vger.kernel.org> # v6.15
Fixes: 845abeb1f06a8a ("xfs: add tunable threshold parameter for triggering zone GC")

The zonegc start makes sense to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> 
>  fs/xfs/xfs_sysfs.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
> index 6c7909838234..ac1329944049 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,11 @@ 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;
> +		if (!xfs_is_readonly(mp))
> +			xfs_zone_gc_start(mp);
> +	}
>  
>  	return count;
>  }
> -- 
> 2.34.1
> 
> 

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

[Hans Holmberg]

On 20/03/2026 15:39, Darrick J. Wong wrote:
> On Fri, Mar 20, 2026 at 02:02:56PM +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.
>>
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> 
> This sounds like a bug fix, so
> 
> Cc: <stable@vger.kernel.org> # v6.15
> Fixes: 845abeb1f06a8a ("xfs: add tunable threshold parameter for triggering zone GC")
> 
> The zonegc start makes sense to me
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Yeah, this is really a bug fix, thanks for the fixes tag!

> 
> --D
> 
>> ---
>>
>>  fs/xfs/xfs_sysfs.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/xfs_sysfs.c b/fs/xfs/xfs_sysfs.c
>> index 6c7909838234..ac1329944049 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,11 @@ 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;
>> +		if (!xfs_is_readonly(mp))
>> +			xfs_zone_gc_start(mp);
>> +	}
>>  
>>  	return count;
>>  }
>> -- 
>> 2.34.1
>>
>>
> 

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

[Damien Le Moal]

On 3/20/26 22:02, 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.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>

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

And I agree with Darrick: a fixes tag and cc stable would be nice.

-- 
Damien Le Moal
Western Digital Research

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

[Christoph Hellwig]

> +		mp->m_zonegc_low_space = val;
> +		if (!xfs_is_readonly(mp))
> +			xfs_zone_gc_start(mp);

Do we need to take s_umount here to prevent races with remount?
(Not that it has any major effect, but we might as well get it
tight).

Otherwise looks good.  Do we want a test to ensure this takes
effect?  And one that it doesn't do anything stupid on a hard
read-only file system?

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

[Hans Holmberg]

On 23/03/2026 07:36, Christoph Hellwig wrote:
>> +		mp->m_zonegc_low_space = val;
>> +		if (!xfs_is_readonly(mp))
>> +			xfs_zone_gc_start(mp);
> 
> Do we need to take s_umount here to prevent races with remount?
> (Not that it has any major effect, but we might as well get it
> tight).

Yeah, I think we might as well.

> 
> Otherwise looks good.  Do we want a test to ensure this takes
> effect?  And one that it doesn't do anything stupid on a hard
> read-only file system?
> 

Yep, I can add add a test that checks that:

1) No gc is performed if the value is set 100 on a half-full
   RO scratch fs with partially used zones
2) When the file system is remounted RW, gc does its thing.

Do do this, I'll need to add a dependency to mountstats in 
xfstests though (checking gc status and rt available blocks),
but that should be fine? 

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

[Christoph Hellwig]

On Mon, Mar 23, 2026 at 10:32:43AM +0000, Hans Holmberg wrote:
> > Otherwise looks good.  Do we want a test to ensure this takes
> > effect?  And one that it doesn't do anything stupid on a hard
> > read-only file system?
> > 
> 
> Yep, I can add add a test that checks that:
> 
> 1) No gc is performed if the value is set 100 on a half-full
>    RO scratch fs with partially used zones
> 2) When the file system is remounted RW, gc does its thing.

Sounds great!

> Do do this, I'll need to add a dependency to mountstats in 
> xfstests though (checking gc status and rt available blocks),
> but that should be fine? 

Yeah, we depend on all kinds of procfs files, and mountstats have
been around from the start of the zoned allocator.