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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Babu,

On 8/21/24 6:31 PM, Moger, Babu wrote:
Hi Reinette,

On 8/16/24 16:28, Reinette Chatre wrote:
Hi Babu,

On 8/6/24 3:00 PM, Babu Moger wrote:

Feature adds following interface files:

/sys/fs/resctrl/info/L3_MON/mbm_mode: Reports the list of assignable
monitoring features supported. The enclosed brackets indicate which
feature is enabled.

I've been considering this file as a generic file where all future "MBM
modes"
can be captured, while this series treats it as specific to "assignable
monitoring
features" (btw, should this be "assignable monitoring modes" to match the
name?).
Looking closer at this implementation it does make things easier that
"mbm_mode" is
specific to "assignable monitoring features" but when doing so I think it
should have
a less generic name to avoid the obstacles we have with the existing
"mon_features".
Apologies that this goes back to be close to what you had earlier ... maybe
"mbm_assign_mode"?

Lets see:
#cat /sys/fs/resctrl/info/L3_MON/mbm_mode
[mbm_cntr_assign]  <- This already says 'assign'. Isn't that enough?

It will be enough if "mbm_mode" is intended to be used for all current
and future MBM modes/features but this series instead dedicates this file
to just "assignable monitoring counters" feature. Doing so prevents us
from, in the future, expanding this file to, for example, contain
a new entry representing a new feature.


default            <-  Default mode is not related assignable features.

If not assignable features, what is it related to? "default" being the
absence of assignable features still seems related to me.


I would think mbm_mode is fine. Let me know.

If this work is reworded that it is intended to support any MBM mode then
it is fine, if this work remains to dedicate this file just to assignable
features then I think its name should be changed.

...


    Flags can be one of the following:

         t  MBM total event.
         l  MBM local event.
         tl Both total and local MBM events.
         _  None of the MBM events. Only works with '=' opcode.

Please take care with the implementation that seems to support a variety of
combinations. If I understand correctly the implementation support flags
like,
for example, "tttt", "llll", "ltlt" ... those may not be an issue but of most
concern is, for example, a pattern like "_lt" that (unexpectedly) appears to
result in set of total and local.

Yes. Should we not allow flag combinations with "_"?
I am not very sure about how to go about this.


This topic seems to have moved to patch #22.

...

    # echo "legacy" > /sys/fs/resctrl/info/L3_MON/mbm_mode
    # cat /sys/fs/resctrl/info/L3_MON/mbm_mode
    mbm_cntr_assign
    [legacy]

k. Unmount the resctrl
    #umount /sys/fs/resctrl/
---
v6:
   We still need to finalize few interface details on mbm_mode and
mbm_control
   in case of ABMC and Soft-ABMC. We can continue the discussion with
this series.

Could you please list the details that need to be finalized?

1. mbm_mode display
    # cat /sys/fs/resctrl/info/L3_MON/mbm_mode
      mbm_cntr_assign
      [legacy]

     "mbm_cntr_assign"
      Are we sticking with ""mbm_cntr_assign" for ABMC?
      What should we name for soft-ABMC?

2. Also we had some concerns about Individual event assignment(ABMC)
    and group assignment(soft-ABMC)?
    Are the flags "t" and 'l' good for both these modes?

If I remember correctly the previous discussion ended with the need for
"modes" that indicate to user space what to expect when interacting with the
MBM flags in the "mbm_control" file. The term used by ABMC should reflect that
each MBM flag/event can be set independently, while the term used by soft-ABMC
reflects that setting one flag/event makes the same change to the other
MBM flag/event.

Reinette




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux