[PATCH 0/3] xfs: hint based zone allocation improvements

[PATCH 0/3] xfs: hint based zone allocation improvements

[Hans Holmberg]

This series makes the zone allocation policy a bit easier to
understand and adjusts the policy for unset and "none" rw hints,
avoiding mixing these with files with file data with set values.

The first patch adds an enum for the number of hints available,
the second introduces allocation matrix without changing the policy,
and rhe third adjusts the allocation policy.

Hans Holmberg (3):
  fs: add an enum for number of life time hints
  xfs: refactor hint based zone allocation
  xfs: adjust the hint based zone allocation policy

 fs/xfs/xfs_zone_alloc.c | 116 +++++++++++++++++++---------------------
 include/linux/rw_hint.h |   1 +
 2 files changed, 57 insertions(+), 60 deletions(-)

-- 
2.34.1

[PATCH 1/3] fs: add an enum for number of life time hints

[Hans Holmberg]

Add WRITE_LIFE_HINT_NR into the rw_hint enum to define the number of
values write life time hints can be set to. This is useful for e.g.
file systems which may want to map these values to allocation groups.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 include/linux/rw_hint.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/rw_hint.h b/include/linux/rw_hint.h
index 309ca72f2dfb..adcc43042c90 100644
--- a/include/linux/rw_hint.h
+++ b/include/linux/rw_hint.h
@@ -14,6 +14,7 @@ enum rw_hint {
 	WRITE_LIFE_MEDIUM	= RWH_WRITE_LIFE_MEDIUM,
 	WRITE_LIFE_LONG		= RWH_WRITE_LIFE_LONG,
 	WRITE_LIFE_EXTREME	= RWH_WRITE_LIFE_EXTREME,
+	WRITE_LIFE_HINT_NR,
 } __packed;
 
 /* Sparse ignores __packed annotations on enums, hence the #ifndef below. */
-- 
2.34.1

[PATCH 2/3] xfs: refactor hint based zone allocation

[Hans Holmberg]

Replace the co-location code with a matrix that makes it more clear
on how the decisions are made.

The matrix contains scores for zone/file hint combinations. A "GOOD"
score for an open zone will result in immediate co-location while "OK"
combinations will only be picked if we cannot open a new zone.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 fs/xfs/xfs_zone_alloc.c | 122 ++++++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 60 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index f28214c28ab5..ff24769b8870 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -493,64 +493,64 @@ xfs_try_open_zone(
 	return oz;
 }
 
+enum xfs_zone_alloc_score {
+	/* Any open zone will do it, we're desperate */
+	XFS_ZONE_ALLOC_ANY	= 0,
+
+	/* It better fit somehow */
+	XFS_ZONE_ALLOC_OK	= 1,
+
+	/* Only reuse a zone if it fits really well. */
+	XFS_ZONE_ALLOC_GOOD	= 2,
+};
+
 /*
- * For data with short or medium lifetime, try to colocated it into an
- * already open zone with a matching temperature.
+ * Life time hint co-location matrix.  Fields not set default to 0
+ * aka XFS_ZONE_ALLOC_ANY.
  */
