Re: [PATCH v7 04/24] x86/resctrl: Move rmid allocation out of mkdir_rdt_prepare()

[James Morse <james.morse@arm.com>]

Hi Babu,

On 09/11/2023 20:29, Moger, Babu wrote:
> On 10/25/23 13:03, James Morse wrote:
>> RMID are allocated for each monitor or control group directory, because
>> each of these needs its own RMID. For control groups,
>> rdtgroup_mkdir_ctrl_mon() later goes on to allocate the CLOSID.
>>
>> MPAM's equivalent of RMID is not an independent number, so can't be
>> allocated until the CLOSID is known. An RMID allocation for one CLOSID
>> may fail, whereas another may succeed depending on how many monitor
>> groups a control group has.
>>
>> The RMID allocation needs to move to be after the CLOSID has been
>> allocated.
>>
>> Move the RMID allocation out of mkdir_rdt_prepare() to occur in its caller,
>> after the mkdir_rdt_prepare() call. This allows the RMID allocator to
>> know the CLOSID.

>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 1eb3a3075093..8fb0f56f64be 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -3317,6 +3317,12 @@ static int mkdir_rdt_prepare_rmid_alloc(struct rdtgroup *rdtgrp)
>>  	return 0;
>>  }
>>  
>> +static void mkdir_rdt_prepare_rmid_free(struct rdtgroup *rgrp)
>> +{
>> +	if (rdt_mon_capable)
>> +		free_rmid(rgrp->mon.rmid);
>> +}
> 
> The check if (rdt_mon_capable) is extra here. Not required. Otherwise
> looks good.

It pairs with the behaviour of mkdir_rdt_prepare_rmid_alloc(), which returns early if
(!rdt_mon_capable). This avoids double-adding rmid-0 to the limbo list.


> Reviewed-by: Babu Moger <babu.moger@amd.com>

Thanks! I assume you're happy with my above description of the rdt_mon_capable check is
needed.


Thanks,

James