Hi Babu, On 7/22/25 7:23 AM, Moger, Babu wrote: > Hi Reinette, > > > On 7/17/25 22:51, Reinette Chatre wrote: >> Hi Babu, >> >> On 7/8/25 3:17 PM, Babu Moger wrote: >>> resctrl_arch_rmid_read() modifies the value obtained from MSR_IA32_QM_CTR >>> to account for overflow. This adjustment is common to both >> >> The portion factored out does not just handle MBM overflow counts but also >> handles counter scaling for *all* events, including cache occupancy. > > Yes. Got it. thanks > >> >>> resctrl_arch_rmid_read() and resctrl_arch_cntr_read(). >>> >>> To prepare for the implementation of resctrl_arch_cntr_read(), refactor >>> this logic into a new function called mbm_corrected_val(). >> >> This thus cannot be made specific to MBM. More accurate may be >> get_corrected_val(). > > Sure. > > Rephrased the changelog. > > x86/resctrl: Refactor resctrl_arch_rmid_read() > > resctrl_arch_rmid_read() modifies the value obtained from MSR_IA32_QM_CTR "modifies" -> "adjusts"? > to account for the overflow for MBM events and apply counter scaling for > all the events. This logic is common to both resctrl_arch_rmid_read() and > resctrl_arch_cntr_read(). This may not be obvious since resctrl_arch_cntr_read() does not exist at this point in the series. Perhaps just make it a bit higher level, for example: "This logic is common to both reading an RMID and reading a hardware counter directly." > > To prepare for the implementation of resctrl_arch_cntr_read(), refactor > this logic into a new function called get_corrected_val(). No need for "function" when using (). Could be : "Refactor the hardware value adjustment logic into get_corrected_val() to prepare for support of reading a hardware counter."? Reinette
