Re: [PATCH v6 31/42] x86/resctrl: Remove the limit on the number of CLOSID

[James Morse]

Hi Reinette,

On 20/02/2025 04:21, Reinette Chatre wrote:
> On 2/7/25 10:18 AM, James Morse wrote:
>> From: Amit Singh Tomar <amitsinght@xxxxxxxxxxx>
>>
>> Resctrl allocates and finds free CLOSID values using the bits of a u32.
>> This restricts the number of control groups that can be created by
>> user-space.
>>
>> MPAM has an architectural limit of 2^16 CLOSID values, Intel x86 could
>> be extended beyond 32 values. There is at least one MPAM platform which
>> supports more than 32 CLOSID values.
>>
>> Replace the fixed size bitmap with calls to the bitmap API to allocate
>> an array of a sufficient size.
>>
>> ffs() returns '1' for bit 0, hence the existing code subtracts 1 from
>> the index to get the CLOSID value. find_first_bit() returns the bit
>> number which does not need adjusting.
>>
>> Signed-off-by: Amit Singh Tomar <amitsinght@xxxxxxxxxxx>
>> [ morse: fixed the off-by-one in the allocator and the wrong
>>  not-found value. Removed the limit. Rephrase the commit message. ]
>> Signed-off-by: James Morse <james.morse@xxxxxxx>

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 08fec23a38bf..de79da30d500 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c

>> @@ -152,20 +152,30 @@ int closids_supported(void)

>> +static void closid_exit(void)
>> +{
>> +	bitmap_free(closid_free_map);

> With closid_free_map being a global, could this also set
> closid_free_map to NULL?

Makes sense,

> 
>>  }
>>  
>>  static int closid_alloc(void)

>> @@ -2754,20 +2763,22 @@ static int rdt_get_tree(struct fs_context *fc)
>>  		goto out_ctx;
>>  	}
>>  
>> -	closid_init();
>> +	ret = closid_init();
>> +	if (ret)
>> +		goto out_schemata_free;
>>  
>>  	if (resctrl_arch_mon_capable())
>>  		flags |= RFTYPE_MON;
>>  
>>  	ret = rdtgroup_add_files(rdtgroup_default.kn, flags);
>>  	if (ret)
>> -		goto out_schemata_free;
>> +		goto out_closid_exit;
>>  
>>  	kernfs_activate(rdtgroup_default.kn);
>>  
>>  	ret = rdtgroup_create_info_dir(rdtgroup_default.kn);
>>  	if (ret < 0)
>> -		goto out_schemata_free;
>> +		goto out_closid_exit;
>>  
>>  	if (resctrl_arch_mon_capable()) {
>>  		ret = mongroup_create_dir(rdtgroup_default.kn,

> With closid_init() called from rdt_get_tree() during mount I expected
> closid_exit() to be called from rdt_kill_sb() during unmount ?

Ah, I'd missed that - the old version got called multiple times but it was harmless. Now
it potentially leaks the bitmap. Fixed.


Thanks!

James