Received-SPF: pass (google.com: domain of linux-kernel+bounces-59538-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <89dc9022-c9a4-42dc-8710-be2d6d9a4e7c@amd.com>
Date: Fri, 9 Feb 2024 09:29:16 -0600
User-Agent: Mozilla Thunderbird
From: "Moger, Babu" <babu.moger@amd.com>
Subject: Re: [PATCH v15-RFC 6/8] x86/resctrl: Introduce snc_nodes_per_l3_cache
Reply-To: babu.moger@amd.com
To: Tony Luck <tony.luck@intel.com>, 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
Cc: 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
References: <20240126223837.21835-1-tony.luck@intel.com>
 <20240130222034.37181-1-tony.luck@intel.com>
 <20240130222034.37181-7-tony.luck@intel.com>
Content-Language: en-US
In-Reply-To: <20240130222034.37181-7-tony.luck@intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?bFZpM0VNczhXc0hQS2tuYUtPQmRsRTVicG53eFk1TlRmOE1xWjd1d2w3Zzhl?=
 =?utf-8?B?SWNQQlFsamdjbDhxL2VSdVBYTlJ3VDdUNFJkZHZWb05ybG5KYVpUY0J6MllE?=
 =?utf-8?B?b0dBR2VDNlVzSlRLRXkxbnphZWNqWkdFSXlBekJ6bHlBT3RDZ1hORkx5NzFM?=
 =?utf-8?B?U2ZJOHBnQ293Wk5PREVCcTFlMjFvakVnaytxUFRLdXp5OC9HRkNtNGwybFFU?=
 =?utf-8?B?SUdxM0R2V1NsQ0dpSms5cDVXdzVrWUhOSFQxVGNEdlhKcm9YV2c3ckNaaTIw?=
 =?utf-8?B?dERUdW5wK1dHejhxTEcxaDVKSVFvcjVOTlRZVGIwUUszRVkzeHFYa2dFRGJI?=
 =?utf-8?B?UWthYjVnZi9CR2dlcFh4UURUWTJrMDJNQzhBUFBhY3VDNVdGbzVIMEZFT2x0?=
 =?utf-8?B?Qlc1cDFsVU1oQzEzRFVIR1VMb0V0M0draTF0SjhXNUkyaVNwbWpTdTM3U0NI?=
 =?utf-8?B?V0g1cmd0SnQvYUp5MU5wMTRXTmNONGh2N3BES1FGTnIzN2RjWDRUSE5FcjBl?=
 =?utf-8?B?N08yeVZTanppdUVqbkdNU2FucEh3VzFpY3RRQncxbGpla3VyUjVnZ1U3REkw?=
 =?utf-8?B?VHhFM1VkeE9yK095S015dFo4TTFFWVB2SWgzUytkbjdWVkN3dnJoR2lsRnpT?=
 =?utf-8?B?Y0taZ1RReDlVa0Y2THRDTlQ4TjZZZ2ZuTE1kRExsZTIxZVNsWHMzQVU1V2lM?=
 =?utf-8?B?Rlk3WDhMK0Z3dDVGRGNqK01EYzBNRUd0ZWZlUzVUVEd0UlVuMnJaZzUyWCs1?=
 =?utf-8?B?dHFDNDBaOGZnblI0UnlTMS8ySldUdjNSR0RkSGNibm9uMFdOdEdBbnQ2K3ln?=
 =?utf-8?B?VjUxb1FLWnZuVHBOWEp5b1pmWFpQbGliTGRYWVdkS1RWdTFHRnZzVGJyUm8z?=
 =?utf-8?B?UFEvcWdQMUQyck5xa01CcG0yYndnbGxxdWw1YnhZMGpLVS9pb0JkZzRvUEQy?=
 =?utf-8?B?VWorSWV5Ry8yMWVaSnBUUmZoWFkyVTk1bDJMRUNRVU54c28xQ0p2Um9NajFJ?=
 =?utf-8?B?TTd1bCtTR3o5R2gyZ1c2dlRTbXdUbXdBQ3A4STFRSjRRRjkwZFNKbk96b3VK?=
 =?utf-8?B?S09lYmF3ZFh1NWVtdXdYNUVUeVlQbU1sbE9QdUF6OFRJb2NDYnRoeER1a3J1?=
 =?utf-8?B?QXJHVEhFdU52NzdmamtrY2FrRUdvaDA0MWlxMTRQeW1teHIvSnZ2R29lRDJl?=
 =?utf-8?B?YmJ1ZHVCNS9NYUdIbVVTcHlzc1l4bFMzeTdKR0xyOVFVakk5aEZWdEMxOWNK?=
 =?utf-8?B?VHNJRkUvbC8xeFBUazBQK3cyYXpOOVRSS0dRZmlXdjdpNUNHMXdkUHljMnVn?=
 =?utf-8?B?THR4QzZwdXNlOWU5VXhrdjdEcWZNOTI4N1MwYmpnN2Q4YnJWZVIrZm95UEVq?=
 =?utf-8?B?bThQVUx3bC80dGllOXdTZnlFTHh0eHlxdk1XUFNoanUySm1keTdzbVhwR1cz?=
 =?utf-8?B?ejMzVzdoaGZrTGkvbGhnRHZEbFJXNEZyZGpPVlgxYVlGNnJ5czJBeEVNYTNY?=
 =?utf-8?B?YjZ3bjRUZTdnRGZpQkRvUGF2UWV6a3lkMDk4dHY2KzQrbGg2RTdDbjRLZVlM?=
 =?utf-8?B?L2lJTURSNXF6VHQ0OFFRVnR1UTVwUENEcnY5ZHIvZlpyMGdIN3NkVFRtcXhw?=
 =?utf-8?B?UlF2T0VoamZjWmdGVXJseXlyU0JhRng4NjN1RHBIOEVmTVhUVk5RYzdUVktO?=
 =?utf-8?B?T05qTkliR2lkVjk5Nzh3SC9uQXZ5Z0UvRlBmSVlpanU1T01OSU83WS9FSkxX?=
 =?utf-8?B?Mk9QZktFOStEVDY3ZC8rN2grb2c1NHdTcFNVUkVoY3MrR3FCbUpVZXY1MmJC?=
 =?utf-8?B?OWpSVWtTTUVwcmhMQWV2NXdQR2t0YVhJZFR5dkw4Y3VGRlBxblUvNFpIOTYw?=
 =?utf-8?B?YUt5TmdvdXU4U2dTL0FpbE96LytFbFJvdENWaWhvRjhGZWJyZUdBcjdGbnZq?=
 =?utf-8?B?dnJUTVVjbGo3ZGNjTENpRDlTYkhvemg2Mm1IcjhMZEJCR2pGVWoyNWdMNzFu?=
 =?utf-8?B?OEdCVDkvMm0xR2xQVllOK21KdVVnaU83SkFZd2hEaDRYdTVEcy9YTy9GcXc1?=
 =?utf-8?B?emVaU1k5OUozeUxKUHlaaDVvTlMzMEpzWm1qMFF1ZVQrRTdNYTcvOTlqcUIy?=
 =?utf-8?Q?r74w=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4bdd5030-16f1-4754-6e0f-08dc2983e15b
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 15:29:17.9213
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WCvzIwpnCg8QS9vKlc0AHn9YFQLPWxrB3AAeggK9Oyx+WcFxZhSwR/cKjZdV2CPP
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7503

Hi Tony,

On 1/30/24 16:20, Tony Luck wrote:
> Intel Sub-NUMA Cluster (SNC) is a feature that subdivides the CPU cores
> and memory controllers on a socket into two or more groups. These are
> presented to the operating system as NUMA nodes.
> 
> This may enable some workloads to have slightly lower latency to memory
> as the memory controller(s) in an SNC node are electrically closer to the
> CPU cores on that SNC node. This cost may be offset by lower bandwidth
> since the memory accesses for each core can only be interleaved between
> the memory controllers on the same SNC node.
> 
> Resctrl monitoring on an Intel system depends upon attaching RMIDs to tasks
> to track L3 cache occupancy and memory bandwidth. There is an MSR that
> controls how the RMIDs are shared between SNC nodes.
> 
> The default mode divides them numerically. E.g. when there are two SNC
> nodes on a socket the lower number half of the RMIDs are given to the
> first node, the remainder to the second node. This would be difficult
> to use with the Linux resctrl interface as specific RMID values assigned
> to resctrl groups are not visible to users.
> 
> The other mode divides the RMIDs and renumbers the ones on the second
> SNC node to start from zero.
> 
> Even with this renumbering SNC mode requires several changes in resctrl
> behavior for correct operation.
> 
> Add a global integer "snc_nodes_per_l3_cache" that shows how many
> SNC nodes share each L3 cache. When "snc_nodes_per_l3_cache" is "1",
> SNC mode is either not implemented, or not enabled.
> 
> Update all places to take appropriate action when SNC mode is enabled:
> 1) The number of logical RMIDs per L3 cache available for use is the
>    number of physical RMIDs divided by the number of SNC nodes.
> 2) Likewise the "mon_scale" value must be divided by the number of SNC
>    nodes.
> 3) The RMID renumbering operates when using the value from the
>    IA32_PQR_ASSOC MSR to count accesses by a task. When reading an RMID
>    counter, adjust from the logical RMID to the physical
>    RMID value for the SNC node that it wishes to read and load the
>    adjusted value into the IA32_QM_EVTSEL MSR.
> 4) Divide the L3 cache between the SNC nodes. Divide the value
>    reported in the resctrl "size" file by the number of SNC
>    nodes because the effective amount of cache that can be allocated
>    is reduced by that factor.
> 5) Disable the "-o mba_MBps" mount option in SNC mode
>    because the monitoring is being done per SNC node, while the
>    bandwidth allocation is still done at the L3 cache scope.
>    Trying to use this feedback loop might result in contradictory
>    changes to the throttling level coming from each of the SNC
>    node bandwidth measurements.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/internal.h |  2 ++
>  arch/x86/kernel/cpu/resctrl/core.c     |  6 ++++++
>  arch/x86/kernel/cpu/resctrl/monitor.c  | 16 +++++++++++++---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |  5 +++--
>  4 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index c6051bc70e96..d9c6dcf30922 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -428,6 +428,8 @@ DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
>  
>  extern struct dentry *debugfs_resctrl;
>  
> +extern unsigned int snc_nodes_per_l3_cache;

