RE: [PATCH v2 17/24] selftests/resctrl: Replace count_bits with count_consecutive_bits()

[Shaopeng Tan (Fujitsu)]

Hi Ilpo,

> CAT and CMT tests depends on masks being continuous.
> 
> Replace count_bits with more appropriate variant that counts consecutive bits.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
>  tools/testing/selftests/resctrl/cat_test.c  |  6 ++---
> tools/testing/selftests/resctrl/cmt_test.c  |  3 +--
>  tools/testing/selftests/resctrl/resctrl.h   |  1 +
>  tools/testing/selftests/resctrl/resctrlfs.c | 30
> +++++++++++++++++++++
>  4 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/cat_test.c
> b/tools/testing/selftests/resctrl/cat_test.c
> index d3fbd4de9f8a..a1834dd5ad9a 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -78,7 +78,7 @@ static int check_results(struct resctrl_val_param *param)
>  	}
> 
>  	fclose(fp);
> -	no_of_bits = count_bits(param->mask);
> +	no_of_bits = count_consecutive_bits(param->mask, NULL);
> 
>  	return show_cache_info(sum_llc_perf_miss, no_of_bits,
> param->span / 64,
>  			       MAX_DIFF, MAX_DIFF_PERCENT,
> NUM_OF_RUNS, @@ -103,6 +103,7 @@ int cat_perf_miss_val(int cpu_no, int
> n, char *cache_type)
>  	ret = get_cbm_mask(cache_type, &long_mask);
>  	if (ret)
>  		return ret;
> +	count_of_bits = count_consecutive_bits(long_mask, NULL);
> 
>  	/* Get L3/L2 cache size */
>  	ret = get_cache_size(cpu_no, cache_type, &cache_size); @@ -110,9
> +111,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>  		return ret;
>  	ksft_print_msg("Cache size :%lu\n", cache_size);
> 
> -	/* Get max number of bits from default-cabm mask */
> -	count_of_bits = count_bits(long_mask);
> -
>  	if (!n)
>  		n = count_of_bits / 2;
> 
> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
> b/tools/testing/selftests/resctrl/cmt_test.c
> index efe77e0f1d4c..98e7d3accd73 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -84,14 +84,13 @@ int cmt_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
>  	ret = get_cbm_mask("L3", &long_mask);
>  	if (ret)
>  		return ret;
> +	count_of_bits = count_consecutive_bits(long_mask, NULL);
> 
>  	ret = get_cache_size(cpu_no, "L3", &cache_size);
>  	if (ret)
>  		return ret;
>  	ksft_print_msg("Cache size :%lu\n", cache_size);
> 
> -	count_of_bits = count_bits(long_mask);
> -
>  	if (n < 1 || n > count_of_bits) {
>  		ksft_print_msg("Invalid input value for numbr_of_bits n!\n");
>  		ksft_print_msg("Please enter value in range 1 to %d\n",
> count_of_bits); diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index 65425d92684e..aa5dc8b95a06 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -106,6 +106,7 @@ void tests_cleanup(void);  void
> mbm_test_cleanup(void);  int mba_schemata_change(int cpu_no, char
> *bw_report, char **benchmark_cmd);  void mba_test_cleanup(void);
> +unsigned int count_consecutive_bits(unsigned long val, unsigned int
> +*start);
>  int get_cbm_mask(char *cache_type, unsigned long *mask);  int
> get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
> int cache_alloc_size(int cpu_no, char *cache_type, unsigned long alloc_mask,
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index f01ecfa64063..4efaf69c8152 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -10,6 +10,8 @@
>   */
>  #include "resctrl.h"
> 
> +#include <strings.h>
> +
>  static int find_resctrl_mount(char *buffer)  {
>  	FILE *mounts;
> @@ -218,6 +220,34 @@ static int get_bit_mask(char *filename, unsigned long
> *mask)
>  	return 0;
>  }
> 
> +/*
> + * count_consecutive_bits - Returns the longest train of bits in a bit mask
> + * @val		A bit mask
> + * @start	The location of the least-significant bit of the longest train
> + *
> + * Return:	The length of the consecutive bits in the longest train of bits
> + */
> +unsigned int count_consecutive_bits(unsigned long val, unsigned int
> +*start) {
> +	unsigned long last_val;
> +	int count = 0;
> +
> +	while (val) {
> +		last_val = val;
> +		val &= (val >> 1);
> +		count++;
> +	}

There may not be a case that the most-significant bit is 1 at present, 
but if this case exists, it cannot count correctly.

Best regards,
Shaopeng TAN