Re: [PATCH v5 02/24] x86/resctrl: Access per-rmid structures by index

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

Hi Reinette,

On 09/08/2023 23:32, Reinette Chatre wrote:
> On 7/28/2023 9:42 AM, James Morse wrote:
>> @@ -377,14 +399,17 @@ static void add_rmid_to_limbo(struct rmid_entry *entry)
>>  
>>  void free_rmid(u32 closid, u32 rmid)
>>  {
>> +	u32 idx = resctrl_arch_rmid_idx_encode(closid, rmid);
>>  	struct rmid_entry *entry;
>>  
>> -	if (!rmid)
>> -		return;
>> -
>>  	lockdep_assert_held(&rdtgroup_mutex);
>>  
>> -	entry = __rmid_entry(closid, rmid);
>> +	/* do not allow the default rmid to be free'd */
>> +	if (idx == resctrl_arch_rmid_idx_encode(RESCTRL_RESERVED_CLOSID,
>> +						RESCTRL_RESERVED_RMID))
>> +		return;
>> +

> Why is this encoding necessary? Can the provided function parameters
> not be tested directly against RESCTRL_RESERVED_CLOSID and
> RESCTRL_RESERVED_RMID ?

Doing this by encoding means if the architecture code supplies an
resctrl_arch_rmid_idx_encode() that ignores the closid, this reduces down to:
| if (rmid == RESCTRL_RESERVED_RMID)

which is what the code did before. I'll add a comment:
|	/*
|	 * Do not allow RESCTRL_RESERVED_RMID to be free'd. Comparing by index
|	 * allows architectures that ignore the closid parameter to avoid an
|	 * unnecessary check.
|	 */


Thanks,

James