Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	linux-kselftest@vger.kernel.org, "Shuah Khan" <shuah@kernel.org>,
	"Shaopeng Tan" <tan.shaopeng@jp.fujitsu.com>,
	"Maciej Wieczór-Retman" <maciej.wieczor-retman@intel.com>,
	"Fenghua Yu" <fenghua.yu@intel.com>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 16/24] selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test
Date: Thu, 2 Nov 2023 10:51:27 -0700	[thread overview]
Message-ID: <fe5284f7-c27a-4ec3-b12f-f3556a9bb456@intel.com> (raw)
In-Reply-To: <20231024092634.7122-17-ilpo.jarvinen@linux.intel.com>

Hi Ilpo,

On 10/24/2023 2:26 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 allocated 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.

Thank you very much for doing this.

> 
> 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  | 286 +++++++++-----------
>  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, 137 insertions(+), 204 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> index e71690a9bbb3..7518c520c5cc 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -11,65 +11,68 @@
>  #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 mask to
> + * the test with n bits. 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.

This formula is not clear to me. In the code the formula is always:
MIN_DIFF_PERCENT_PER_BIT * (bits - 1) ... is the "-1" because it always
decrements the number of bits tested by one? So, for example, if testing
5 then 3 bits it would be  MIN_DIFF_PERCENT_PER_BIT * (3 - 2)?
Would above example thus be:
MIN_DIFF_PERCENT_PER_BIT * (4 - (5 - 4)) = 3 ?



> + *
> + * 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.
>   */

...

