Added linked list based management for IMC counter configurations,
allowing the system to dynamically allocate and clean up resources based on
actual hardware capabilities.

Signed-off-by: Yifan Wu <wuyifan50@huawei.com>
---
 tools/testing/selftests/resctrl/resctrl.h     |  1 +
 tools/testing/selftests/resctrl/resctrl_val.c | 25 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 175101022bf3..29c9f76132f0 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -24,6 +24,7 @@
 #include <linux/perf_event.h>
 #include <linux/compiler.h>
 #include <linux/bits.h>
+#include  <linux/list.h>
 #include "kselftest.h"
 
 #define MB			(1024 * 1024)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index f20d2194c35f..ac58d3862281 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -28,6 +28,7 @@ struct membw_read_format {
 };
 
 struct imc_counter_config {
+	struct list_head imc_list;
 	__u32 type;
 	__u64 event;
 	__u64 umask;
@@ -38,6 +39,7 @@ struct imc_counter_config {
 static char mbm_total_path[1024];
 static int imcs;
 static struct imc_counter_config imc_counters_config[MAX_IMCS];
+LIST_HEAD(imc_counters_configs);
 static const struct resctrl_test *current_test;
 
 static void read_mem_bw_initialize_perf_event_attr(int i)
@@ -235,6 +237,7 @@ static int read_from_imc_dir(char *imc_dir, unsigned int *count)
  */
 static int num_of_imcs(void)
 {
+	struct imc_counter_config *imc_counters_config;
 	char imc_dir[512], *temp;
 	unsigned int count = 0;
 	struct dirent *ep;
@@ -263,14 +266,23 @@ static int num_of_imcs(void)
 			 * first character is a numerical digit or not.
 			 */
 			if (temp[0] >= '0' && temp[0] <= '9') {
+				imc_counters_config = malloc(sizeof(struct imc_counter_config));
+				if (!imc_counters_config) {
+					ksft_print_msg("Unable to allocate memory for iMC counters\n");
+
+					return -1;
+				}
+				memset(imc_counters_config, 0, sizeof(struct imc_counter_config));
 				sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
 					ep->d_name);
 				ret = read_from_imc_dir(imc_dir, &count);
 				if (ret) {
+					free(imc_counters_config);
 					closedir(dp);
 
 					return ret;
 				}
+				list_add(&imc_counters_config->imc_list, &imc_counters_configs);
 			}
 		}
 		closedir(dp);
@@ -303,6 +315,19 @@ int initialize_read_mem_bw_imc(void)
 	return 0;
 }
 
+void cleanup_read_mem_bw_imc(void)
+{
+	struct imc_counter_config *next_imc_counters_config;
+	struct imc_counter_config *imc_counters_config;
+
+	list_for_each_entry_safe(imc_counters_config, next_imc_counters_config,
+				 &imc_counters_configs, imc_list) {
+		list_del(&imc_counters_config->imc_list);
+		free(imc_counters_config);
+	}
+	INIT_LIST_HEAD(&imc_counters_configs);
+}
+
 static void perf_close_imc_read_mem_bw(void)
 {
 	int mc;
-- 
2.33.0

Hi Yifan,

On 3/24/26 5:50 AM, Yifan Wu wrote:
> Added linked list based management for IMC counter configurations,
> allowing the system to dynamically allocate and clean up resources based on
> actual hardware capabilities.

Thank you very much for taking on this change, this is a good addition.

One note on the customs, even though this is user space code it is the Linux kernel
developers that work with it mostly and to support this the style is expected to
(as much as possible) match kernel coding style. Specifically related to this, please
follow coding and changelog guidance in Documentation/process/coding-style.rst and 
Documentation/process/maintainer-tip.rst.

> 
> Signed-off-by: Yifan Wu <wuyifan50@huawei.com>
> ---
>  tools/testing/selftests/resctrl/resctrl.h     |  1 +
>  tools/testing/selftests/resctrl/resctrl_val.c | 25 +++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index 175101022bf3..29c9f76132f0 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -24,6 +24,7 @@
>  #include <linux/perf_event.h>
>  #include <linux/compiler.h>
>  #include <linux/bits.h>
> +#include  <linux/list.h>

(extra space)

>  #include "kselftest.h"
>  
>  #define MB			(1024 * 1024)
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index f20d2194c35f..ac58d3862281 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -28,6 +28,7 @@ struct membw_read_format {
>  };
>  
>  struct imc_counter_config {
> +	struct list_head imc_list;

To make it obvious that this is an entry/node of a list as opposed to a list
header, please use a name like "entry" or "node" or similar.

>  	__u32 type;
>  	__u64 event;
>  	__u64 umask;
> @@ -38,6 +39,7 @@ struct imc_counter_config {
>  static char mbm_total_path[1024];
>  static int imcs;
>  static struct imc_counter_config imc_counters_config[MAX_IMCS];
> +LIST_HEAD(imc_counters_configs);
>  static const struct resctrl_test *current_test;
>  
>  static void read_mem_bw_initialize_perf_event_attr(int i)
> @@ -235,6 +237,7 @@ static int read_from_imc_dir(char *imc_dir, unsigned int *count)
>   */
>  static int num_of_imcs(void)
>  {
> +	struct imc_counter_config *imc_counters_config;

Please avoid variable shadowing.

>  	char imc_dir[512], *temp;
>  	unsigned int count = 0;
>  	struct dirent *ep;
> @@ -263,14 +266,23 @@ static int num_of_imcs(void)
>  			 * first character is a numerical digit or not.
>  			 */
>  			if (temp[0] >= '0' && temp[0] <= '9') {
> +				imc_counters_config = malloc(sizeof(struct imc_counter_config));

One example of the kernel coding style applied here (see "Allocating memory" in
Documentation/process/coding-style.rst) is to use pointer variable and
not typing out the struct.

Even so, this does not look to be the right place to do this allocation ... (more below)

> +				if (!imc_counters_config) {
> +					ksft_print_msg("Unable to allocate memory for iMC counters\n");
> +
> +					return -1;
> +				}
> +				memset(imc_counters_config, 0, sizeof(struct imc_counter_config));

Please use calloc() instead of the malloc() followed by memset().

>  				sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
>  					ep->d_name);
>  				ret = read_from_imc_dir(imc_dir, &count);

read_from_imc_dir() obtains "count", which is used as index to imc_counters_config[],
as reference because it increments this counter. Doing so enables read_from_imc_dir()
to initialize more than one array element.

Only allocating one element for the list thus does not look right and I would actually
expect this allocation to be closer to the initialization where it is known whether
a new element is needed or not. Having new list element allocation within
parse_imc_read_bw_events() looks more appropriate?

>  				if (ret) {
> +					free(imc_counters_config);
>  					closedir(dp);
>  
>  					return ret;
>  				}
> +				list_add(&imc_counters_config->imc_list, &imc_counters_configs);
>  			}
>  		}
>  		closedir(dp);
> @@ -303,6 +315,19 @@ int initialize_read_mem_bw_imc(void)
>  	return 0;
>  }
>  
> +void cleanup_read_mem_bw_imc(void)
> +{
> +	struct imc_counter_config *next_imc_counters_config;

This could just be "*tmp" to make its usage clear.

> +	struct imc_counter_config *imc_counters_config;

I find the "imc_counters_configs" and "imc_counters_config" names too similar
for comfort and makes the code harder to follow. A short name that is obviously
different from the global list name will make the code much easier to read.
How about "imc_counter"?

> +
> +	list_for_each_entry_safe(imc_counters_config, next_imc_counters_config,
> +				 &imc_counters_configs, imc_list) {
> +		list_del(&imc_counters_config->imc_list);
> +		free(imc_counters_config);
> +	}
> +	INIT_LIST_HEAD(&imc_counters_configs);

Why is INIT_LIST_HEAD() needed?

> +}
> +
>  static void perf_close_imc_read_mem_bw(void)
>  {
>  	int mc;

This patch allocates memory but never frees is. This is done later in patch #3
as a fix but that change would be better as part of this to make it easier to
consider the memory management.

Reinette