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 <[email protected]>
Reinette
