Messages in this thread | | | From | Peter Newman <> | Date | Mon, 8 Apr 2024 14:41:22 -0700 | Subject | Re: [PATCH v1 1/6] x86/resctrl: Move __resctrl_sched_in() out-of-line |
| |
Hi Reinette,
On Mon, Apr 8, 2024 at 1:59 PM Reinette Chatre <reinette.chatre@intel.com> 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@intel.com> 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
| |