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

From: Dave Martin
Date: Thu Apr 11 2024 - 10:38:33 EST


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(?)

Did you have other concerns here?

Cheers
---Dave