[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZJX+lmQTUEujF88U@agluck-desk3>
Date: Fri, 23 Jun 2023 13:20:38 -0700
From: Tony Luck <tony.luck@...el.com>
To: Peter Newman <peternewman@...gle.com>
Cc: "Yu, Fenghua" <fenghua.yu@...el.com>,
"Chatre, Reinette" <reinette.chatre@...el.com>,
Jonathan Corbet <corbet@....net>,
"x86@...nel.org" <x86@...nel.org>,
Shaopeng Tan <tan.shaopeng@...itsu.com>,
James Morse <james.morse@....com>,
Jamie Iles <quic_jiles@...cinc.com>,
Babu Moger <babu.moger@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"patches@...ts.linux.dev" <patches@...ts.linux.dev>,
"Eranian, Stephane" <eranian@...gle.com>
Subject: Re: [PATCH v2 7/7] x86/resctrl: Determine if Sub-NUMA Cluster is
enabled and initialize.
On Fri, Jun 23, 2023 at 05:19:37PM +0200, Peter Newman wrote:
> Hi Tony,
>
> On Thu, Jun 22, 2023 at 6:02 PM Luck, Tony <tony.luck@...el.com> wrote:
> >
> > > Unfortunately I'm not getting as good of results with the new series.
> > > The main difference seems to be updating the 0xca0 MSR instead of
> > > applying the offset to PQR_ASSOC.
> >
> > I think I may have reversed the actions to update the MSR in one of
> > my refactor/rebase. The comment here is correct, but that's not
> > what the code is doing :-(
> >
> > Can you swap the bodies of these two functions and retest?
>
> It's a small improvement, but still not great. Still only node 0
> giving believable results, but at least no more empty results from the
> second package.
>
> I poked around in /proc/kcore and noticed that my snc_ways is still 1, though.
Below is the patch I applied to reverse the add/remove package actions
together with some debug to make double sure SNC is being detected as
I expect and the right actions taken.
When booting the debug messages say:
[ 9.458624] resctrl: SNC_ways = 2
[ 9.458801] resctrl: CPU0: set MSR_RMID_SNC_CONFIG to 0x0
[ 9.461986] resctrl: CPU56: set MSR_RMID_SNC_CONFIG to 0x0
which is all good and correct.
For my tests I have a memory hog process that loops on a memcpy()
of 100 MBytes to generate enough traffic to be totally obvious when
looking at the mbm counters.
Test 1: I used taskset(1) to start a copy on the first CPU of node0
and checked the MBM counters. Both local and remote showed around
25 GB/s on node 0. Killed this process.
Tests 2, 3, 4: Same as 1, but started the process on first CPU of node
1, 2, 3. Same result. Around 25 GB/s appeared in the MBM counts for
the right node in each cycle.
Test 5: Started on node0, then periodically used taskset to bind the
running process onto a CPU on another node. It looks like Linux
migrates the memory for the job shortly after the affinity of the
process is changed. A few seconds after each process migration, the
MBM counters reflect traffic on the node that is the new home of the
process.
-Tony
>From 414341db02cd51daaf4a9ea8bd68b22a23cf4b59 Mon Sep 17 00:00:00 2001
From: Tony Luck <tony.luck@...el.com>
Date: Fri, 23 Jun 2023 08:57:57 -0700
Subject: [PATCH] Fix inverted SNC enable/disable MSR writes. Add some debug
too.
---
arch/x86/kernel/cpu/resctrl/core.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 869cfb46e8e4..e66b2b84fe6f 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -546,18 +546,20 @@ static void snc_add_pkg(void)
{
u64 msrval;
- rdmsrl(MSR_RMID_SNC_CONFIG, msrval);
- msrval |= BIT_ULL(0);
- wrmsrl(MSR_RMID_SNC_CONFIG, msrval);
-}
-
-static void snc_remove_pkg(void)
-{
- u64 msrval;
-
rdmsrl(MSR_RMID_SNC_CONFIG, msrval);
msrval &= ~BIT_ULL(0);
wrmsrl(MSR_RMID_SNC_CONFIG, msrval);
+pr_info("CPU%d: set MSR_RMID_SNC_CONFIG to 0x%llx\n", raw_smp_processor_id(), msrval);
+}
+
+static void snc_remove_pkg(void)
+{
+ u64 msrval;
+
+ rdmsrl(MSR_RMID_SNC_CONFIG, msrval);
+ msrval |= BIT_ULL(0);
+ wrmsrl(MSR_RMID_SNC_CONFIG, msrval);
+pr_info("CPU%d: set MSR_RMID_SNC_CONFIG to 0x%llx\n", raw_smp_processor_id(), msrval);
}
/*
@@ -987,6 +989,7 @@ static __init int find_snc_ways(void)
if (ret > 1)
rdt_resources_all[RDT_RESOURCE_PKG].r_resctrl.pkg_actions = true;
+pr_info("SNC_ways = %d\n", ret);
return ret;
}
--
2.40.1
Powered by blists - more mailing lists