>  /*
>   * cat_test:	execute CAT benchmark and measure LLC cache misses
>   * @param:	parameters passed to cat_test()
>   * @span:	buffer size for the benchmark
> + * @current_mask	start mask for the first iteration
> + *
> + * Run CAT test, bits are removed one-by-one from the current_mask for each
> + * subsequent test.
>   *

Could this please be expanded to provide more details about how test works,
measurements taken, and how pass/fail is determined?

> - * Return:	0 on success. non-zero on failure.
> + * Return:		0 on success. non-zero on failure.

Is non-zero specific enough? Does that mean that <0 and >0 are failure?

>   */
> -static int cat_test(struct resctrl_val_param *param, size_t span)
> +static int cat_test(struct resctrl_val_param *param, size_t span, unsigned long current_mask)
>  {
> -	int memflush = 1, operation = 0, ret = 0;
>  	char *resctrl_val = param->resctrl_val;
>  	static struct perf_event_read pe_read;
>  	struct perf_event_attr pea;
> +	unsigned char *buf;
> +	char schemata[64];
> +	int ret, i, pe_fd;
>  	pid_t bm_pid;
> -	int pe_fd;
>  
>  	if (strcmp(param->filename, "") == 0)
>  		sprintf(param->filename, "stdio");
> @@ -143,54 +168,64 @@ static int cat_test(struct resctrl_val_param *param, size_t span)
>  	if (ret)
>  		return ret;
>  
> +	buf = alloc_buffer(span, 1);
> +	if (buf == NULL)
> +		return -1;
> +
>  	perf_event_attr_initialize(&pea, PERF_COUNT_HW_CACHE_MISSES);
>  	perf_event_initialize_read_format(&pe_read);
>  
> -	/* Test runs until the callback setup() tells the test to stop. */
> -	while (1) {
> -		ret = param->setup(param);
> -		if (ret == END_OF_TESTS) {
> -			ret = 0;
> -			break;
> -		}
> -		if (ret < 0)
> -			break;
> -		pe_fd = perf_event_reset_enable(&pea, bm_pid, param->cpu_no);
> -		if (pe_fd < 0) {
> -			ret = -1;
> -			break;
> -		}
> +	while (current_mask) {
> +		snprintf(schemata, sizeof(schemata), "%lx", param->mask & ~current_mask);
> +		ret = write_schemata("", schemata, param->cpu_no, param->resctrl_val);
> +		if (ret)
> +			goto free_buf;
> +		snprintf(schemata, sizeof(schemata), "%lx", current_mask);
> +		ret = write_schemata(param->ctrlgrp, schemata, param->cpu_no, param->resctrl_val);
> +		if (ret)
> +			goto free_buf;
> +
> +		for (i = 0; i < NUM_OF_RUNS; i++) {
> +			mem_flush(buf, span);
> +			ret = fill_cache_read(buf, span, true);
> +			if (ret)
> +				goto free_buf;
> +
> +			pe_fd = perf_event_reset_enable(&pea, bm_pid, param->cpu_no);
> +			if (pe_fd < 0) {
> +				ret = -1;
> +				goto free_buf;
> +			}

It seems to me that the perf counters are reconfigured at every iteration.
Can it not just be configured once and then the counters just reset and
enabled at each iteration? I'd expect this additional work to impact
values measured.

>  
> -		if (run_fill_buf(span, memflush, operation, true)) {
> -			fprintf(stderr, "Error-running fill buffer\n");
> -			ret = -1;
> -			goto pe_close;
> -		}
> +			fill_cache_read(buf, span, true);
>  
> -		sleep(1);
> -		ret = perf_event_measure(pe_fd, &pe_read, param, bm_pid);
> -		if (ret)
> -			goto pe_close;
> +			ret = perf_event_measure(pe_fd, &pe_read, param, bm_pid);
> +			if (ret)
> +				goto pe_close;
>  
> -		close(pe_fd);
> +			close(pe_fd);
> +		}
> +		current_mask = next_mask(current_mask);
>  	}
>  
> +free_buf:
> +	free(buf);
> +
>  	return ret;
>  
>  pe_close:
>  	close(pe_fd);
> -	return ret;
> +	goto free_buf;
>  }
>  

Reinette

  parent reply	other threads:[~2023-11-02 17:51 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-24  9:26 [PATCH 00/24] selftests/resctrl: CAT test improvements & generalized test framework Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 01/24] selftests/resctrl: Split fill_buf to allow tests finer-grained control Ilpo Järvinen
2023-10-27 11:24   ` Maciej Wieczór-Retman
2023-11-02 17:46   ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 02/24] selftests/resctrl: Refactor fill_buf functions Ilpo Järvinen
2023-10-27 11:32   ` Maciej Wieczór-Retman
2023-10-27 11:41     ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 03/24] selftests/resctrl: Refactor get_cbm_mask() Ilpo Järvinen
2023-10-27 11:39   ` Maciej Wieczór-Retman
2023-11-02 17:47   ` Reinette Chatre
2023-11-03 12:09     ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 04/24] selftests/resctrl: Mark get_cache_size() cache_type const Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 05/24] selftests/resctrl: Create cache_size() helper Ilpo Järvinen
2023-11-02 17:47   ` Reinette Chatre
2023-11-03  8:53     ` Ilpo Järvinen
2023-11-03 22:49       ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 06/24] selftests/resctrl: Exclude shareable bits from schemata in CAT test Ilpo Järvinen
2023-11-02 17:48   ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 07/24] selftests/resctrl: Split measure_cache_vals() function Ilpo Järvinen
2023-11-02 17:48   ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 08/24] selftests/resctrl: Split show_cache_info() to test specific and generic parts Ilpo Järvinen
2023-11-02 17:48   ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 09/24] selftests/resctrl: Remove unnecessary __u64 -> unsigned long conversion Ilpo Järvinen
2023-11-02 17:48   ` Reinette Chatre
2023-11-03  9:19     ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 10/24] selftests/resctrl: Remove nested calls in perf event handling Ilpo Järvinen
2023-11-02 17:49   ` Reinette Chatre
2023-11-03  9:45     ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 11/24] selftests/resctrl: Consolidate naming of perf event related things Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 12/24] selftests/resctrl: Improve perf init Ilpo Järvinen
2023-10-27 11:45   ` Maciej Wieczór-Retman
2023-10-24  9:26 ` [PATCH 13/24] selftests/resctrl: Convert perf related globals to locals Ilpo Järvinen
2023-10-27 11:47   ` Maciej Wieczór-Retman
2023-10-24  9:26 ` [PATCH 14/24] selftests/resctrl: Move cat_val() to cat_test.c and rename to cat_test() Ilpo Järvinen
2023-10-27 11:51   ` Maciej Wieczór-Retman
2023-10-27 12:18     ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 15/24] selftests/resctrl: Read in less obvious order to defeat prefetch optimizations Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 16/24] selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test Ilpo Järvinen
2023-10-27 12:05   ` Maciej Wieczór-Retman
     [not found]     ` <fb5e1a50-ba7-1ee8-8bf2-bb8b64b27b1@linux.intel.com>
2023-10-31  7:24       ` Maciej Wieczór-Retman
2023-11-02 17:51   ` Reinette Chatre [this message]
2023-11-03 10:57     ` Ilpo Järvinen
2023-11-03 22:50       ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 17/24] selftests/resctrl: Create struct for input parameter Ilpo Järvinen
2023-10-27 12:07   ` Maciej Wieczór-Retman
2023-11-02 17:51   ` Reinette Chatre
2023-11-03 11:24     ` Ilpo Järvinen
2023-11-03 22:50       ` Reinette Chatre
2023-11-06  9:06         ` Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 18/24] selftests/resctrl: Introduce generalized test framework Ilpo Järvinen
2023-11-02 17:52   ` Reinette Chatre
2023-11-03  9:54     ` Ilpo Järvinen
2023-11-03 22:50       ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 19/24] selftests/resctrl: Pass write_schemata() resource instead of test name Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 20/24] selftests/resctrl: Add helper to convert L2/3 to integer Ilpo Järvinen
2023-10-27 12:09   ` Maciej Wieczór-Retman
2023-10-24  9:26 ` [PATCH 21/24] selftests/resctrl: Get resource id from cache id Ilpo Järvinen
     [not found]   ` <cb2ctfignowlom7lb2t5zhdgtm4s2jlzlvtumlnvxecwwtjk34@ysgepmgkv6bb>
     [not found]     ` <ab4c6aa5-ea49-363a-ff7b-2215665f185d@linux.intel.com>
2023-10-31  7:58       ` Maciej Wieczór-Retman
2023-10-24  9:26 ` [PATCH 22/24] selftests/resctrl: Add test groups and name L3 CAT test L3_CAT Ilpo Järvinen
2023-10-24  9:26 ` [PATCH 23/24] selftests/resctrl: Add L2 CAT test Ilpo Järvinen
2023-11-02 17:57   ` Reinette Chatre
2023-11-03 10:39     ` Ilpo Järvinen
2023-11-03 22:53       ` Reinette Chatre
2023-11-06  9:53         ` Ilpo Järvinen
2023-11-06 17:03           ` Reinette Chatre
2023-11-06 21:22             ` Reinette Chatre
2023-11-07  9:33               ` Ilpo Järvinen
2023-11-08 16:31                 ` Reinette Chatre
2023-10-24  9:26 ` [PATCH 24/24] selftests/resctrl: Ignore failures from L2 CAT test with <= 2 bits Ilpo Järvinen
2023-11-02 17:57   ` Reinette Chatre
2023-11-03 10:24     ` Ilpo Järvinen
2023-11-03 22:53       ` Reinette Chatre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fe5284f7-c27a-4ec3-b12f-f3556a9bb456@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=shuah@kernel.org \
    --cc=tan.shaopeng@jp.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox