public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: start gc on zonegc_low_space attribute updates
@ 2026-03-20 13:02 Hans Holmberg
  2026-03-20 14:39 ` Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hans Holmberg @ 2026-03-20 13:02 UTC (permalink / raw)
  To: Carlos Maiolino
  Cc: Dave Chinner, Darrick J . Wong, Christoph Hellwig, Damien Le Moal,
	linux-xfs, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-20 13:02 [PATCH] xfs: start gc on zonegc_low_space attribute updates Hans Holmberg
@ 2026-03-20 14:39 ` Darrick J. Wong
  2026-03-23 10:01   ` Hans Holmberg
  2026-03-20 23:28 ` Damien Le Moal
  2026-03-23  6:36 ` Christoph Hellwig
  2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2026-03-20 14:39 UTC (permalink / raw)
  To: Hans Holmberg
  Cc: Carlos Maiolino, Dave Chinner, Christoph Hellwig, Damien Le Moal,
	linux-xfs

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-20 13:02 [PATCH] xfs: start gc on zonegc_low_space attribute updates Hans Holmberg
  2026-03-20 14:39 ` Darrick J. Wong
@ 2026-03-20 23:28 ` Damien Le Moal
  2026-03-23  6:36 ` Christoph Hellwig
  2 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2026-03-20 23:28 UTC (permalink / raw)
  To: Hans Holmberg, Carlos Maiolino
  Cc: Dave Chinner, Darrick J . Wong, Christoph Hellwig, linux-xfs

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-20 13:02 [PATCH] xfs: start gc on zonegc_low_space attribute updates Hans Holmberg
  2026-03-20 14:39 ` Darrick J. Wong
  2026-03-20 23:28 ` Damien Le Moal
@ 2026-03-23  6:36 ` Christoph Hellwig
  2026-03-23 10:32   ` Hans Holmberg
  2 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2026-03-23  6:36 UTC (permalink / raw)
  To: Hans Holmberg
  Cc: Carlos Maiolino, Dave Chinner, Darrick J . Wong,
	Christoph Hellwig, Damien Le Moal, linux-xfs

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-20 14:39 ` Darrick J. Wong
@ 2026-03-23 10:01   ` Hans Holmberg
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Holmberg @ 2026-03-23 10:01 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Carlos Maiolino, Dave Chinner, Christoph Hellwig, Damien Le Moal,
	linux-xfs@vger.kernel.org

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-23  6:36 ` Christoph Hellwig
@ 2026-03-23 10:32   ` Hans Holmberg
  2026-03-25  6:04     ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: Hans Holmberg @ 2026-03-23 10:32 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Carlos Maiolino, Dave Chinner, Darrick J . Wong, Damien Le Moal,
	linux-xfs@vger.kernel.org

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? 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xfs: start gc on zonegc_low_space attribute updates
  2026-03-23 10:32   ` Hans Holmberg
@ 2026-03-25  6:04     ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-03-25  6:04 UTC (permalink / raw)
  To: Hans Holmberg
  Cc: Christoph Hellwig, Carlos Maiolino, Dave Chinner,
	Darrick J . Wong, Damien Le Moal, linux-xfs@vger.kernel.org

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-03-25  6:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 13:02 [PATCH] xfs: start gc on zonegc_low_space attribute updates Hans Holmberg
2026-03-20 14:39 ` Darrick J. Wong
2026-03-23 10:01   ` Hans Holmberg
2026-03-20 23:28 ` Damien Le Moal
2026-03-23  6:36 ` Christoph Hellwig
2026-03-23 10:32   ` Hans Holmberg
2026-03-25  6:04     ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox