A new resource group is intended to be created with sane defaults. For
a cache resource this means all cache portions the new group could possibly
allocate into. This includes unused cache portions and shareable cache
portions used by other groups and hardware.

New resource group creation does not take sparse masks into account. After
determining the bitmask reflecting the new group's possible allocations the
bitmask is forced to be contiguous even if the system supports sparse masks.
For example, a new group could by default allocate into a large portion of
cache represented by 0xff0f, but it is instead created with a mask of 0xf.

Do not force a contiguous allocation range if the system supports sparse
masks.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
Not a stable candidate because nobody has complained about this and it is
not a serious issue but instead an optimization. Discovered during code
inspection as part of review of:
https://lore.kernel.org/lkml/c5807e73e0f4068392036a867d24a8e21c200421.1761464280.git.christophe.jaillet@wanadoo.fr/

Tested on system that supports sparse masks.
---
 fs/resctrl/rdtgroup.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
index 0320360cd7a6..41ce4b377af4 100644
--- a/fs/resctrl/rdtgroup.c
+++ b/fs/resctrl/rdtgroup.c
@@ -3383,11 +3383,12 @@ static u32 cbm_ensure_valid(u32 _val, struct rdt_resource *r)
 {
 	unsigned int cbm_len = r->cache.cbm_len;
 	unsigned long first_bit, zero_bit;
-	unsigned long val = _val;
+	unsigned long val;
 
-	if (!val)
-		return 0;
+	if (!_val || r->cache.arch_has_sparse_bitmasks)
+		return _val;
 
+	val = _val;
 	first_bit = find_first_bit(&val, cbm_len);
 	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
 
-- 
2.50.1

On Mon, Nov 17, 2025 at 04:42:45PM -0800, Reinette Chatre wrote:
> A new resource group is intended to be created with sane defaults. For
> a cache resource this means all cache portions the new group could possibly
> allocate into. This includes unused cache portions and shareable cache
> portions used by other groups and hardware.
> 
> New resource group creation does not take sparse masks into account. After
> determining the bitmask reflecting the new group's possible allocations the
> bitmask is forced to be contiguous even if the system supports sparse masks.
> For example, a new group could by default allocate into a large portion of
> cache represented by 0xff0f, but it is instead created with a mask of 0xf.
> 
> Do not force a contiguous allocation range if the system supports sparse
> masks.

Note that the allocated mask on systems that require contiguous bit allocation
is not optimal. In your example where the available bits for the new group
are 0xff0f, resctrl will focus on the least significant contguous range and
set 0x000f, rather than the larger group of bits 0xff00.

Fixing this would be more complex, and I don't see a lot of value as the
user is very likley to rewrite the schemata immediatley after creating
the new group.

So for this patch as-is:

Reviewed-by: Tony Luck <tony.luck@intel.com>

> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> Not a stable candidate because nobody has complained about this and it is
> not a serious issue but instead an optimization. Discovered during code
> inspection as part of review of:
> https://lore.kernel.org/lkml/c5807e73e0f4068392036a867d24a8e21c200421.1761464280.git.christophe.jaillet@wanadoo.fr/
> 
> Tested on system that supports sparse masks.
> ---
>  fs/resctrl/rdtgroup.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 0320360cd7a6..41ce4b377af4 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -3383,11 +3383,12 @@ static u32 cbm_ensure_valid(u32 _val, struct rdt_resource *r)
>  {
>  	unsigned int cbm_len = r->cache.cbm_len;
>  	unsigned long first_bit, zero_bit;
> -	unsigned long val = _val;
> +	unsigned long val;
>  
> -	if (!val)
> -		return 0;
> +	if (!_val || r->cache.arch_has_sparse_bitmasks)
> +		return _val;
>  
> +	val = _val;
>  	first_bit = find_first_bit(&val, cbm_len);
>  	zero_bit = find_next_zero_bit(&val, cbm_len, first_bit);
>  
> -- 
> 2.50.1
>

Hi Tony,

On 11/20/25 11:35 AM, Luck, Tony wrote:
> On Mon, Nov 17, 2025 at 04:42:45PM -0800, Reinette Chatre wrote:
>> A new resource group is intended to be created with sane defaults. For
>> a cache resource this means all cache portions the new group could possibly
>> allocate into. This includes unused cache portions and shareable cache
>> portions used by other groups and hardware.
>>
>> New resource group creation does not take sparse masks into account. After
>> determining the bitmask reflecting the new group's possible allocations the
>> bitmask is forced to be contiguous even if the system supports sparse masks.
>> For example, a new group could by default allocate into a large portion of
>> cache represented by 0xff0f, but it is instead created with a mask of 0xf.
>>
>> Do not force a contiguous allocation range if the system supports sparse
>> masks.
> 
> Note that the allocated mask on systems that require contiguous bit allocation
> is not optimal. In your example where the available bits for the new group
> are 0xff0f, resctrl will focus on the least significant contguous range and
> set 0x000f, rather than the larger group of bits 0xff00.

Right. The priority, as supported by aptly named cbm_ensure_valid(), is to ensure
the mask can be supported by the hardware.

> 
> Fixing this would be more complex, and I don't see a lot of value as the
> user is very likley to rewrite the schemata immediatley after creating
> the new group.

I agree this can be optimized and that it is not a priority (would appreciate
to hear if there are different opinions on this). Users that may encounter
the "less than optimal" initial allocation are the ones using exclusive and/or
pseudo-locking groups where I expect user space controls allocations with care.

> 
> So for this patch as-is:
> 
> Reviewed-by: Tony Luck <tony.luck@intel.com>

Thank you very much.

Reinette