Re: [PATCH v1 1/6] x86/resctrl: Move __resctrl_sched_in() out-of-line

[Peter Newman]

Hi Reinette,

On Mon, Apr 8, 2024 at 1:59 PM Reinette Chatre
<reinette.chatre@xxxxxxxxx> wrote:
>
> Hi Peter,
>
> On 4/8/2024 12:05 PM, Peter Newman wrote:
> > Hi Reinette,
> >
> > On Sun, Apr 7, 2024 at 12:21 PM Reinette Chatre
> > <reinette.chatre@xxxxxxxxx> wrote:
> >>
> >> I think this needs more thought. rdt_enable_key is x86 specific now and should not
> >> be in the fs code. Every architecture will have its own task switch code, with
> >> __resctrl_sched_in() belonging to x86 and thus not something to be directly called
> >> from the fs code.
> >
> > I think we will need to discuss whether __resctrl_sched_in() is really
> > arch or FS code following the changes in this series. This series
> > removes the explicit MSR access and James has already provided inline
>
> Indeed, I missed how resctrl_arch_update_cpu() introduced in
> "x86/resctrl: Abstract PQR_ASSOC from generic code" results in
> __resctrl_sched_in() being architecture agnostic.
>
> (sidenotes ...  it is not obvious to me why resctrl_arch_update_cpu()
> is not consistently used, for example, in clear_closid_rmid(),
> and it also seems a better candidate to be inline within
> arch/x86/include/asm/resctrl.h)

I must have renamed resctrl_pqr_state to resctrl_cpu_state after I
last looked over clear_closid_rmid(). Now looking at the function
again, it seems clearly portable now and should definitely use
resctrl_arch_update_cpu(). I need to look over the accesses to
resctrl_cpu_state again to reconsider whether they're
architecture-dependent.

>
> > wrappers for the mon and alloc enable keys[1], so I can only assume
> > they are architecture-independent in concept.
>
> The wrappers are intended to be architecture-independent, but the keys
> are architecture dependent. Quoting the commit you linked to:
> "This means other architectures don't have to mirror the static keys"

The static keys seem to exist mainly for the benefit of
__resctrl_sched_in(), so if it becomes architecture-agnostic, I think
it would make sense for the static keys to move into the FS code with
it. I don't see any other usage of these keys in the code that
remained under arch/x86 on James' latest series.

-Peter