Received-SPF: pass (google.com: domain of linux-kernel+bounces-195876-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <fe9a4cd5-f74d-4c89-8667-8b5b1841b84e@intel.com>
Date: Thu, 30 May 2024 13:21:01 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 08/20] x86/resctrl: Prepare for new Sub-NUMA Cluster
 (SNC) monitor files
To: Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@intel.com>,
	"Maciej Wieczor-Retman" <maciej.wieczor-retman@intel.com>, Peter Newman
	<peternewman@google.com>, James Morse <james.morse@arm.com>, Babu Moger
	<babu.moger@amd.com>, Drew Fustini <dfustini@baylibre.com>, Dave Martin
	<Dave.Martin@arm.com>
CC: <x86@kernel.org>, <linux-kernel@vger.kernel.org>,
	<patches@lists.linux.dev>
References: <20240528222006.58283-1-tony.luck@intel.com>
 <20240528222006.58283-9-tony.luck@intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
Content-Language: en-US
In-Reply-To: <20240528222006.58283-9-tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Z0EvMXF4R0VrcVB4RjhkcHJHRjQ3RXNFNFlTRkRyOGVZNlVsUG5rb2srTjhK?=
 =?utf-8?B?K2FSZTNRVi9QaVpWbDJDYklWTTFNcFdjK3QrbXdvbWdUTEFpbDcyWTRKWXhC?=
 =?utf-8?B?TDM4d0FXbmh0bXRZMG0vbFQ2OWdrZjFCK2hLZFBldlE1Sk1SN3dRTjRPNURQ?=
 =?utf-8?B?eCtkNVFrN1FDZUJjYW4vV1kwQi9lM3F3eC9QWFVHaU5LNFMvRGlZSGFyaTk1?=
 =?utf-8?B?b01ySlRlNHdmck5LOTcvQmNYaFh2RTVMbE9mSHBlQkVET0h2YmswL3R1eFJr?=
 =?utf-8?B?M2ZReUhCV20yeERiOGsvZnI3bzM3WE4rbm9MbDBUM3RaYnZxSCtSL2tWckJt?=
 =?utf-8?B?Y1U2TkJsaDJoUkFMWEFneU53KzVSeXZYYmM1SjEyUWpYUWhJYXZMcDVyaThy?=
 =?utf-8?B?NE9MdDIxRUQrU0x3ZHN5VStQSlRSUXlJblloUUpwTWtnVFkzWUxyUXpMdXhW?=
 =?utf-8?B?TXNzaGpuQjV2RnhONzNFMEk3WmNpUVVpYkxDaTBWVEEvWWdQc2xLMEoyS0pM?=
 =?utf-8?B?Q0lZK3pQSkdaenl5bUwrVHRjT0JqNmRHWTdTTkVVMWdaRGhYVWI5cSs2RHR1?=
 =?utf-8?B?N3lZaW1CY29ZaWgvQW1qT2kvekprM1U5S0hTQ09hR3lRc1J2RnRMeHZDTG1V?=
 =?utf-8?B?MTNxMlRrR0tBOTZSckdIaC9uRGJCQS81KzBPMUZDR2t3RW5jTTY3NDc4TDd0?=
 =?utf-8?B?aFdhVTc0a3F6N29pK0k2VkR1czFmNXM4RWRwT0EvbTNqVyt3ZjEwaWkwV2pz?=
 =?utf-8?B?bHRtMS9EY0h0dXVSYXhJSnFsaGhNd2FURXoxdFRtdFhpTENlV3h4SlN5WlE3?=
 =?utf-8?B?UGEycG4rdUYyak4wN2gxMk54SnVwdGZsNmROK3diRE1xSU04QnJWVWF5aTR0?=
 =?utf-8?B?bHRJVStYZGNCb1h2TUFkWm9NeHgvLzZmeUF2cS9ySzMrcHlpSkozVGR2aVpR?=
 =?utf-8?B?YnhVMWR2Y0g4bEVwM0tUN3hNcEIzV1U0MEZwUGxxcHg4M3JQRW5jRjBjOXdy?=
 =?utf-8?B?MC9yc1ZkV3V6c0VjeWJLdkpuQXJYcjdGOVdTQUtjemdsZ3FFOWduMVlnSVFO?=
 =?utf-8?B?bjVmWU0vS2pQL0diVU9TWlZ3ZU1MZkZjWUY3cEZ1eGhzK0lXV2tXdDY2cGEz?=
 =?utf-8?B?UUF2RGdxZDR4VmJva1E4Y2ZMclBEMURkaWtxaStxbkQ3L1d2SVUyY0txeTZN?=
 =?utf-8?B?RGZ2WlBnWE5OVnN4VHdNM25tWmZHQXU4UWlaUVdKQ09vdXNrZUZNUGlWcFFt?=
 =?utf-8?B?RzBleGJGbjZMWHhwd1dyMjUwVlRTbERwckNNYktQWERBcWYwNzNhUXRoYlIv?=
 =?utf-8?B?eENnWTVhRUN4K0FHa1hKd3o2VS84S1BZb2VObmxBeG1SZEVLWHRDWGpxWDEw?=
 =?utf-8?B?djdSQlgrQ24zYm9PTEJqc1JjSEYrS1B6L3NCOGoxTXByYlFHTzhGdE5WNEVX?=
 =?utf-8?B?TmpGWlJPRmlnd0Evc1IzTDNVU3JjZTBhVE9QYTlTeE9qZytwblBuZ3hPbzNm?=
 =?utf-8?B?VVNMd05BTmhRNUFFUU1WMGs5YzNLZUdvKzk4TFdBbFhjWmtnRTBKRllqZ0No?=
 =?utf-8?B?dGRBRHFHVUpqRUlPMGdPcVVCVHI0dlNFclAwaGd5Wnp5S2lEWFIzSUdVc1cz?=
 =?utf-8?B?R09GTXVlOU4wcjIxbFMxd1ZuaXZEMnBJUW1zeVphdDVWRnQ0T2tINE9seHNs?=
 =?utf-8?B?aG5TMTV5V3lINjlwemNYM205cnl1TlluTlFHMU02MDJtSUpsbWYwOWwyR2ht?=
 =?utf-8?B?NFlORjc0TmtoanEyN0NGNjRPVXkwQ2FyQjMvbUlVb0NWU2lJUk5YMVcycmRw?=
 =?utf-8?B?ZlRQUkg2NDBiMGdpSGs3TnhJVlNuMXBMbGx6U3ZtM2psWlJ1b2F1SDZmTzJQ?=
 =?utf-8?B?NjltWVIvZSsyQlJXV3FjVjN5QWEyby9xYy9sTnRzRWVDdEhDTFdXcTlyLzZp?=
 =?utf-8?B?UGlrSWxRS2w0MjF3bmdkaytpTjdkMVU2SmxrQzhvZjg1RC9DQ2RLVUZWR3dL?=
 =?utf-8?B?U096aTY1U3FrWUI5Y0tkOUZaMllEb1liM2dsZEtiWU1mQ3JhbXQ0OXlnbEZw?=
 =?utf-8?B?dnJpOVMyNXpvL2sxOGVNNlIzQnlVd08wb3pGdVRFZ0xuT2dVWTJYeGpSY2pq?=
 =?utf-8?B?MWxza0JsekpFaW1hZWttbXNPcjBHOUJsLzh4ckNXUVRSVnpmcEdvUXJmUzk2?=
 =?utf-8?B?RUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 22662501-6c7b-45f4-a392-08dc80e607ca
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 20:21:04.2713
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PHOdht30vodLDJgJW6PXBbFH8SMC0/0O+sqKDERESJM/E+FZPDmw052goh2d4b1oUuFFFXOvc71mZCv5Lz6H4UR23/PrJjLYoX9PDNb8TW4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6453
X-OriginatorOrg: intel.com

