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

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

Hi Babu,

On 2/14/25 10:31 AM, Moger, Babu wrote:
> On 2/14/2025 12:26 AM, Reinette Chatre wrote:
>> On 2/13/25 9:37 AM, Dave Martin wrote:
>>> On Wed, Feb 12, 2025 at 03:33:31PM -0800, Reinette Chatre wrote:
>>>> On 2/12/25 9:46 AM, Dave Martin wrote:
>>>>> On Wed, Jan 22, 2025 at 02:20:08PM -0600, Babu Moger wrote:

(quoting relevant parts with goal to focus discussion on new possible syntax)

>>>> I see the support for MPAM events distinct from the support of assignable counters.
>>>> Once the MPAM events are sorted, I think that they can be assigned with existing interface.
>>>> Please help me understand if you see it differently.
>>>>     
>>>> Doing so would need to come up with alphabetical letters for these events,
>>>> which seems to be needed for your proposal also? If we use possible flags of:
>>>>
>>>> mbm_local_read_bytes a
>>>> mbm_local_write_bytes b
>>>>
>>>> Then mbm_assign_control can be used as:
>>>> # echo '//0=ab;1=b' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control
>>>> # cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_read_bytes
>>>> <value>
>>>> # cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_bytes
>>>> <sum of mbm_local_read_bytes and mbm_local_write_bytes>
>>>>
>>>> One issue would be when resctrl needs to support more than 26 events (no more flags available),
>>>> assuming that upper case would be used for "shared" counters (unless this interface is defined
>>>> differently and only few uppercase letters used for it). Would this be too low of a limit?

As mentioned above, one possible issue with existing interface is that
it is limited to 26 events (assuming only lower case letters are used). The limit
is low enough to be of concern.

....

>>>
>>> Alternatively, if we want to be able to expand beyond single letters,
>>> could we reserve one or more characters for extension purposes?
>>>
>>> If braces are forbidden by the syntax today, could we add support for
>>> something like the following later on, without breaking anything?
>>>
>>> # echo '//0={foo}{bar};1={bar}' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control
>>>
>>

Dave proposed a change in syntax that can (a) support unlimited events,
(b) be more intuitive than the one letter flags that may be hard to match
to the events they correspond to.

>> Thank you for the suggestion. I think we may need something like this.
>> Babu, what do you think?
> 
> I'm not quite clear on this. Do we know what 'foo' and 'bar' refer to?
> It is a random text?

Not random text. It refers to the events.

I do not know if braces is what will be settled on but a slight change in
example to make it match your series can be:

# echo '//0={mbm_total_bytes}{mbm_local_bytes};1={mbm_local_bytes}' >/sys/fs/resctrl/info/L3_MON/mbm_assign_control

With syntax like above there is no concern that we will run out of
flags and the events assigned are clear without needing to parse separate flags.
For a system with a lot of events and domains this will become quite a lot
to parse though.

> 
> In his example from
> https://lore.kernel.org/lkml/Z643WdXYARTADSBy@e133380.arm.com/
> --------------------------------------------------------------
> The numbers are not supposed to have an hardware significance.
> 
>     '//0=6'
> 
> just "means assign some unused counter for domain 0, and create files
> in resctrl so I can configure and read it".

Thanks for pointing this out. I missed that the idea was that the
configuration files are dynamically created.

> 
> The "6" is really just a tag for labelling the resulting resctrl
> file names so that the user can tell them apart.  It's not supposed
> to imply any specific hardware counter or event.

Right.

> ------------------------------------------------------------------
> 
> It seems that 'foo' and 'bar' are tags used to create files in /sys/fs/resctrl/info/L3_MON/.
> 
> Given that, it looks like we're discussing entirely different things.

I am still trying to understand how MPAM counters can be supported.

Reinette