Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <516f3152-0690-403e-a8eb-c142888a16d5@intel.com>
Date:   Tue, 28 Nov 2023 14:17:54 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 19/26] selftests/resctrl: Rewrite Cache Allocation
 Technology (CAT) test
Content-Language: en-US
To:     =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
        "Shaopeng Tan" <tan.shaopeng@jp.fujitsu.com>,
        =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>,
        Fenghua Yu <fenghua.yu@intel.com>
CC:     <linux-kernel@vger.kernel.org>
References: <20231120111340.7805-1-ilpo.jarvinen@linux.intel.com>
 <20231120111340.7805-20-ilpo.jarvinen@linux.intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231120111340.7805-20-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YWY4ZDR6T2dvaHlGQ1h1b3FJRnFPbHk2RmFPb3pzUWFmVk1pTHhwUzZCSDR5?=
 =?utf-8?B?Z1MxWWdYMTQ4aVlnc1VtTWtqMDZBQUxNczNYajdsbTlJQldoSWFrSlhoU0pl?=
 =?utf-8?B?UjFDMzJXMERRR1Jzemo4ZlQ2MnNlSHJBMVBNZ1hidTR0NGE1QmNwU3BhdVJv?=
 =?utf-8?B?QVNya0E3ZThrek84dSsySGF6U2FHU3dOWURnN29IMlNlWVdRZlBnYTVGZ25W?=
 =?utf-8?B?U1FXUEQ3clQ4VXV5dDNYa1BsNk0rZnE2Y2VBc3Zqb2U2S1l6VU41RWJtZmdF?=
 =?utf-8?B?SEZvNEs5SS9lbndvRnV3WDR3VVMzbEtQRENQdlhRNjZQdXVjZ1U2eldrRnh3?=
 =?utf-8?B?dkRhUnQ0MHY1Wlc0NWQvczlGaEhDVGNvdkRPdEQ4WFVXd2haQjJTeUllRXlM?=
 =?utf-8?B?MlFHaGw0a09JTUdDajh0aURBa0V0b1JhcnkvM0ZHTUhRVGV2WnU2cjV4ck9E?=
 =?utf-8?B?WmVTRFJtOUxyeUZTYm96czJ3R3BJdEs3Rm9pZDRXK3NycFU5VHFHQTJQYlFV?=
 =?utf-8?B?VUMxaFlnSERWb3ZYYW9pODdJWTJ6K1psUTh5N2w3YXJGdEJKUTRaL0dXcWYy?=
 =?utf-8?B?d05mRzJ2OFB1YWRhMmozYUFxZUczdmVlRnBGdDV5TnRyak9tUldMRDQ4ZTY4?=
 =?utf-8?B?YVQ5Z01jYjZKVC9rbmN3d3lrNkFvZUlnbmRNWEl1U0llTHVHZ3FSQnY4c0d4?=
 =?utf-8?B?MzZjeGlPUTZLWkNmOFZUN1pFelU5YUlWRjlWU1NXSlZzWGdzNmpHRjk2WVVa?=
 =?utf-8?B?MGhaQkYwczNHcjExUlZNYXFJV2EwRkFzVndMMW1acTlka3RyMmZvWTdnRlNV?=
 =?utf-8?B?WDArbEtudFVOdHpvQW1MeWF0bTJDYzQvQng5TkxQRU8wMWIvYmNEbTFCUDFF?=
 =?utf-8?B?MmJrdFo2Z2NZWGhsRzU2SVM5Z29yUElWMTVydDJGbEVzUnBVeDgxOGhtMW9I?=
 =?utf-8?B?SE9SaGR2ZThRVGF6ZWc4aHoyYUNlVGg2TklBdWlDQ3ZMQTAreVp1MFRsOVU3?=
 =?utf-8?B?Y0M3YVd0cVJ5M2hHbEszeFpucWdpNDQ4M3FtSk4zQ1BtT1hhdE0xRnMxVmxz?=
 =?utf-8?B?WjlsT2VrQWdwWGQ1YkNRU1dhZmVtQnFzU3lUc1ZoWjJSVExSaEdvcWEzRFlZ?=
 =?utf-8?B?YmltK3FJcW5mZTJPYUE3S2ZQWmFIZFcxWlVSSDV5YzVjNDkzdTUxUEZraHM0?=
 =?utf-8?B?QmZyYzh1YWo5cC9GRHhPQVR3NG1HOTVWWGg5bEVNaC9kZzR2alpQUVFHZlBN?=
 =?utf-8?B?UEJES0cxeUJVNkRjdFYxSVlQUE05ZXFEZnVNUVVOTFo1ditPZHRzVmFiUnU0?=
 =?utf-8?B?dGQybnRPZHVScERBbmdGVzVkNVN3VHI0b1RqL2VSdHVVcU9TMkVKNEhmRVJS?=
 =?utf-8?B?aDJ4QS9DRlhaS2xhTnF3WUs0YThnSXNHamdjYTRvcmY4d3I2ZXZZVlhWSE9P?=
 =?utf-8?B?ckRabTVMR05CL1FESmJuMzI5dFRldHNoSlgwaVp5bzZnbi9obzNkNEJpeVR3?=
 =?utf-8?B?V2pmcW9RSWs5Zng5WkNoaGxBWlhUYWkrWmc0ZXFLdjcxYzFmV3h2VGZha3V3?=
 =?utf-8?B?bk92bXZyYytyL0Jnd0thN0JCY1pUZjFsemM4USt5aU1rdC9qWFYwaWFXbWVH?=
 =?utf-8?B?UVhPdWIyODlaV08zL1JKY29LS0R6NEFOeVNPNGJkeWZZbzlrbzBLbUF4OUpU?=
 =?utf-8?B?cFM3MTlHczdtNzU4NXNDczZTaURjTStOYW95bzdvL2RNL0JRYjIyeXRONXhu?=
 =?utf-8?B?QllUTXJOcm5KR2IwYldBU1EraUc4WGFIUUVmcUx5NmlrRGYwcGErcmMxVlIx?=
 =?utf-8?B?OWpKT0RzOXFBVjVDZXUxRk83NGEwUDRONW92YktTKzJaZW9SZG1IVUcxb01F?=
 =?utf-8?B?SHhpQ3Z6TUhiRlAwUjcxY0ZyY0NrT2U2a2szdlFBU3RhV0FRdEl6QWowV0h2?=
 =?utf-8?B?RW45cmhxRHFRRjJ1OG9zT2FzRVh5T2N4NzYwbEdCRlZXcjRLZURUQWhQZHZ1?=
 =?utf-8?B?cVJBb2RUNUxEUzVJaG1YOW84U05BTjFzc255alp0VHlUNmJGaWJkenNzZjlY?=
 =?utf-8?B?UWdwWUx4QkcvZXMzMUtsVkZKeUtnN093ZXRiVm9KTzY1YTU0UUdpUzJCWG1B?=
 =?utf-8?B?aE9DRlJDdDEvR2J4U3hhNVdXRFhtSUFhc05aMXhBNXRUbWV6akdVMEg5eGdO?=
 =?utf-8?B?cmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: db786357-72a0-4223-59b3-08dbf05fded4
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Nov 2023 22:17:55.4978
 (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: poomZ1NjAPFYU5irBFqQB7W3ho89ky/GoBzBEkVKtYr2Za3PgFxnmGzyPgLaVrXCcQ8zzCNpmshTGcE0663QKgI+AMCIl5pnyT0N9b62LlI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7419
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 28 Nov 2023 14:18:32 -0800 (PST)

Hi Ilpo,

On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
> CAT test spawns two processes into two different control groups with
> exclusive schemata. Both the processes alloc a buffer from memory
> matching their allocated LLC block size and flush the entire buffer out
> of caches. Since the processes are reading through the buffer only once
> during the measurement and initially all the buffer was flushed, the
> test isn't testing CAT.
> 
> Rewrite the CAT test to allocate a buffer sized to half of LLC. Then
> perform a sequence of tests with different LLC alloc sizes starting
> from half of the CBM bits down to 1-bit CBM. Flush the buffer before
> each test and read the buffer twice. Observe the LLC misses on the
> second read through the buffer. As the allocated LLC block gets smaller
> and smaller, the LLC misses will become larger and larger giving a
> strong signal on CAT working properly.
> 
> The new CAT test is using only a single process because it relies on
> measured effect against another run of itself rather than another
> process adding noise. The rest of the system is set to use the CBM bits
> not used by the CAT test to keep the test isolated.
> 
> Replace count_bits() with count_contiguous_bits() to get the first bit
> position in order to be able to calculate masks based on it.
> 
> This change has been tested with a number of systems from different
> generations.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  tools/testing/selftests/resctrl/cat_test.c  | 282 +++++++++-----------
>  tools/testing/selftests/resctrl/fill_buf.c  |   6 +-
>  tools/testing/selftests/resctrl/resctrl.h   |   5 +-
>  tools/testing/selftests/resctrl/resctrlfs.c |  44 +--
>  4 files changed, 138 insertions(+), 199 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index cfda87667b46..4169b17b8f91 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -11,65 +11,69 @@
>  #include "resctrl.h"
>  #include <unistd.h>
>  
> -#define RESULT_FILE_NAME1	"result_cat1"
> -#define RESULT_FILE_NAME2	"result_cat2"
> +#define RESULT_FILE_NAME	"result_cat"
>  #define NUM_OF_RUNS		5
> -#define MAX_DIFF_PERCENT	4
> -#define MAX_DIFF		1000000
>  
>  /*
> - * Change schemata. Write schemata to specified
> - * con_mon grp, mon_grp in resctrl FS.
> - * Run 5 times in order to get average values.
> + * Minimum difference in LLC misses between a test with n+1 bits CBM to the
> + * test with n bits is MIN_DIFF_PERCENT_PER_BIT * (n - 1). With e.g. 5 vs 4
> + * bits in the CBM mask, the minimum difference must be at least
> + * MIN_DIFF_PERCENT_PER_BIT * (4 - 1) = 3 percent.
> + *
> + * The relationship between number of used CBM bits and difference in LLC
> + * misses is not expected to be linear. With a small number of bits, the
> + * margin is smaller than with larger number of bits. For selftest purposes,
> + * however, linear approach is enough because ultimately only pass/fail
> + * decision has to be made and distinction between strong and stronger
> + * signal is irrelevant.
>   */
> -static int cat_setup(struct resctrl_val_param *p)
> -{
> -	char schemata[64];
> -	int ret = 0;
> -
> -	/* Run NUM_OF_RUNS times */
> -	if (p->num_of_runs >= NUM_OF_RUNS)
> -		return END_OF_TESTS;
> -
> -	if (p->num_of_runs == 0) {
> -		sprintf(schemata, "%lx", p->mask);
> -		ret = write_schemata(p->ctrlgrp, schemata, p->cpu_no,
> -				     p->resctrl_val);
> -	}
> -	p->num_of_runs++;
> -
> -	return ret;
> -}
> +#define MIN_DIFF_PERCENT_PER_BIT	1
>  
>  static int show_results_info(__u64 sum_llc_val, int no_of_bits,
> -			     unsigned long cache_span, unsigned long max_diff,
> -			     unsigned long max_diff_percent, unsigned long num_of_runs,
> -			     bool platform)
> +			     unsigned long cache_span, long min_diff_percent,

With all care taken in unsigned use I wonder why min_diff_percent is just long?

> +			     unsigned long num_of_runs, bool platform,
> +			     __s64 *prev_avg_llc_val)
>  {
>  	__u64 avg_llc_val = 0;
> -	float diff_percent;
> -	int ret;
> +	float avg_diff;
> +	int ret = 0;
>  
>  	avg_llc_val = sum_llc_val / num_of_runs;
> -	diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
> +	if (*prev_avg_llc_val) {
> +		float delta = (__s64)(avg_llc_val - *prev_avg_llc_val);
>  
> -	ret = platform && abs((int)diff_percent) > max_diff_percent;
> +		avg_diff = delta / *prev_avg_llc_val;
> +		ret = platform && (avg_diff * 100) < (float)min_diff_percent;
>  
> -	ksft_print_msg("%s Check cache miss rate within %lu%%\n",
> -		       ret ? "Fail:" : "Pass:", max_diff_percent);
> +		ksft_print_msg("%s Check cache miss rate changed more than %.1f%%\n",
> +			       ret ? "Fail:" : "Pass:", (float)min_diff_percent);
>  
> -	ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
> +		ksft_print_msg("Percent diff=%.1f\n", avg_diff * 100);
> +	}
> +	*prev_avg_llc_val = avg_llc_val;
>  
>  	show_cache_info(no_of_bits, avg_llc_val, cache_span, true);
>  
>  	return ret;
>  }
>  
> -static int check_results(struct resctrl_val_param *param, size_t span)
> +/* Remove one bit from the consecutive cbm mask */

The use of "consecutive" is not clear. In this usage it removes one
bit from the previous mask in order to create the consecutive mask, no?

> +static unsigned long next_mask(unsigned long current_mask)
> +{
> +	return current_mask & (current_mask >> 1);
> +}
> +

The new test looks very good to me. Thank you very much for creating it.

It looks to me as though this test impacts the affinity of main program
since it is only one process, changes its affinity, but never change it back.

Reinette