Re: [PATCH v9 14/26] x86/resctrl: Introduce interface to display number of free counters

[Reinette Chatre]

Hi Babu,

On 11/19/24 11:20 AM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 11/15/24 18:31, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 10/29/24 4:21 PM, Babu Moger wrote:
>>> Provide the interface to display the number of free monitoring counters
>>> available for assignment in each doamin when mbm_cntr_assign is supported.
>>>
>>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>>> ---
>>> v9: New patch.
>>> ---
>>>  Documentation/arch/x86/resctrl.rst     |  4 ++++
>>>  arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
>>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 33 ++++++++++++++++++++++++++
>>>  3 files changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>>> index 2f3a86278e84..2bc58d974934 100644
>>> --- a/Documentation/arch/x86/resctrl.rst
>>> +++ b/Documentation/arch/x86/resctrl.rst
>>> @@ -302,6 +302,10 @@ with the following files:
>>>  	memory bandwidth tracking to a single memory bandwidth event per
>>>  	monitoring group.
>>>  
>>> +"available_mbm_cntrs":
>>> +	The number of free monitoring counters available assignment in each domain
>>
>> "The number of free monitoring counters available assignment" -> "The number of monitoring
>> counters available for assignment"?
>>
>> (not taking into account how text may change after addressing Peter's feedback)
> 
> How about this?
> 
> "The number of monitoring counters available for assignment in each domain
> when the architecture supports mbm_cntr_assign mode. There are a total of
> "num_mbm_cntrs" counters are available for assignment. Counters can be
> assigned or unassigned individually in each domain. A counter is available
> for new assignment if it is unassigned in all domains."

Please consider the context of this paragraph. It follows right after the description
of "num_mbm_cntrs" that states "Up to two counters can be assigned per monitoring group".
I think it is confusing to follow that with a paragraph that states "Counters can be
assigned or unassigned individually in each domain." I wonder if it may be helpful to
use a different term ... for example a counter is *assigned* to an event of a monitoring
group but this assignment may be to specified (not yet supported) or all (this work) domains while
it is only *programmed*/*activated* to specified domains. Of course, all of this documentation
needs to remain coherent if future work decides to indeed support per-domain assignment.

Reinette