Re: [PATCH v3 3/3] selftests/resctrl: Enable dynamic management of IMC counters via linked list

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Yifan,

On 5/22/26 2:05 AM, Yifan Wu wrote:
> @@ -242,14 +233,13 @@ static int read_from_imc_dir(char *imc_dir, unsigned int *count)
>   * counter's event and umask for the memory read events that will be
>   * measured.
>   *
> - * Enumerate all these details into an array of structures.
> + * Enumerate all these details into a linked list of structures.
>   *
>   * Return: >= 0 on success. < 0 on failure.
>   */
> -static int num_of_imcs(void)
> +static int enumerate_imcs(void)

The first sentence of this function's comments still describes the behavior being
changed in this patch. How about a fixup like below:

diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 129e8d76222a..fe3b3517ae49 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -226,12 +226,10 @@ static int read_from_imc_dir(char *imc_dir)
 }
 
 /*
- * A system can have 'n' number of iMC (Integrated Memory Controller)
- * counters, get that 'n'. Discover the properties of the available
- * counters in support of needed performance measurement via perf.
- * For each iMC counter get it's type and config. Also obtain each
- * counter's event and umask for the memory read events that will be
- * measured.
+ * Discover the properties of the available iMC (Integrated Memory Controller)
+ * counters in support of needed performance measurement via perf. For each iMC
+ * counter get it's type and config. Also obtain each counter's event and umask
+ * for the memory read events that will be measured.
  *
  * Enumerate all these details into a linked list of structures.
  *

...  
>  int initialize_read_mem_bw_imc(void)
>  {
> -	int imc;
> +	struct imc_counter_config *imc_counter;
> +	int ret;
>  
> -	imcs = num_of_imcs();
> -	if (imcs <= 0)
> -		return imcs;
> +	ret = enumerate_imcs();
> +	if (ret < 0)
> +		return ret;
>  
>  	/* Initialize perf_event_attr structures for all iMC's */
> -	for (imc = 0; imc < imcs; imc++)
> -		read_mem_bw_initialize_perf_event_attr(&imc_counters_config[imc]);
> +	list_for_each_entry(imc_counter, &imc_counters_list, entry) {
> +		read_mem_bw_initialize_perf_event_attr(imc_counter);
> +	}

nit: unnecessary braces

>  
>  	return 0;
>  }
> @@ -328,11 +320,11 @@ void cleanup_read_mem_bw_imc(void)
>  
>  static void perf_close_imc_read_mem_bw(void)
>  {
> -	int mc;
> +	struct imc_counter_config *imc_counter;
>  
> -	for (mc = 0; mc < imcs; mc++) {
> -		if (imc_counters_config[mc].fd != -1)
> -			close(imc_counters_config[mc].fd);
> +	list_for_each_entry(imc_counter, &imc_counters_list, entry) {
> +		if (imc_counter->fd != -1)
> +			close(imc_counter->fd);
>  	}
>  }
>  
With fixup applied and nit addressed:
| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette