Re: [PATCH v11 00/23] x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC)

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 2/18/25 11:32 AM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 2/18/25 12:14, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 2/18/25 7:39 AM, Moger, Babu wrote:
>>  
>>> 3. Use the actual events instead of flags based on the below comment.
>>>
>>> https://lore.kernel.org/lkml/a07fca4c-c8fa-41a6-b126-59815b9a58f9@intel.com/
>>>
>>>  Something like this.
>>>  # echo '//0={mbm_total_bytes}{mbm_local_bytes};1={mbm_local_bytes}'
>>>> /sys/fs/resctrl/info/L3_MON/mbm_assign_control
>>>
>>>  Are we ready to go with this approach? I am still not clear on this.
>>>
>>>  Reinette, What do you think?
>>
>> I was actually expecting some push back or at least discussion on this interface
>> because the braces seem difficult to parse when compared to, for example, using
> 
> I am yet to work on it. Will work on it after confirmation.
> 
> Here is the output from a system with 12 domains. I created one "test" group.
> 
> Output is definitely harder to parse for human eyes.
> 
> #cat info/L3_MON/mbm_assign_control
> test//0={mbm_total_bytes}{mbm_local_bytes};1={mbm_total_bytes}{mbm_local_bytes};2={mbm_total_bytes}{mbm_local_bytes};3={mbm_total_bytes}{mbm_local_bytes};4={mbm_total_bytes}{mbm_local_bytes};5={mbm_total_bytes}{mbm_local_bytes};6={mbm_total_bytes}{mbm_local_bytes};7={mbm_total_bytes}{mbm_local_bytes};8={mbm_total_bytes}{mbm_local_bytes};9={mbm_total_bytes}{mbm_local_bytes};10={mbm_total_bytes}{mbm_local_bytes};11={mbm_total_bytes}{mbm_local_bytes}
> //0={mbm_total_bytes}{mbm_local_bytes};1={mbm_total_bytes}{mbm_local_bytes};2={mbm_total_bytes}{mbm_local_bytes};3={mbm_total_bytes}{mbm_local_bytes};4={mbm_total_bytes}{mbm_local_bytes};5={mbm_total_bytes}{mbm_local_bytes};6={mbm_total_bytes}{mbm_local_bytes};7={mbm_total_bytes}{mbm_local_bytes};8={mbm_total_bytes}{mbm_local_bytes};9={mbm_total_bytes}{mbm_local_bytes};10={mbm_total_bytes}{mbm_local_bytes};11={mbm_total_bytes}{mbm_local_bytes}
> 
> It is harder to parse in code also. We should consider only if there is a
> value-add with this format.

Please see my comments in [2] for some motivations.

> 
> Otherwise I prefer our current flag format.
> 
> # cat info/L3_MON/mbm_assign_control
> test//0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;8=tl;9=tl;10=tl;11=tl
> //0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;8=tl;9=tl;10=tl;11=tl

We could possibly consider some middle ground where flags are separated by
commas and when the amount of used flags reach 26 the interface can use
"two letter flags" or "longer names" or "the actual event name" or ....

> 
> 
>> commas to separate the events of a domain. Peter [1] has some reservations about
> 
> Yes. I would like to hear from Peter.
> 

Reinette


[2] https://lore.kernel.org/lkml/ccd9c5d7-0266-4054-879e-e084b6972ad5@intel.com/