Re: [PATCH v8 08/25] x86/resctrl: Introduce interface to display number of monitoring counters

[Reinette Chatre]

Hi Babu,

On 10/14/24 10:46 AM, Moger, Babu wrote:
> On 10/14/24 11:25, Reinette Chatre wrote:
>> On 10/10/24 8:12 AM, Moger, Babu wrote:
>>>
>>> All good points. How about this text:
>>>
>>> "num_mbm_cntrs":
>>> The number of monitoring counters available for assignment when the
>>> architecture supports mbm_cntr_assign mode.
>>>
>>> Resctrl subsystem provides the interface to count maximum of two memory
>>
>> subsystem -> filesystem
> 
> Sure.
>>
>>> bandwidth events per group, from a combination of available total and
>>
>> Is this "from a combination of ..." snippet intended to hint at BMEC?
> 
> No. We support 2 MBM events right now. That is why I added combination of
> total and local. I can remove that text.
> 
>>
>>> local events. Keeping the current interface, users can enable a maximum of
>>
>> What is meant by "Keeping the current interface"? Which interface? What will
>> "current" mean when a user reads this documentation?
> 
> I meant not to change any interface to support mbm_cntrl_assign feature.
> 
>>
>>> 2 counters per group. User will also have the option to enable only one
>>
>> "User will also have" is talking about the future. When will this be the case?
> 
> Again.. will have change the text here.
> 
>>
>>> counter to the group to maximize the number of groups monitored.
>>>
>>>
>>
>> I think that we need to be careful when making this documentation so specific
>> to the ABMC implementation. We already know that "soft-ABMC" is coming and
>> Peter already shared [1] that with software assignment it will not be possible
>> to assign counters to individual events. 
>>
>> The goal of this work is to create a generic interface and this is the documentation
>> for it. If this documentation is created to be specific to the first implementation
>> it will make it difficult to use this same interface to support other
>> implementations.
>>
> 
> Agree.
> 
> How about this?
> 
> 
> "num_mbm_cntrs":
> The number of monitoring counters available for assignment when the
> architecture supports mbm_cntr_assign mode.
> 
> The resctrl filesystem allows user track up to two memory bandwidth events
> per group, using a mix of total and local events. Users can enable up to 2

"a mix of" remains unclear to me since there are only two options. I think we
can be specific here.

> counters per group. There's also an option to enable just one counter per
> group, which allows monitoring more groups.
> 

How about below for the second paragraph:

	The resctrl filesystem supports tracking up to two memory bandwidth
	events per monitoring group: mbm_total_bytes and/or mbm_local_bytes.
	Up to two counters can be assigned per monitoring group, one for each
	memory bandwidth event. More monitoring groups can be tracked by
	assigning one counter per monitoring group. However, doing so limits
	memory bandwidth tracking to a single memory bandwidth event per
	monitoring group.
 
Reinette