Hi Tony, On Tue, Oct 24, 2023 at 8:16 PM Tony Luck <tony.luck@xxxxxxxxx> wrote: > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -418,6 +418,14 @@ static int __mon_event_count(u32 rmid, struct rmid_read *rr) > return 0; > } > > +static struct mbm_state *get_mbm_data(struct rdt_domain *dom_mbm, int rmid) > +{ > + if (is_mbm_local_enabled()) > + return &dom_mbm->mbm_local[rmid]; > + > + return &dom_mbm->mbm_total[rmid]; > +} That looks very similar to the get_mbm_state() function I added to this same file recently: https://lore.kernel.org/all/20221220164132.443083-2-peternewman%40google.com I think the name you picked is misleadingly general. "local if available, otherwise total" seems to be a choice specific to the mbps controller. I think these functions should be reconciled a little better. Other than that, looks good. Reviewed-by: Peter Newman <peternewman@xxxxxxxxxx>