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

From: Moger, Babu
Date: Thu Mar 13 2025 - 16:17:57 EST


Hi Reinette,

On 3/13/25 11:08, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/12/25 11:14 AM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 3/12/25 12:14, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 3/12/25 9:03 AM, Moger, Babu wrote:
>>>> Hi Reinette,
>>>>
>>>> On 3/12/25 10:07, Reinette Chatre wrote:
>>>>> Hi Babu,
>>>>>
..

>>>>>> We can add the mkdir support later. That way we can provide basic ABMC
>>>>>> support without too much code complexity with mkdir support.
>>>>>
>>>>> This is not clear to me how you envision the "first phase". Is it what you
>>>>> proposed above, for example:
>>>>> #echo "LclFill, LclNTWr, RmtFill" >
>>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes
>>>>>
>>>>> In above the counter configuration name is a file.
>>>>
>>>> Yes. That is correct.
>>>>
>>>> There will be two configuration files by default when resctrl is mounted
>>>> when ABMC is enabled.
>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes
>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes
>>>>
>>>>>
>>>>> How could mkdir support be added to this later if there are already files present?
>>>>
>>>> We already have these directories when resctrl is mounted.
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_total_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_local_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_total_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_local_bytes
>>>>
>>>> We dont need "mkdir" support for default configurations.
>>>
>>> I was referring to the "mkdir" support for additional configurations that
>>> I understood you are thinking about adding later. For example,
>>> (copied from Peter's message
>>> https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@xxxxxxxxxxxxxx/):
>>>
>>>
>>> # mkdir info/L3_MON/counter_configs/mbm_local_bytes
>>> # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>> # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>> # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>> # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>> LclFill
>>> LclNTWr
>>> LclSlowFill
>>>
>>> Any "later" work needs to be backward compatible with the first phase.
>>
>> Actually, we dont need extra file "event_filter".
>> This was discussed here.
>> https://lore.kernel.org/lkml/CALPaoChLL8p49eANYgQ0dJiFs7G=223fGae+LJyx3DwEhNeR8A@xxxxxxxxxxxxxx/
>
> I undestand from that exchange that it is possible to read/write from
> an *existing* kernfs file but it is not obvious to me how that file is
> planned to be created.

My bad.. I misspoke here. We need "event_filter" file under each
configuration.


>
> My understanding of the motivation behind support for "mkdir" is to enable
> user space to create custom counter configurations.
>

That is correct.

> I understand that ABMC support aims to start with existing mbm_total_bytes/mbm_local_bytes
> configurations but I believe the consensus is that custom configurations need
> to be supported in the future.
> If resctrl starts with support where counter configuration as
> managed with a *file*, for example:
> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes
> how will user space create future custom configurations?
> As I understand that is only possible with mkdir.
>
>>
>> # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes
>> # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes
>> # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes
>> # cat info/L3_MON/counter_configs/mbm_local_bytes
>> LclFill
>> LclNTWr
>> LclSlowFill
>>
>> In the future, we can add mkdir support.
>>
>> # mkdir info/L3_MON/counter_configs/mbm_read_only
>
> This is exactly my concern. resctrl should not start with a user space where
> a counter configuration is a file (mbm_local_bytes/mbm_total_bytes) and then
> switch user space interface to have counter configuration be done with
> directories.
>
>> # echo LclFill > info/L3_MON/counter_configs/mbm_read_only
>> # cat info/L3_MON/counter_configs/mbm_read_only
>> LclFill
>
> ... wait ... user space writes to the directory?
>

My bad. This is wrong. Let me rewrite the steps below.

>
>
>>
>> #echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive
>>
>> Which would result in the creation of test/mon_data/mon_L3_*/mbm_read_only
>>
>> So, there is not breakage of backword compatibility.
>
> The way I understand it I am seeing many incompatibilities. Perhaps I am missing
> something. Could you please provide detailed steps of how first phase and
> second phase would look?

No. You didn't miss anything. I misspoke on few steps.

Here are the steps. Just copying steps from Peters proposal.
https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@xxxxxxxxxxxxxx/


1. Mount the resctrl
mount -t resctrl resctrl /sys/fs/resctrl

2. When ABMC is supported two default configurations will be created.

a. info/L3_MON/counter_configs/mbm_total_bytes/event_filter
b. info/L3_MON/counter_configs/mbm_local_bytes/event_filter

These files will be populated with default total and local events
# cat info/L3_MON/counter_configs/mbm_total_bytes/event_filter
VictimBW
RmtSlowFill
RmtNTWr
RmtFill
LclFill
LclNTWr
LclSlowFill

# cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter
LclFill,
LclNTWr
LclSlowFill

3. Users will have options to update the event configuration.
echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter

4. As usual the events can be read from the mon_data directories.
#mkdir /sys/fs/resctrl/test
#cd /sys/fs/resctr/test
#cat test/mon_data/mon_data/mon_L3_00/mbm_tota_bytes
101010
#cat test/mon_data/mon_data/mon_L3_00/mbm_local_bytes
32323

5. There will be 3 files created in each group's mon_data directory when
ABMC is supported.

a. test/mon_data/mon_L3_00/assign_exclusive
b. test/mon_data/mon_L3_00/assign_shared
c. test/mon_data/mon_L3_00/unassign


6. Events can be assigned/unassigned by these commands

# echo mbm_total_bytes > test/mon_data/mon_L3_00/assign_exclusive
# echo mbm_local_bytes > test/mon_data/mon_L3_01/assign_exclusive
# echo mbm_local_bytes > test/mon_data/mon_L3_01/unassign


Note:
I feel 3 files are excessive here. We can probably achieve everything in
just one file.

Not sure about mbm_assign_control interface as there are concerns with
group listing holding the lock for long.

-----------------------------------------------------------------------
Second phase, we can add support for "mkdir"

1. mkdir info/L3_MON/counter_configs/mbm_read_only

2. mkdir option will create "event_filter" file.
info/L3_MON/counter_configs/mbm_read_only/event_filter

3. Users can modify event configuration.
echo LclFill > info/L3_MON/counter_configs/mbm_read_only/event_filter

4. Users can assign the events

echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive

5. Events can be read in

test/mon_data/mon_data/mon_L3_00/mbm_read_only

--
Thanks
Babu Moger