Re: [PATCH v3 05/10] selftests/resctrl: Support multiple events associated with iMC

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7402 bytes --]

On Fri, 13 Mar 2026, Reinette Chatre wrote:

> The resctrl selftests discover needed parameters to perf_event_open() via
> sysfs. The PMU associated with every memory controller (iMC) is discovered
> via the /sys/bus/event_source/devices/uncore_imc_N/type file while
> the read memory bandwidth event type and umask is discovered via
> /sys/bus/event_source/devices/uncore_imc_N/events/cas_count_read.
> 
> Newer systems may have multiple events that expose read memory bandwidth.
> Running a recent kernel that includes
> commit 6a8a48644c4b ("perf/x86/intel/uncore: Add per-scheduler IMC CAS count events")
> on these systems expose the multiple events. For example,
>  /sys/bus/event_source/devices/uncore_imc_N/events/cas_count_read_sch0
>  /sys/bus/event_source/devices/uncore_imc_N/events/cas_count_read_sch1
> 
> Support parsing of iMC PMU properties when the PMU may have multiple events
> to measure read memory bandwidth. The PMU only needs to be discovered once.
> Split the parsing of event details from actual PMU discovery in order to
> loop over all events associated with the PMU. Match all events with the
> cas_count_read prefix instead of requiring there to be one file with that
> name.
> 
> Make the parsing code more robust. With strings passed around to create
> needed paths, use snprintf() instead of sprintf() to ensure there is
> always enough space to create the path while using the standard PATH_MAX
> for path lengths. Ensure there is enough room in imc_counters_config[]
> before attempting to add an entry.
> 
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> Tested-by: Chen Yu <yu.c.chen@intel.com>
> Reviewed-by: Zide Chen <zide.chen@intel.com>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> ---
> Changes since v1:
> - Add Zide Chen's RB tag.
> 
> Changes since v2:
> - Update changelog to note merged perf change that supports this change.
> - Use PATH_MAX instead of magic number for path lengths. (Ilpo)
> - Rename "org_count" -> "orig_count". (Ilpo)
> - Rework flow surrounding fscanf() used in both parse_imc_read_bw_events()
>   and read_from_imc_dir(). (Ilpo)
> - Handle error first to reduce indentation. (Ilpo)
> - Add Chen Yu's tag.
> ---
>  tools/testing/selftests/resctrl/resctrl_val.c | 118 ++++++++++++++----
>  1 file changed, 93 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index 6d766347e3fc..f20d2194c35f 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -11,10 +11,10 @@
>  #include "resctrl.h"
>  
>  #define UNCORE_IMC		"uncore_imc"
> -#define READ_FILE_NAME		"events/cas_count_read"
> +#define READ_FILE_NAME		"cas_count_read"
>  #define DYN_PMU_PATH		"/sys/bus/event_source/devices"
>  #define SCALE			0.00006103515625
> -#define MAX_IMCS		20
> +#define MAX_IMCS		40
>  #define MAX_TOKENS		5
>  
>  #define CON_MBM_LOCAL_BYTES_PATH		\
> @@ -109,46 +109,114 @@ static int open_perf_read_event(int i, int cpu_no)
>  	return 0;
>  }
>  
> -/* Get type and config of an iMC counter's read event. */
> -static int read_from_imc_dir(char *imc_dir, unsigned int *count)
> +static int parse_imc_read_bw_events(char *imc_dir, unsigned int type,
> +				    unsigned int *count)
>  {
> -	char cas_count_cfg[1024], imc_counter_cfg[1024], imc_counter_type[1024];
> +	char imc_events_dir[PATH_MAX], imc_counter_cfg[PATH_MAX];
> +	unsigned int orig_count = *count;
> +	char cas_count_cfg[1024];
> +	struct dirent *ep;
> +	int path_len;
> +	int ret = -1;
> +	int num_cfg;
>  	FILE *fp;
> +	DIR *dp;
>  
> -	/* Get type of iMC counter */
> -	sprintf(imc_counter_type, "%s%s", imc_dir, "type");
> -	fp = fopen(imc_counter_type, "r");
> -	if (!fp) {
> -		ksft_perror("Failed to open iMC counter type file");
> +	path_len = snprintf(imc_events_dir, sizeof(imc_events_dir), "%sevents",
> +			    imc_dir);
> +	if (path_len >= sizeof(imc_events_dir)) {
> +		ksft_print_msg("Unable to create path to %sevents\n", imc_dir);
> +		return -1;
> +	}
>  
> +	dp = opendir(imc_events_dir);
> +	if (!dp) {
> +		ksft_perror("Unable to open PMU events directory");
>  		return -1;
>  	}
> -	if (fscanf(fp, "%u", &imc_counters_config[*count].type) <= 0) {
> -		ksft_perror("Could not get iMC type");
> +
> +	while ((ep = readdir(dp))) {
> +		/*
> +		 * Parse all event files with READ_FILE_NAME prefix that
> +		 * contain the event number and umask. Skip files containing
> +		 * "." that contain unused properties of event.
> +		 */
> +		if (!strstr(ep->d_name, READ_FILE_NAME) ||
> +		    strchr(ep->d_name, '.'))
> +			continue;
> +
> +		path_len = snprintf(imc_counter_cfg, sizeof(imc_counter_cfg),
> +				    "%s/%s", imc_events_dir, ep->d_name);
> +		if (path_len >= sizeof(imc_counter_cfg)) {
> +			ksft_print_msg("Unable to create path to %s/%s\n",
> +				       imc_events_dir, ep->d_name);
> +			goto out_close;
> +		}
> +		fp = fopen(imc_counter_cfg, "r");
> +		if (!fp) {
> +			ksft_perror("Failed to open iMC config file");
> +			goto out_close;
> +		}
> +		num_cfg = fscanf(fp, "%1023s", cas_count_cfg);
>  		fclose(fp);
> +		if (num_cfg <= 0) {
> +			ksft_perror("Could not get iMC cas count read");
> +			goto out_close;
> +		}
> +		if (*count >= MAX_IMCS) {
> +			ksft_print_msg("Maximum iMC count exceeded\n");
> +			goto out_close;
> +		}
>  
> -		return -1;
> +		imc_counters_config[*count].type = type;
> +		get_read_event_and_umask(cas_count_cfg, *count);
> +		/* Do not fail after incrementing *count. */
> +		*count += 1;
>  	}
> -	fclose(fp);
> +	if (*count == orig_count) {
> +		ksft_print_msg("Unable to find events in %s\n", imc_events_dir);
> +		goto out_close;
> +	}
> +	ret = 0;
> +out_close:
> +	closedir(dp);
> +	return ret;
> +}
>  
> -	/* Get read config */
> -	sprintf(imc_counter_cfg, "%s%s", imc_dir, READ_FILE_NAME);
> -	fp = fopen(imc_counter_cfg, "r");
> -	if (!fp) {
> -		ksft_perror("Failed to open iMC config file");
> +/* Get type and config of an iMC counter's read event. */
> +static int read_from_imc_dir(char *imc_dir, unsigned int *count)
> +{
> +	char imc_counter_type[PATH_MAX];
> +	unsigned int type;
> +	int path_len;
> +	FILE *fp;
> +	int ret;
>  
> +	/* Get type of iMC counter */
> +	path_len = snprintf(imc_counter_type, sizeof(imc_counter_type),
> +			    "%s%s", imc_dir, "type");
> +	if (path_len >= sizeof(imc_counter_type)) {
> +		ksft_print_msg("Unable to create path to %s%s\n",
> +			       imc_dir, "type");
>  		return -1;
>  	}
> -	if (fscanf(fp, "%1023s", cas_count_cfg) <= 0) {
> -		ksft_perror("Could not get iMC cas count read");
> -		fclose(fp);
> +	fp = fopen(imc_counter_type, "r");
> +	if (!fp) {
> +		ksft_perror("Failed to open iMC counter type file");
>  
>  		return -1;
>  	}
> +	ret = fscanf(fp, "%u", &type);
>  	fclose(fp);
> -
> -	get_read_event_and_umask(cas_count_cfg, *count);
> -	*count += 1;
> +	if (ret <= 0) {
> +		ksft_perror("Could not get iMC type");
> +		return -1;
> +	}
> +	ret = parse_imc_read_bw_events(imc_dir, type, count);
> +	if (ret) {
> +		ksft_print_msg("Unable to parse bandwidth event and umask\n");
> +		return ret;
> +	}
>  
>  	return 0;
>  }
>