Messages in this thread | | | Date | Tue, 10 Jan 2023 10:01:56 -0600 | Subject | Re: [PATCH v3 2/2] x86/resctrl: Avoid redundant counter read in __mon_event_count() | From | "Moger, Babu" <> |
| |
On 12/20/22 10:41, Peter Newman wrote: > __mon_event_count() does the per-RMID, per-domain work for > user-initiated event count reads and the initialization of new monitor > groups. > > In the initialization case, after resctrl_arch_reset_rmid() calls > __rmid_read() to record an initial count for a new monitor group, it > immediately calls resctrl_arch_rmid_read(). This re-read of the hardware > counter is unnecessary and the following computations are ignored by the > caller during initialization. > > Following return from resctrl_arch_reset_rmid(), just clear the > mbm_state and return. This involves moving the mbm_state lookup into the > rr->first case, as it's not needed for regular event count reads: the > QOS_L3_OCCUP_EVENT_ID case was redundant with the accumulating logic at > the end of the function. > > Signed-off-by: Peter Newman <peternewman@google.com> > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> > --- > v3: > - changelog clarifications suggested by Reinette > v2: > (patch introduced) > > v2: https://lore.kernel.org/lkml/20221214160856.2164207-2-peternewman@google.com/ > --- > arch/x86/kernel/cpu/resctrl/monitor.c | 43 ++++++++++++--------------- > 1 file changed, 19 insertions(+), 24 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index 77538abeb72a..e708df478077 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -366,41 +366,36 @@ void free_rmid(u32 rmid) > list_add_tail(&entry->list, &rmid_free_lru); > } > > +static struct mbm_state *get_mbm_state(struct rdt_domain *d, u32 rmid, > + enum resctrl_event_id evtid) > +{ > + switch (evtid) { > + case QOS_L3_MBM_TOTAL_EVENT_ID: > + return &d->mbm_total[rmid]; > + case QOS_L3_MBM_LOCAL_EVENT_ID: > + return &d->mbm_local[rmid]; > + default: > + return NULL; > + } > +} > + > static int __mon_event_count(u32 rmid, struct rmid_read *rr) > { > struct mbm_state *m; > u64 tval = 0; > > - if (rr->first) > + if (rr->first) { > resctrl_arch_reset_rmid(rr->r, rr->d, rmid, rr->evtid); > + m = get_mbm_state(rr->d, rmid, rr->evtid); > + if (m) > + memset(m, 0, sizeof(struct mbm_state)); > + return 0; > + } > > rr->err = resctrl_arch_rmid_read(rr->r, rr->d, rmid, rr->evtid, &tval); > if (rr->err) > return rr->err; > > - switch (rr->evtid) { > - case QOS_L3_OCCUP_EVENT_ID: > - rr->val += tval; > - return 0; > - case QOS_L3_MBM_TOTAL_EVENT_ID: > - m = &rr->d->mbm_total[rmid]; > - break; > - case QOS_L3_MBM_LOCAL_EVENT_ID: > - m = &rr->d->mbm_local[rmid]; > - break; > - default: > - /* > - * Code would never reach here because an invalid > - * event id would fail in resctrl_arch_rmid_read(). > - */ > - return -EINVAL; > - } > - > - if (rr->first) { > - memset(m, 0, sizeof(struct mbm_state)); > - return 0; > - } > - > rr->val += tval; > > return 0;
Tested the patches on AMD systems. Looks good. Tested-by: Babu Moger <babu.moger@amd.com>
Thanks Babu Moger
| |