Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dc06f2ea-ed1e-60e5-f2d7-ccf7facde25b@intel.com>
Date:   Fri, 11 Aug 2023 10:33:43 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.14.0
Subject: Re: [PATCH v4 7/7] selftests/resctrl: Adjust effective L3 cache size
 when SNC enabled
Content-Language: en-US
To:     Tony Luck <tony.luck@intel.com>, Fenghua Yu <fenghua.yu@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>,
        Babu Moger <babu.moger@amd.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <patches@lists.linux.dev>
References: <20230713163207.219710-1-tony.luck@intel.com>
 <20230722190740.326190-1-tony.luck@intel.com>
 <20230722190740.326190-8-tony.luck@intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230722190740.326190-8-tony.luck@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Qlg1R2FxbVM5UldyNHdzUm42dk9jUU9CV3UyTnBZQ3c3WEZncVcyd1plZXQ4?=
 =?utf-8?B?aGpmY0NTQVZPVUZNMHBRMi8yNTVXblhXY2tTeS94b3pyLytFeUQ0MC9aUTdS?=
 =?utf-8?B?c1V5QXhta3pHZzE1UHBTeFRTTzExVHVTRkgxMjhsbGpSamg3TjdGZVlDcW5C?=
 =?utf-8?B?ZGhqWXJXb1pOZjhOeVZXNldtVWFCeGw4TkdyQjJjd1crd2hvSkpvcmxjYmw1?=
 =?utf-8?B?c2Z3L2xrNjdyTTg2QWd1d2pWUW9pWGtBZ2l1YWkvejJCRFk5eVVBbSt1K3R4?=
 =?utf-8?B?V0paVnJCMExWVU5JeW1UY3JCVFg0UDZVZDh3aDZ5QXZRNWxBZjd5NFVMbVl4?=
 =?utf-8?B?cFRUSE5XZWNOTWZrL3RzSy9RaFpqVUZ6WjFHNEl0b1dZSGxyd3gwaGxWa1VE?=
 =?utf-8?B?SFh6SGxncmlqMmRLeUxoaFpWMm13YTJzWUR5OHUvNVZFSFpmS3lwZnBXNmRO?=
 =?utf-8?B?ck4zbUN4SEo2aGxURnJ4Wm1mSWtYeVIrWUJrZlFBSjFYNjdEa1FQVktNSVdS?=
 =?utf-8?B?SS8zYnVLR3ZWUmIvb0t0NDB3dndOc25nd3hzMDRDV1pDSWJxZFZlSWFUSzdu?=
 =?utf-8?B?ZG1iUVNMZXBoc0pRU2Q1TXlpYk1RaURVSmpTaVlxZ0Y1Vmc5OGhxeWdYWnll?=
 =?utf-8?B?dDVuQzFvVlZFRUFtQlBDL05vWkM2RUxUTnE1YXV1akVEMjBnNXpXWEFpR0dS?=
 =?utf-8?B?c0hhc1lON1VvQUZEN0NRUnFtUVRyRko4VTJnbWpSV0VVanNnQUY3UUFkVGVv?=
 =?utf-8?B?cGxpL3ZIZDZYa3g0b1U0bUxsY1lta2lGb3VwRjFxZnM5aVI3aElKZ2ZzdExI?=
 =?utf-8?B?ZlZzZzZVZ21CSHF3eG8rQVoydWd6bW9UNlpBYW9qZDN6bm53SkhSbVBMQnhp?=
 =?utf-8?B?LzQycW1nM3hVeTJaZm9kQU1kSU4rV3FTc0loWGJpTGdoZXQyUHVubE5kMWpo?=
 =?utf-8?B?SFgwMEN2Z01VZnN1RGRSeXM3bWVLWTN5VllJT1NiaXd4aktnT0dlYlJ0d3RM?=
 =?utf-8?B?ZzBQTGxxc0NkWDE4WWZqV2lnUkJ1ZW1aN2ZORy9ZdlNoaUZBRGNZd29lWmNR?=
 =?utf-8?B?MC9XY0NNR0tnaXZnbEQ0Zjh1czROZ2RqRWtROXREc1VzakxGV2l1d0ZLakZM?=
 =?utf-8?B?bEFwV0JDY29RQTE5QUg1SVE3U0piZUM0Q2NzWWdvWlhReHVXYmdmQnY2Ymo5?=
 =?utf-8?B?K2pQbjQzVFlqVXFNYzExb3pORHFRQlZ6SEZMT1BEYVkvMDNSTDZobUdJemZk?=
 =?utf-8?B?ZGRXM0h6ZTVFL2lXV0l2NW1xaTlzQm5MZ2VxSE5qZGVleXBiNWhCOGd2S1p2?=
 =?utf-8?B?a1BSVllUNzFWSlREYS9BNTJEdzRiOWEwYzJBZHNmaVZDVS9sODgxRmJoYTBl?=
 =?utf-8?B?ZlhZOWdMelhIVGk0bXF5K0gyNlJjbllkMnI1a0NwUVVnblBHTkxvaHREd2Yx?=
 =?utf-8?B?VmMyY05NcWIzdzZpZkNZdGg5bFhHL0xkSTBSUXR2a2xjcndSa3VSa3dPZmto?=
 =?utf-8?B?VUNyNXVCNyszdVJBcjhtcGkvdVY0dWYyY241aGtweHZGcFAxK29SWXNsU29x?=
 =?utf-8?B?WkNXc29qTnlOR0VnS3pyZDdpUmI0NndhTHMvSWd1WXpaWm5iYlNZV3pPQVVz?=
 =?utf-8?B?N0VzR0dhMjVVdkpIOERMenA1S2tJMHVNWFJGU2E5VmRZSGVWNGN5VlZaTTE4?=
 =?utf-8?B?YmZQZk0yc0ZiLy9hWnhsVm1hVGR1M25CMEk2dWdLRXlyL1k0cVBTd3UxL2dr?=
 =?utf-8?B?Qk9XeTMrMEZaTXNJTGk1Mm1HYUNTK2syM3h1ZkhBVkMrckROUVJYdGZjVy9S?=
 =?utf-8?B?anJXbGdGZHFiZWZEUGgvU0N2cEpoYTR1T2c3RzlGNkdCN3dQVktxQjA1b1Mz?=
 =?utf-8?B?aTBTTFEwcTh5TWJnMnY5dm1oa1dmMnVUQ1ZGa3dXVjQ0bEkyS1pFR2NsRXpY?=
 =?utf-8?B?WWwvT0ZzR01hV3E1ckV4cVRNalZNcGVlSlovMCtLRUJZQ0lGaFlwNTdEbGxp?=
 =?utf-8?B?RDM2ODB1YkF3TFc1bTl6NDR4VnpYenpUWWwwQkwrWGs3aDIzY3RBeVh0T1NU?=
 =?utf-8?B?WDJDeGNhTTJoUHV2N2tNaGQ1WENORjFiZlp3M0lxZzdwUW5OdVFadHRhQ1JR?=
 =?utf-8?B?R0tVT3VONEhrMUZmZkxxeGJJYkRPbzVoYzZPTnlTUDVsZFkwZk4weVFaUzc2?=
 =?utf-8?B?QVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b4d728a-cb21-4dd8-57dc-08db9a911d7b
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Aug 2023 17:33:46.0000
 (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: /xFXjzR+g9bg3B4IFgHwCqaHnpZ5Nnihki6KsnYaPiWwP/iBslqMk04CFLXSyp3Dn2SXG24VDiUfLFK563Ha789n7EWFoEwR9z3QnKwuaH0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5615
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Tony,

On 7/22/2023 12:07 PM, Tony Luck wrote:
> Sub-NUMA Cluster divides CPUs sharing an L3 cache into separate NUMA
> nodes. Systems may support splitting into either two or four nodes.
> 
> When SNC mode is enabled the effective amount of L3 cache available
> for allocation is divided by the number of nodes per L3.
> 
> Detect which SNC mode is active by comparing the number of CPUs
> that share a cache with CPU0, with the number of CPUs on node0.
> 
> Reported-by: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@fujitsu.com>
> Closes: https://lore.kernel.org/r/TYAPR01MB6330B9B17686EF426D2C3F308B25A@TYAPR01MB6330.jpnprd01.prod.outlook.com

This does not seem to be the case when looking at
https://lore.kernel.org/all/TYAPR01MB6330A4EB3633B791939EA45E8B39A@TYAPR01MB6330.jpnprd01.prod.outlook.com/

> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  tools/testing/selftests/resctrl/resctrl.h   |  1 +
>  tools/testing/selftests/resctrl/resctrlfs.c | 57 +++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index 87e39456dee0..a8b43210b573 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -13,6 +13,7 @@
>  #include <signal.h>
>  #include <dirent.h>
>  #include <stdbool.h>
> +#include <ctype.h>
>  #include <sys/stat.h>
>  #include <sys/ioctl.h>
>  #include <sys/mount.h>
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index fb00245dee92..79eecbf9f863 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -130,6 +130,61 @@ int get_resource_id(int cpu_no, int *resource_id)
>  	return 0;
>  }
>  
> +/*
> + * Count number of CPUs in a /sys bit map
> + */
> +static int count_sys_bitmap_bits(char *name)
> +{
> +	FILE *fp = fopen(name, "r");
> +	int count = 0, c;
> +
> +	if (!fp)
> +		return 0;
> +
> +	while ((c = fgetc(fp)) != EOF) {
> +		if (!isxdigit(c))
> +			continue;
> +		switch (c) {
> +		case 'f':
> +			count++;
> +		case '7': case 'b': case 'd': case 'e':
> +			count++;
> +		case '3': case '5': case '6': case '9': case 'a': case 'c':
> +			count++;
> +		case '1': case '2': case '4': case '8':
> +			count++;
> +		}
> +	}
> +	fclose(fp);
> +
> +	return count;
> +}
> +
> +/*
> + * Detect SNC by compating #CPUs in node0 with #CPUs sharing LLC with CPU0
> + * Try to get this right, even if a few CPUs are offline so that the number
> + * of CPUs in node0 is not exactly half or a quarter of the CPUs sharing the
> + * LLC of CPU0.
> + */
> +static int snc_ways(void)
> +{
> +	int node_cpus, cache_cpus;
> +
> +	node_cpus = count_sys_bitmap_bits("/sys/devices/system/node/node0/cpumap");
> +	cache_cpus = count_sys_bitmap_bits("/sys/devices/system/cpu/cpu0/cache/index3/shared_cpu_map");
> +
> +	if (!node_cpus || !cache_cpus) {
> +		fprintf(stderr, "Warning could not determine Sub-NUMA Cluster mode\n");
> +		return 1;
> +	}
> +
> +	if (4 * node_cpus >= cache_cpus)
> +		return 4;
> +	else if (2 * node_cpus >= cache_cpus)
> +		return 2;
> +	return 1;
> +}
> +
>  /*
>   * get_cache_size - Get cache size for a specified CPU
>   * @cpu_no:	CPU number
> @@ -190,6 +245,8 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size)
>  			break;
>  	}
>  
> +	if (cache_num == 3)
> +		*cache_size /= snc_ways();
>  	return 0;
>  }
>  

I am surprised that this small change is sufficient. The resctrl
selftests are definitely not NUMA aware and the CAT and CMT tests
are not taking that into account when picking CPUs to run on. From
what I understand LLC occupancy counters need to be added in this
scenario but I do not see that done either.

Reinette