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

[Reinette Chatre <reinette.chatre@intel.com>]

Hi James,

On 8/24/2023 9:51 AM, James Morse wrote:
> 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.
> |	 */
> 

Sounds good. Thank you.

Reinette