Re: [PATCH v15 21/34] x86/resctrl: Refactor resctrl_arch_rmid_read()

[Reinette Chatre]

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