Re: [RFC PATCH v3 00/17] x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC)

Dave Martin
2024-04-23T12:37:47Z

On Mon, Apr 22, 2024 at 03:44:26PM -0500, Moger, Babu wrote:
> Hi Dave,
>
> On 4/22/24 11:34, Dave Martin wrote:
> > Hi Babu,
> >
> > On Thu, Apr 04, 2024 at 03:02:45PM -0500, Moger, Babu wrote:
> >> Hi Peter,
> >>
> >>
> >> On 4/4/24 14:08, Peter Newman wrote:
> >>> Hi Babu,
> >>>
> >>> On Thu, Mar 28, 2024 at 6:07 PM Babu Moger <babu.moger@amd.com> wrote:
> >>>> The list follows the following format:
> >>>>
> >>>> * Default CTRL_MON group:
> >>>> "//<domain_id>=<assignment_flags>"
> >>>>
> >>>> * Non-default CTRL_MON group:
> >>>> "<CTRL_MON group>//<domain_id>=<assignment_flags>"
> >>>>
> >>>> * Child MON group of default CTRL_MON group:
> >>>> "/<MON group>/<domain_id>=<assignment_flags>"
> >>>>
> >>>> * Child MON group of non-default CTRL_MON group:
> >>>> "<CTRL_MON group>/<MON group>/<domain_id>=<assignment_flags>"
> >>>>
> >>>> Assignment flags can be one of the following:
> >>>>
> >>>> t MBM total event is assigned
> >>>> l MBM local event is assigned
> >>>> tl Both total and local MBM events are assigned
> >>>> _ None of the MBM events are assigned
> >>>>
> >>>> Examples:
> >>>>
> >>>> # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
> >>>> non_defult_group//0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;
> >>>> non_defult_group/non_default_mon1/0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;
> >>>> //0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;
> >>>> /default_mon1/0=tl;1=tl;2=tl;3=tl;4=tl;5=tl;6=tl;7=tl;
> >>>>
> >>>> There are four groups and all the groups have local and total event assigned.
> >>>>
> >>>> "//" - This is a default CONTROL MON group
> >>>>
> >>>> "non_defult_group//" - This is non default CONTROL MON group
> >>>>
> >>>> "/default_mon1/" - This is Child MON group of the defult group
> >>>>
> >>>> "non_defult_group/non_default_mon1/" - This is child MON group of the non default group
> >>>>
> >>>> =tl means both total and local events are assigned.
> >>>
> >>> I recall there was supposed to be a way to perform the same update on
> >>> all domains together so that it isn't tedious to not do per-domain
> >>
> >> Yes. Correct. Reinette suggested to have "no domains" means ALL the domains.
> >
> > Would "*" be more intuitive?
>
> We could. But I don't see the need for wildcard ("*") or ranges and
> complexity that comes with that.

For "*", I mean that this would just stand for "all cpus", not a generic
string match; apologies if I didn't make that clear.

I think that an explicit "*" is still a less surprising way to say
"everything" than "" (which if it means anything at all, usually means
"nothing").

I may have misunderstood the intention here: _if_ the intention is to
provide a way to enable/disable an event in all domains without having
to enumerate them all one by one, then I think "*" is preferable syntax
to "". That was my only real suggestion here.

>
> Even in schemata processing we don't use the wildcard or ranges and also
> there is no mention of that in documentation.
> https://www.kernel.org/doc/Documentation/x86/resctrl.rst

I know, though writing the schemata files can be tedious and annoying,
since their content is often very repetitive, so ...

>
> Domains(or nodes) are processed one by one. Some examples.
>
> # cat schemata
> SMBA:0=2048;1=2048;2=2048;3=2048
> MB:0=2048;1=2048;2=2048;3=2048
> L3:0=ffff;1=ffff;2=ffff;3=ffff
>
> # echo "SMBA:1=64" > schemata
> # cat schemata
> SMBA:0=2048;1= 64;2=2048;3=2048
> MB:0=2048;1=2048;2=2048;3=2048
> L3:0=ffff;1=ffff;2=ffff;3=ffff

... it would be convenient to be able to do something like

# echo "SMBA:*=64" >schemata
# grep SMBA: schemata
SMBA:0= 64;1= 64;2= 64;3= 64

Anyway, this is nothing directly to do with this series; just a
thought.

> > Whatever is done here to describe the "wildcard node", would it be worth
> > having the node field parse the same way in the "schemata" files?
> >
> > Is there any merit in having range match expressions, e.g. something like
> >
> > 0-3,8-11=foo;4-7,12-*=bar
> >
> > (The latter is obvious feature creep though, so a real use case for this
> > would be needed to justify it. I don't have one right now...)

[...]

> Thanks
> Babu Moger

I do agree that unless someone jumps up and down saying this would
help their use case, this is probably a step too far.

Just thinking aloud (and this kind of feature could be added later in a
backwards compatible way if someone really needs it).

Cheers
---Dave