Hi Babu, On 7/25/25 11:29 AM, Babu Moger wrote: > When supported, "mbm_event" counter assignment mode allows users to assign > a hardware counter to an RMID, event pair and monitor the bandwidth usage > as long as it is assigned. The hardware continues to track the assigned > counter until it is explicitly unassigned by the user. > > Introduce the architecture calls resctrl_arch_cntr_read() and > resctrl_arch_reset_cntr() to read and reset event counters when "mbm_event" > mode is supported. Function names are chosen to match existing (apologies if I gave you the text ... trying to polish with more focus on imperative tone now) "Function names are chosen to match" -> "Function names match"? > resctrl_arch_rmid_read() and resctrl_arch_reset_rmid(). > > Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx> > --- ... > --- > include/linux/resctrl.h | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h > index 50e38445183a..4d37827121a6 100644 > --- a/include/linux/resctrl.h > +++ b/include/linux/resctrl.h > @@ -613,6 +613,44 @@ void resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d, > enum resctrl_event_id evtid, u32 rmid, u32 closid, > u32 cntr_id, bool assign); > > +/** > + * resctrl_arch_cntr_read() - Read the event data corresponding to the counter ID > + * assigned to the RMID, event pair for this resource > + * and domain. > + * @r: Resource that the counter should be read from. > + * @d: Domain that the counter should be read from. > + * @closid: CLOSID that matches the RMID. > + * @rmid: RMID used for counter ID assignment. Can this be more specific, for example: The RMID to which @cntr_id is assigned. > + * @cntr_id: The counter ID whose event data should be read. Valid when > + * "mbm_event" mode is enabled and @eventid is MBM event. Would the counter ID not always be valid? Specifically, resctrl_arch_cntr_read() is _only_ called when "mbm_event" mode is enabled and @eventid is _always_ an MBM event, no? If you agree, the @cntr_id description can be something like below with the calling context details moved to general function description: @cntr_id: The counter to read. > + * @eventid: eventid used for counter ID assignment, such as > + * QOS_L3_MBM_TOTAL_EVENT_ID or QOS_L3_MBM_LOCAL_EVENT_ID. The "@eventid is an MBM event" can move here? For example: The MBM event to which @cntr_id is assigned. > + * @val: Result of the counter read in bytes. > + * It looks to me as though some of the @cntr_id text could move to be the function description. The description can also be expanded to include where this will be called from. For example, Called on a CPU that belongs to domain @d when "mbm_event" mode is enabled. Called from a non-migrateable process context via smp_call_on_cpu() unless all CPUs are nohz_full, in which case it is called via IPI (smp_call_function_any()). The goal is to make information specific. Please feel free to improve. > + * Return: > + * 0 on success, or -EIO, -EINVAL etc on error. > + */ > +int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_mon_domain *d, > + u32 closid, u32 rmid, int cntr_id, > + enum resctrl_event_id eventid, u64 *val); > + > +/** > + * resctrl_arch_reset_cntr() - Reset any private state associated with counter ID. > + * @r: The domain's resource. > + * @d: The counter ID's domain. > + * @closid: CLOSID that matches the RMID. > + * @rmid: RMID used for counter ID assignment. > + * @cntr_id: The counter ID whose event data should be reset. Valid when > + * "mbm_event" mode is enabled and @eventid is MBM event. > + * @eventid: eventid used for counter ID assignment, such as > + * QOS_L3_MBM_TOTAL_EVENT_ID or QOS_L3_MBM_LOCAL_EVENT_ID. Above should similarly be specific. > + * > + * This can be called from any CPU. > + */ > +void resctrl_arch_reset_cntr(struct rdt_resource *r, struct rdt_mon_domain *d, > + u32 closid, u32 rmid, int cntr_id, > + enum resctrl_event_id eventid); > + > extern unsigned int resctrl_rmid_realloc_threshold; > extern unsigned int resctrl_rmid_realloc_limit; > Reinette
