Message-ID: <e295deca-647c-4547-a599-9a20cc8741cc@amd.com>
Date: Fri, 14 Feb 2025 13:51:35 -0600
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
 Dave Martin <Dave.Martin@....com>
Cc: Babu Moger <babu.moger@....com>, peternewman@...gle.com, corbet@....net,
 tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, tony.luck@...el.com, x86@...nel.org,
 hpa@...or.com, paulmck@...nel.org, akpm@...ux-foundation.org,
 thuth@...hat.com, rostedt@...dmis.org, xiongwei.song@...driver.com,
 pawan.kumar.gupta@...ux.intel.com, daniel.sneddon@...ux.intel.com,
 jpoimboe@...nel.org, perry.yuan@....com, sandipan.das@....com,
 kai.huang@...el.com, xiaoyao.li@...el.com, seanjc@...gle.com,
 xin3.li@...el.com, andrew.cooper3@...rix.com, ebiggers@...gle.com,
 mario.limonciello@....com, james.morse@....com, tan.shaopeng@...itsu.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 maciej.wieczor-retman@...el.com, eranian@...gle.com
Subject: Re: [PATCH v11 00/23] x86/resctrl : Support AMD Assignable Bandwidth
 Monitoring Counters (ABMC)

Hi Reinette,

On 2/14/2025 1:18 PM, Reinette Chatre wrote:
> 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.

Yes. Agree.

> 
> ....
> 
>>>>
>>>> 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.

Yea. Sounds good.

> 
>>> 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.

Yes. We need to change our current "flag parsing". It should not be a 
problem.

> 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


Thanks
Babu

Powered by blists - more mailing lists