Hi Tony,

On 5/28/24 3:19 PM, Tony Luck wrote:
> When SNC is enabled monitoring data is collected at the SNC node
> granularity, but must be reported at L3-cache granularity for
> backwards compatibility in addition to reporting at the node
> level.
> 
> Add a "ci" field to the rdt_mon_domain structure to save the
> cache information about the enclosing L3 cache for the domain.
> This provides:
> 
> 1) The cache id which is needed to compose the name of the legacy
> monitoring directory, and to determine which domains should be
> summed to provide L3-scoped data.
> 
> 2) The shared_cpu_map which is needed to determine which CPUs can
> be used to read the RMID counters with the MSR interface.
> 
> This is the first step to an eventual goal of monitor reporting files
> like this (for a system with two SNC nodes per L3):
> 
> $ cd /sys/fs/resctrl/mon_data
> $ tree mon_L3_00
> mon_L3_00			<- 00 here is L3 cache id
> ├── llc_occupancy		\  These files provide legacy support
> ├── mbm_local_bytes		 > for non-SNC aware monitor apps
> ├── mbm_total_bytes		/  that expect data at L3 cache level
> ├── mon_sub_L3_00		<- 00 here is SNC node id
> │   ├── llc_occupancy		\  These files are finer grained
> │   ├── mbm_local_bytes		 > data from each SNC node
> │   └── mbm_total_bytes		/
> └── mon_sub_L3_01
>      ├── llc_occupancy		\
>      ├── mbm_local_bytes		 > As above, but for node 1.
>      └── mbm_total_bytes		/
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   include/linux/resctrl.h                   |  2 ++
>   arch/x86/kernel/cpu/resctrl/internal.h    | 21 +++++++++++++++++++++
>   arch/x86/kernel/cpu/resctrl/core.c        |  7 ++++++-
>   arch/x86/kernel/cpu/resctrl/pseudo_lock.c |  1 -
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c    |  1 -
>   5 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 64b6ad1b22a1..d733e1f6485d 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -96,6 +96,7 @@ struct rdt_ctrl_domain {
>   /**
>    * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
>    * @hdr:		common header for different domain types
> + * @ci:			cache info for this domain
>    * @rmid_busy_llc:	bitmap of which limbo RMIDs are above threshold
>    * @mbm_total:		saved state for MBM total bandwidth
>    * @mbm_local:		saved state for MBM local bandwidth
> @@ -106,6 +107,7 @@ struct rdt_ctrl_domain {
>    */
>   struct rdt_mon_domain {
>   	struct rdt_domain_hdr		hdr;
> +	struct cacheinfo		*ci;
>   	unsigned long			*rmid_busy_llc;
>   	struct mbm_state		*mbm_total;
>   	struct mbm_state		*mbm_local;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 135190e0711c..eb70d3136ced 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -2,6 +2,7 @@
>   #ifndef _ASM_X86_RESCTRL_INTERNAL_H
>   #define _ASM_X86_RESCTRL_INTERNAL_H
>   
> +#include <linux/cacheinfo.h>
>   #include <linux/resctrl.h>
>   #include <linux/sched.h>
>   #include <linux/kernfs.h>
> @@ -509,6 +510,26 @@ static inline bool resctrl_arch_get_cdp_enabled(enum resctrl_res_level l)
>   
>   int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable);
>   
> +/*
> + * Get the cacheinfo structure of the cache associated with @cpu at level @level.
> + * cpuhp lock must be held.
> + */
> +static inline struct cacheinfo *get_cpu_cacheinfo_level(int cpu, int level)
> +{
> +	struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> +	int i;
> +
> +	for (i = 0; i < ci->num_leaves; i++) {
> +		if (ci->info_list[i].level == level) {
> +			if (ci->info_list[i].attributes & CACHE_ID)
> +				return &ci->info_list[i];
> +			break;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +

This does not belong in resctrl. It really looks to partner well with existing
cache helpers in include/linux/cacheinfo.h that already contains get_cpu_cacheinfo_id().
Considering the existing naming get_cpu_cacheinfo() may be more appropriate.

Reinette