-static bool
-xfs_colocate_eagerly(
-	enum rw_hint		file_hint)
-{
-	switch (file_hint) {
-	case WRITE_LIFE_MEDIUM:
-	case WRITE_LIFE_SHORT:
-	case WRITE_LIFE_NONE:
-		return true;
-	default:
-		return false;
-	}
-}
-
-static bool
-xfs_good_hint_match(
-	struct xfs_open_zone	*oz,
-	enum rw_hint		file_hint)
-{
-	switch (oz->oz_write_hint) {
-	case WRITE_LIFE_LONG:
-	case WRITE_LIFE_EXTREME:
-		/* colocate long and extreme */
-		if (file_hint == WRITE_LIFE_LONG ||
-		    file_hint == WRITE_LIFE_EXTREME)
-			return true;
-		break;
-	case WRITE_LIFE_MEDIUM:
-		/* colocate medium with medium */
-		if (file_hint == WRITE_LIFE_MEDIUM)
-			return true;
-		break;
-	case WRITE_LIFE_SHORT:
-	case WRITE_LIFE_NONE:
-	case WRITE_LIFE_NOT_SET:
-		/* colocate short and none */
-		if (file_hint <= WRITE_LIFE_SHORT)
-			return true;
-		break;
-	}
-	return false;
-}
+static const unsigned int
+xfs_zoned_hint_score[WRITE_LIFE_HINT_NR][WRITE_LIFE_HINT_NR] = {
+	[WRITE_LIFE_NOT_SET]	= {
+		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_OK,
+		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_OK,
+		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_OK,
+	},
+	[WRITE_LIFE_NONE]	= {
+		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_OK,
+		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_GOOD,
+		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_GOOD,
+	},
+	[WRITE_LIFE_SHORT]	= {
+		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_GOOD,
+		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_GOOD,
+		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_GOOD,
+	},
+	[WRITE_LIFE_MEDIUM]	= {
+		[WRITE_LIFE_MEDIUM]	= XFS_ZONE_ALLOC_GOOD,
+	},
+	[WRITE_LIFE_LONG]	= {
+		[WRITE_LIFE_LONG]	= XFS_ZONE_ALLOC_OK,
+		[WRITE_LIFE_EXTREME]	= XFS_ZONE_ALLOC_OK,
+	},
+	[WRITE_LIFE_EXTREME]	= {
+		[WRITE_LIFE_LONG]	= XFS_ZONE_ALLOC_OK,
+		[WRITE_LIFE_EXTREME]	= XFS_ZONE_ALLOC_OK,
+	},
+};
 
 static bool
 xfs_try_use_zone(
 	struct xfs_zone_info	*zi,
 	enum rw_hint		file_hint,
 	struct xfs_open_zone	*oz,
-	bool			lowspace)
+	unsigned int		goodness)
 {
 	if (oz->oz_allocated == rtg_blocks(oz->oz_rtg))
 		return false;
-	if (!lowspace && !xfs_good_hint_match(oz, file_hint))
+
+	if (xfs_zoned_hint_score[oz->oz_write_hint][file_hint] < goodness)
 		return false;
+
 	if (!atomic_inc_not_zero(&oz->oz_ref))
 		return false;
 
@@ -581,14 +581,14 @@ static struct xfs_open_zone *
 xfs_select_open_zone_lru(
 	struct xfs_zone_info	*zi,
 	enum rw_hint		file_hint,
-	bool			lowspace)
+	unsigned int		goodness)
 {
 	struct xfs_open_zone	*oz;
 
 	lockdep_assert_held(&zi->zi_open_zones_lock);
 
 	list_for_each_entry(oz, &zi->zi_open_zones, oz_entry)
-		if (xfs_try_use_zone(zi, file_hint, oz, lowspace))
+		if (xfs_try_use_zone(zi, file_hint, oz, goodness))
 			return oz;
 
 	cond_resched_lock(&zi->zi_open_zones_lock);
@@ -651,9 +651,11 @@ xfs_select_zone_nowait(
 	 * data.
 	 */
 	spin_lock(&zi->zi_open_zones_lock);
-	if (xfs_colocate_eagerly(write_hint))
-		oz = xfs_select_open_zone_lru(zi, write_hint, false);
-	else if (pack_tight)
+	oz = xfs_select_open_zone_lru(zi, write_hint, XFS_ZONE_ALLOC_GOOD);
+	if (oz)
+		goto out_unlock;
+
+	if (pack_tight)
 		oz = xfs_select_open_zone_mru(zi, write_hint);
 	if (oz)
 		goto out_unlock;
@@ -667,16 +669,16 @@ xfs_select_zone_nowait(
 		goto out_unlock;
 
 	/*
-	 * Try to colocate cold data with other cold data if we failed to open a
-	 * new zone for it.
+	 * Try to find an zone that is an ok match to colocate data with.
+	 */
+	oz = xfs_select_open_zone_lru(zi, write_hint, XFS_ZONE_ALLOC_OK);
+	if (oz)
+		goto out_unlock;
+
+	/*
+	 * Pick the least recently used zone, regardless of hint match
 	 */
-	if (write_hint != WRITE_LIFE_NOT_SET &&
-	    !xfs_colocate_eagerly(write_hint))
-		oz = xfs_select_open_zone_lru(zi, write_hint, false);
-	if (!oz)
-		oz = xfs_select_open_zone_lru(zi, WRITE_LIFE_NOT_SET, false);
-	if (!oz)
-		oz = xfs_select_open_zone_lru(zi, WRITE_LIFE_NOT_SET, true);
+	oz = xfs_select_open_zone_lru(zi, write_hint, XFS_ZONE_ALLOC_ANY);
 out_unlock:
 	spin_unlock(&zi->zi_open_zones_lock);
 	return oz;
-- 
2.34.1

[PATCH 3/3] xfs: adjust the hint based zone allocation policy

[Hans Holmberg]

As we really can't make any general assumptions about files that don't
have any life time hint set or are set to "NONE", adjust the allocation
policy to avoid co-locating data from those files with files with a set
life time.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 fs/xfs/xfs_zone_alloc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_zone_alloc.c b/fs/xfs/xfs_zone_alloc.c
index ff24769b8870..23a027387933 100644
--- a/fs/xfs/xfs_zone_alloc.c
+++ b/fs/xfs/xfs_zone_alloc.c
@@ -512,17 +512,11 @@ static const unsigned int
 xfs_zoned_hint_score[WRITE_LIFE_HINT_NR][WRITE_LIFE_HINT_NR] = {
 	[WRITE_LIFE_NOT_SET]	= {
 		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_OK,
-		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_OK,
-		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_OK,
 	},
 	[WRITE_LIFE_NONE]	= {
-		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_OK,
-		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_GOOD,
-		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_GOOD,
+		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_OK,
 	},
 	[WRITE_LIFE_SHORT]	= {
-		[WRITE_LIFE_NOT_SET]	= XFS_ZONE_ALLOC_GOOD,
-		[WRITE_LIFE_NONE]	= XFS_ZONE_ALLOC_GOOD,
 		[WRITE_LIFE_SHORT]	= XFS_ZONE_ALLOC_GOOD,
 	},
 	[WRITE_LIFE_MEDIUM]	= {
-- 
2.34.1

Re: [PATCH 1/3] fs: add an enum for number of life time hints

[hch]

Looks good, but you probably want to add a few more folks that
created this constant and the header to the Cc list.

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

On Mon, Sep 01, 2025 at 10:52:04AM +0000, Hans Holmberg wrote:
> Add WRITE_LIFE_HINT_NR into the rw_hint enum to define the number of
> values write life time hints can be set to. This is useful for e.g.
> file systems which may want to map these values to allocation groups.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> ---
>  include/linux/rw_hint.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/rw_hint.h b/include/linux/rw_hint.h
> index 309ca72f2dfb..adcc43042c90 100644
> --- a/include/linux/rw_hint.h
> +++ b/include/linux/rw_hint.h
> @@ -14,6 +14,7 @@ enum rw_hint {
>  	WRITE_LIFE_MEDIUM	= RWH_WRITE_LIFE_MEDIUM,
>  	WRITE_LIFE_LONG		= RWH_WRITE_LIFE_LONG,
>  	WRITE_LIFE_EXTREME	= RWH_WRITE_LIFE_EXTREME,
> +	WRITE_LIFE_HINT_NR,
>  } __packed;
>  
>  /* Sparse ignores __packed annotations on enums, hence the #ifndef below. */
> -- 
> 2.34.1
---end quoted text---

Re: [PATCH 2/3] xfs: refactor hint based zone allocation

[hch]

On Mon, Sep 01, 2025 at 10:52:05AM +0000, Hans Holmberg wrote:
> Replace the co-location code with a matrix that makes it more clear
> on how the decisions are made.
> 
> The matrix contains scores for zone/file hint combinations. A "GOOD"
> score for an open zone will result in immediate co-location while "OK"
> combinations will only be picked if we cannot open a new zone.

Looks good:

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

Re: [PATCH 3/3] xfs: adjust the hint based zone allocation policy

[hch]

On Mon, Sep 01, 2025 at 10:52:05AM +0000, Hans Holmberg wrote:
> As we really can't make any general assumptions about files that don't
> have any life time hint set or are set to "NONE", adjust the allocation
> policy to avoid co-locating data from those files with files with a set
> life time.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>

Looks good:

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

Re: [PATCH 1/3] fs: add an enum for number of life time hints

[Hans Holmberg]

On 02/09/2025 07:41, hch wrote:
> Looks good, but you probably want to add a few more folks that
> created this constant and the header to the Cc list.

Yes, thanks for adding Bart and Jens.
Adding the others who where in cc when the header was created.

> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> On Mon, Sep 01, 2025 at 10:52:04AM +0000, Hans Holmberg wrote:
>> Add WRITE_LIFE_HINT_NR into the rw_hint enum to define the number of
>> values write life time hints can be set to. This is useful for e.g.
>> file systems which may want to map these values to allocation groups.
>>
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
>> ---
>>  include/linux/rw_hint.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/linux/rw_hint.h b/include/linux/rw_hint.h
>> index 309ca72f2dfb..adcc43042c90 100644
>> --- a/include/linux/rw_hint.h
>> +++ b/include/linux/rw_hint.h
>> @@ -14,6 +14,7 @@ enum rw_hint {
>>  	WRITE_LIFE_MEDIUM	= RWH_WRITE_LIFE_MEDIUM,
>>  	WRITE_LIFE_LONG		= RWH_WRITE_LIFE_LONG,
>>  	WRITE_LIFE_EXTREME	= RWH_WRITE_LIFE_EXTREME,
>> +	WRITE_LIFE_HINT_NR,
>>  } __packed;
>>  
>>  /* Sparse ignores __packed annotations on enums, hence the #ifndef below. */
>> -- 
>> 2.34.1
> ---end quoted text---
> 

Re: [PATCH 1/3] fs: add an enum for number of life time hints

[Bart Van Assche]

On 9/1/25 10:41 PM, hch wrote:
> Looks good, but you probably want to add a few more folks that
> created this constant and the header to the Cc list.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> On Mon, Sep 01, 2025 at 10:52:04AM +0000, Hans Holmberg wrote:
>> Add WRITE_LIFE_HINT_NR into the rw_hint enum to define the number of
>> values write life time hints can be set to. This is useful for e.g.
>> file systems which may want to map these values to allocation groups.
>>
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
>> ---
>>   include/linux/rw_hint.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/linux/rw_hint.h b/include/linux/rw_hint.h
>> index 309ca72f2dfb..adcc43042c90 100644
>> --- a/include/linux/rw_hint.h
>> +++ b/include/linux/rw_hint.h
>> @@ -14,6 +14,7 @@ enum rw_hint {
>>   	WRITE_LIFE_MEDIUM	= RWH_WRITE_LIFE_MEDIUM,
>>   	WRITE_LIFE_LONG		= RWH_WRITE_LIFE_LONG,
>>   	WRITE_LIFE_EXTREME	= RWH_WRITE_LIFE_EXTREME,
>> +	WRITE_LIFE_HINT_NR,
>>   } __packed;
>>   
>>   /* Sparse ignores __packed annotations on enums, hence the #ifndef below. */
>> -- 
>> 2.34.1
> ---end quoted text---

Thanks Christoph for having Cc-ed me. I'm not a big fan of this type of
change because it makes it harder to write switch-statements without
'default:' clause. From a quick look I haven't found any such
switch-statements on 'enum rw_hint' so I'm fine with this change.

Bart.