I feel this can be part of rdt_resource instead of global.


> +
>  enum resctrl_res_level {
>  	RDT_RESOURCE_L3_MON,
>  	RDT_RESOURCE_L3,
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index b741cbf61843..dc886d2c9a33 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -48,6 +48,12 @@ int max_name_width, max_data_width;
>   */
>  bool rdt_alloc_capable;
>  
> +/*
> + * Number of SNC nodes that share each L3 cache.  Default is 1 for
> + * systems that do not support SNC, or have SNC disabled.
> + */
> +unsigned int snc_nodes_per_l3_cache = 1;
> +
>  static void
>  mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m,
>  		struct rdt_resource *r);
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 080cad0d7288..357919bbadbe 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -148,8 +148,18 @@ static inline struct rmid_entry *__rmid_entry(u32 rmid)
>  
>  static int __rmid_read(u32 rmid, enum resctrl_event_id eventid, u64 *val)
>  {
> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;

RDT_RESOURCE_L3_MON?


> +	int cpu = smp_processor_id();
> +	int rmid_offset = 0;
>  	u64 msr_val;
>  
> +	/*
> +	 * When SNC mode is on, need to compute the offset to read the
> +	 * physical RMID counter for the node to which this CPU belongs.
> +	 */
> +	if (snc_nodes_per_l3_cache > 1)
> +		rmid_offset = (cpu_to_node(cpu) % snc_nodes_per_l3_cache) * r->num_rmid;

Not sure if you have tested or not. r->num_rmid is initialized for the
resource RDT_RESOURCE_L3_MON. For other resource it is always 0.


> +
>  	/*
>  	 * As per the SDM, when IA32_QM_EVTSEL.EvtID (bits 7:0) is configured
>  	 * with a valid event code for supported resource type and the bits
> @@ -158,7 +168,7 @@ static int __rmid_read(u32 rmid, enum resctrl_event_id eventid, u64 *val)
>  	 * IA32_QM_CTR.Error (bit 63) and IA32_QM_CTR.Unavailable (bit 62)
>  	 * are error bits.
>  	 */
> -	wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
> +	wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid + rmid_offset);
>  	rdmsrl(MSR_IA32_QM_CTR, msr_val);
>  
>  	if (msr_val & RMID_VAL_ERROR)
> @@ -757,8 +767,8 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>  	int ret;
>  
>  	resctrl_rmid_realloc_limit = boot_cpu_data.x86_cache_size * 1024;
> -	hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale;
> -	r->num_rmid = boot_cpu_data.x86_cache_max_rmid + 1;
> +	hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale / snc_nodes_per_l3_cache;
> +	r->num_rmid = (boot_cpu_data.x86_cache_max_rmid + 1) / snc_nodes_per_l3_cache;
>  	hw_res->mbm_width = MBM_CNTR_WIDTH_BASE;
>  
>  	if (mbm_offset > 0 && mbm_offset <= MBM_CNTR_WIDTH_OFFSET_MAX)
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 770f2bf98462..e639069f871a 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1425,7 +1425,7 @@ unsigned int rdtgroup_cbm_to_size(struct rdt_resource *r,
>  		}
>  	}
>  
> -	return size;
> +	return size / snc_nodes_per_l3_cache;
>  }
>  
>  /*
> @@ -2293,7 +2293,8 @@ static bool supports_mba_mbps(void)
>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl;
>  
>  	return (is_mbm_local_enabled() &&
> -		r->alloc_capable && is_mba_linear());
> +		r->alloc_capable && is_mba_linear() &&
> +		snc_nodes_per_l3_cache == 1);
>  }
>  
>  /*

-- 
Thanks
Babu Moger