Re: [PATCH v1 22/31] x86/resctrl: Make resctrl_arch_pseudo_lock_fn() take a plr

Reinette Chatre
2024-04-11T17:40:43Z

Hi Dave,

On 4/11/2024 7:38 AM, Dave Martin wrote:
> On Mon, Apr 08, 2024 at 08:24:35PM -0700, Reinette Chatre wrote:
>> Hi James,
>>
>> On 3/21/2024 9:50 AM, James Morse wrote:
>>> resctrl_arch_pseudo_lock_fn() has architecture specific behaviour,
>>> and takes a struct rdtgroup as an argument.
>>>
>>> After the filesystem code moves to /fs/, the definition of struct
>>> rdtgroup will not be available to the architecture code.
>>>
>>> The only reason resctrl_arch_pseudo_lock_fn() wants the rdtgroup is
>>> for the CLOSID. Embed that in the pseudo_lock_region as a hw_closid,
>>
>> Above creates expectation that the new member will be named hw_closid,
>> but that is not what the code does.
>
> I'll flag this for review, but I'd guess that this can probably just be
> "closid". I'll make a note to consider what needs to change to make
> things consistent between the patch and commit message.
>
> James might have had other ideas, connected with the remapping done for
> CDP emulation causing the resctrl closid being different from the actual
> value used by the hardware, at least for MPAM (see my response on
> patch 24). I don't fully understand how this works for x86 though.
>
> So long as functionality is unaffected, and this patch is introducing no
> new confusion that wasn't there beforehand, the exact name may not
> matter too much(?)

closid sounds good. It may be a good match for what is expected to be in
general/fs code.

>
> Did you have other concerns here?

No.

Reinette