Received-SPF: pass (google.com: domain of linux-kernel+bounces-155181-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 23 Apr 2024 13:37:33 +0100
From: Dave Martin <Dave.Martin@arm.com>
To: "Moger, Babu" <babu.moger@amd.com>
Cc: Peter Newman <peternewman@google.com>, corbet@lwn.net,
	fenghua.yu@intel.com, reinette.chatre@intel.com, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	x86@kernel.org, hpa@zytor.com, paulmck@kernel.org,
	rdunlap@infradead.org, tj@kernel.org, peterz@infradead.org,
	yanjiewtw@gmail.com, kim.phillips@amd.com, lukas.bulwahn@gmail.com,
	seanjc@google.com, jmattson@google.com, leitao@debian.org,
	jpoimboe@kernel.org, rick.p.edgecombe@intel.com,
	kirill.shutemov@linux.intel.com, jithu.joseph@intel.com,
	kai.huang@intel.com, kan.liang@linux.intel.com,
	daniel.sneddon@linux.intel.com, pbonzini@redhat.com,
	sandipan.das@amd.com, ilpo.jarvinen@linux.intel.com,
	maciej.wieczor-retman@intel.com, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, eranian@google.com,
	james.morse@arm.com
Subject: Re: [RFC PATCH v3 00/17] x86/resctrl : Support AMD Assignable
 Bandwidth Monitoring Counters (ABMC)
Message-ID: <ZierjRNDMfg5swT8@e133380.arm.com>
References: <cover.1711674410.git.babu.moger@amd.com>
 <CALPaoCheW=Jz2R3gMKcgwQe6ONDrRqu3tUxeg=A3USg6BC8buA@mail.gmail.com>
 <7ccd59b8-9fe3-4d1f-82f5-f33d96dbf5ac@amd.com>
 <ZiaRewZJnLaDSx3m@e133380.arm.com>
 <004dcaeb-30ae-4b27-895a-4025a670fcbf@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <004dcaeb-30ae-4b27-895a-4025a670fcbf@amd.com>

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