Hi Reinette, On 7/22/25 09:56, Reinette Chatre wrote: > 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"? Sure. > >> 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." > Sure. >> >> 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."? > Sure -- Thanks Babu Moger
