Received-SPF: pass (google.com: domain of linux-kernel+bounces-140608-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Thu, 11 Apr 2024 15:13:36 +0100
From: Dave Martin <Dave.Martin@arm.com>
To: David Hildenbrand <david@redhat.com>
Cc: James Morse <james.morse@arm.com>, x86@kernel.org,
	linux-kernel@vger.kernel.org, Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	shameerali.kolothum.thodi@huawei.com,
	D Scott Phillips OS <scott@os.amperecomputing.com>,
	carl@os.amperecomputing.com, lcherian@marvell.com,
	bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com,
	baolin.wang@linux.alibaba.com, Jamie Iles <quic_jiles@quicinc.com>,
	Xin Hao <xhao@linux.alibaba.com>, peternewman@google.com,
	dfustini@baylibre.com, amitsinght@marvell.com,
	Rex Nie <rex.nie@jaguarmicro.com>
Subject: Re: [PATCH v1 01/31] x86/resctrl: Fix allocation of cleanest CLOSID
 on platforms with no monitors
Message-ID: <ZhfwEF5EJiTM5Hjm@e133380.arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-2-james.morse@arm.com>
 <1e19965c-51f2-4f7a-8d29-e40f4e54a72c@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1e19965c-51f2-4f7a-8d29-e40f4e54a72c@redhat.com>

On Tue, Apr 09, 2024 at 10:05:33AM +0200, David Hildenbrand wrote:
> On 21.03.24 17:50, James Morse wrote:
> > commit 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by
> > searching closid_num_dirty_rmid") added a Kconfig option that causes
> > resctrl to search for the CLOSID with the fewest dirty cache lines when
> > creating a new control group. This depends on the values read from the
> > llc_occupancy counters.

[...]

> It's weird to not see RESCTRL_RMID_DEPENDS_ON_CLOSID appear in any Kconfig file.
> I guess it will all make sense once the refactoring is done :)

Agreed; a stub Kconfig item could be added, but since the file layout
and naming conventions change after this patch, doing this would
probably just create noise in the series though.

Looking at <linux/kconfig.h> (yikes!), IS_ENABLED() is designed to do
the right thing for non-existing Kconfigs...

If nobody is too concerned about the temporarily dangling IS_ENABLED()s
in this series, I won't propose any change here.


> As Reinette comments, likely we want here:
> 
> Fixes: 6eac36bb9eb0 ("x86/resctrl: Allocate the cleanest CLOSID by searching closid_num_dirty_rmid")

Noted for James' attention.

> 
> > Signed-off-by: James Morse <james.morse@arm.com>
> > ---
> >   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > index 011e17efb1a6..1767c1affa60 100644
> > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > @@ -149,7 +149,8 @@ static int closid_alloc(void)
> >   	lockdep_assert_held(&rdtgroup_mutex);
> > -	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) {
> > +	if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID) &&
> > +	    is_llc_occupancy_enabled()) {
> >   		cleanest_closid = resctrl_find_cleanest_closid();
> >   		if (cleanest_closid < 0)
> >   			return cleanest_closid;
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> -- 
> Cheers,
> 
> David / dhildenb

Thanks; noted for James' attention.

Cheers
---Dave