Re: [PATCH 00/15] x86/resctrl : Support AMD QoS RMID Pinning feature

[Reinette Chatre]

Hi Peter,

On 12/8/2023 11:45 AM, Peter Newman wrote:
> On Tue, Dec 5, 2023 at 3:17 PM Reinette Chatre
> <reinette.chatre@xxxxxxxxx> wrote:
>> On 11/30/2023 4:57 PM, Babu Moger wrote:
>>> c. Read the monitor states. There will be new file "monitor_state"
>>>    for each monitor group when ABMC feature is enabled. By default,
>>>    both total and local MBM events are in "unassign" state.
>>>
>>>       #cat /sys/fs/resctrl/monitor_state
>>>       total=unassign;local=unassign
>>>
>>> d. Read the event mbm_total_bytes and mbm_local_bytes. Note that MBA
>>>    events are not available until the user assigns the events explicitly.
>>>    Users need to assign the counters to monitor the events in this mode.
>>>
>>>       #cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_total_bytes
>>>       Unavailable
>>
>> How is the llc_occupancy event impacted when ABMC is enabled? Can all RMIDs
>> still be used to track cache occupancy?
>>
>>>
>>>       #cat /sys/fs/resctrl/mon_data/mon_L3_00/mbm_local_bytes
>>>       Unavailable
>>
>> I believe that "Unavailable" already has an accepted meaning within current
>> interface and is associated with temporary failure. Even the AMD spec states "This
>> is generally a temporary condition and subsequent reads may succeed". In the
>> scenario above there is no chance that this counter would produce a value later.
>> I do not think it is ideal to overload existing interface with different meanings
>> associated with a new hardware specific feature ... something like "Disabled" seems
>> more appropriate.
> 
> Could we hide event counter files if they're not enabled? Is there
> value in displaying the value of a non-running counter that will be
> reset the next time it's enabled?

It may be possible to hide the counter file when it is disabled but in this
case it is not clear to me how to communicate to user space that it is
an available counter that can be enabled and by hiding the file one mechanism
to actually enable the counter is lost. It is not required to
display a stale value when a counter is disabled, text like "Disabled"
can be used.

>> Considering this we may even consider using these files themselves as a
>> way to enable the counters if they are disabled. For example, just
>> "echo 1 > /sys/fs/resctrl/mon_data/mon_L3_00/mbm_total_bytes" can be used
>> to enable this counter. No need for a new "monitor_state". Please note that this
>> is not an official proposal since there are two other use cases that still need to
>> be considered as we await James's feedback on how this may work for MPAM and
>> also how this may be useful on AMD hardware that does not support ABMC but
>> users may want to get similar benefits ([1])
> 
> We plan to use the ABMC counters as a window to sample the MB/s rate
> of a very large number of groups, so there's a serious concern about
> the number of write syscalls this will take, as they will add up
> quickly for a large RMID*domain count.
> 
> To that end, the ideal would be the ability to re-assign all ABMC
> counters on all domains in a single system call.

Understood. I've already pointed out that this is a use case needing
to be considered. Please see [1] - search for "global enable/disable".

Reinette

[1] https://lore.kernel.org/lkml/e36699cf-c73e-401b-b770-63eba708df38@xxxxxxxxx/