Received-SPF: pass (google.com: domain of linux-kernel+bounces-59826-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Fri, 9 Feb 2024 10:44:58 -0800
From: Tony Luck <tony.luck@intel.com>
To: "Moger, Babu" <babu.moger@amd.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Peter Newman <peternewman@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>, x86@kernel.org,
	Shaopeng Tan <tan.shaopeng@fujitsu.com>,
	James Morse <james.morse@arm.com>,
	Jamie Iles <quic_jiles@quicinc.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Drew Fustini <dfustini@baylibre.com>, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, patches@lists.linux.dev
Subject: Re: [PATCH v15-RFC 1/8] x86/resctrl: Split the RDT_RESOURCE_L3
 resource
Message-ID: <ZcZyqs5hnQqZ5ZV0@agluck-desk3>
References: <20240126223837.21835-1-tony.luck@intel.com>
 <20240130222034.37181-1-tony.luck@intel.com>
 <20240130222034.37181-2-tony.luck@intel.com>
 <c9629380-c97c-4797-b6b3-5044d5beeb97@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c9629380-c97c-4797-b6b3-5044d5beeb97@amd.com>

On Fri, Feb 09, 2024 at 09:28:16AM -0600, Moger, Babu wrote:
> >  enum resctrl_res_level {
> > +	RDT_RESOURCE_L3_MON,
> >  	RDT_RESOURCE_L3,
> 
> How about?
> RDT_RESOURCE_L3,
> RDT_RESOURCE_L3_MON,

Does the order matter? I put the L3_MON one first because historically
cache occupancy was the first resctrl tool. But if you have a better
argument for the order, then I can change it.

> >  	RDT_RESOURCE_L2,
> >  	RDT_RESOURCE_MBA,
> > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> > index aa9810a64258..c50f55d7790e 100644
> > --- a/arch/x86/kernel/cpu/resctrl/core.c
> > +++ b/arch/x86/kernel/cpu/resctrl/core.c
> > @@ -60,6 +60,16 @@ mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m,
> >  #define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].r_resctrl.domains)
> >  
> >  struct rdt_hw_resource rdt_resources_all[] = {
> > +	[RDT_RESOURCE_L3_MON] =
> > +	{
> > +		.r_resctrl = {
> > +			.rid			= RDT_RESOURCE_L3_MON,
> > +			.name			= "L3",
> 
> L3_MON ?

That was my first choice too. But I found:

$ ls /sys/fs/resctrl/info
L3  L3_MON_MON  last_cmd_status  MB

This would be easy to fix ... just change this code to not append
an extra "_MON" to the directory name:

        for_each_mon_capable_rdt_resource(r) {
                fflags = r->fflags | RFTYPE_MON_INFO;
                sprintf(name, "%s_MON", r->name);
                ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
                if (ret)
                        goto out_destroy;
        }

But I also saw this:
$ ls /sys/fs/resctrl/mon_data/
mon_L3_MON_00  mon_L3_MON_01

which didn't seem to have an easy fix. So I took the easy route and left
the ".name" field as "L3_MON".

-Tony