Re: [PATCH v4 01/13] x86/resctrl: Fix min_cbm_bits for AMD

[Reinette Chatre]

Hi Babu,

On 9/7/2022 10:59 AM, Babu Moger wrote:
> AMD systems support zero CBM (capacity bit mask) for L3 allocation.

Above mentions "for L3 allocation", but the change impacts
L3 as well as L2 allocation. Perhaps just
"cache allocation"?

> That is reflected in rdt_init_res_defs_amd() by:
> 
> 	r->cache.arch_has_empty_bitmaps = true;
> 
> However given the unified code in cbm_validate(), checking for:
> 	val == 0 && !arch_has_empty_bitmaps
> 
> is not enough because of another check in cbm_validate():
> 
> 	if ((zero_bit - first_bit) < r->cache.min_cbm_bits)
> 
> The default value of r->cache.min_cbm_bits = 1.
> 
> Leading to:
> 
> 	$ cd /sys/fs/resctrl
> 	$ mkdir foo
> 	$ cd foo
> 	$ echo L3:0=0 > schemata
>           -bash: echo: write error: Invalid argument
> 	$ cat /sys/fs/resctrl/info/last_cmd_status
> 	  Need at least 1 bits in the mask
> 
> Fix the issue by initializing the min_cbm_bits to 0 for AMD. Also,
> remove the default setting of min_cbm_bits and initialize it separately.
> 
> After the fix
> 	$ cd /sys/fs/resctrl
> 	$ mkdir foo
> 	$ cd foo
> 	$ echo L3:0=0 > schemata
> 	$ cat /sys/fs/resctrl/info/last_cmd_status
> 	  ok
> 
> Link: https://lore.kernel.org/lkml/20220517001234.3137157-1-eranian@xxxxxxxxxx/
> Fixes: 316e7f901f5a ("x86/resctrl: Add struct rdt_cache::arch_has_{sparse, empty}_bitmaps")
> Signed-off-by: Stephane Eranian <eranian@xxxxxxxxxx>
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> Reviewed-by: Ingo Molnar <mingo@xxxxxxxxxx>

(apart from the changelog nitpick)

Thank you for clarifying the way forward for this fix.

Reviewed-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>

Reinette