| Date | Tue, 16 Apr 2024 17:17:06 +0100 | From | Dave Martin <> | Subject | Re: [PATCH v1 05/31] x86/resctrl: Remove rdtgroup from update_cpu_closid_rmid() |
| |
On Mon, Apr 15, 2024 at 03:40:46PM -0500, Moger, Babu wrote: > Hi James/Dave, > > On 3/21/24 11:50, James Morse wrote: > > update_cpu_closid_rmid() takes a struct rdtgroup as an argument, which > > it uses to update the local CPUs default pqr values. This is a problem > > once the resctrl parts move out to /fs/, as the arch code cannot > > poke around inside struct rdtgroup. > > > > Rename update_cpu_closid_rmid() as resctrl_arch_sync_cpus_defaults() > > to be used as the target of an IPI, and pass the effective CLOSID > > and RMID in a new struct. > > > > Signed-off-by: James Morse <james.morse@arm.com> > > --- > > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 19 +++++++++++++++---- > > include/linux/resctrl.h | 11 +++++++++++ > > 2 files changed, 26 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > index 5d2c1ce5b6b1..18f097fce51e 100644 > > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > @@ -341,13 +341,13 @@ static int rdtgroup_cpus_show(struct kernfs_open_file *of, > > * from update_closid_rmid() is protected against __switch_to() because > > * preemption is disabled. > > */ > > -static void update_cpu_closid_rmid(void *info) > > +void resctrl_arch_sync_cpu_defaults(void *info) > > How about keeping the name similar to the old name? > > resctrl_arch_update_cpu_defaults
Ack (Reinette made a similar comment.)
> > > { > > - struct rdtgroup *r = info; > > + struct resctrl_cpu_sync *r = info; > > > > if (r) { > > this_cpu_write(pqr_state.default_closid, r->closid); > > - this_cpu_write(pqr_state.default_rmid, r->mon.rmid); > > + this_cpu_write(pqr_state.default_rmid, r->rmid); > > } > > > > /*
[...]
> > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > > index 6e87bc95f5ea..2b79e4159507 100644 > > --- a/include/linux/resctrl.h > > +++ b/include/linux/resctrl.h > > @@ -220,6 +220,17 @@ struct resctrl_schema { > > u32 num_closid; > > }; > > > > +struct resctrl_cpu_sync { > > How about changing it to resctrl_cpu_defaults? > > > + u32 closid; > > + u32 rmid; > > +};
[...]
> -- > Thanks > Babu Moger >
Yes, your name describes what the struct means, so renaming it as per your suggestion does make sense.
I'll make a note.
Cheers ---Dave
|