Re: [PATCH v2 5/6] perf header: Support memory ranges

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Jul 13, 2026 at 07:43:58PM -0500, Thomas Falcon wrote:
> Memory ranges were created to track different types of memory,
> such as persistent, high bandwidth, or CXL-attached, that may
> be present on a system for performance monitoring and resource
> control purposes.
> 
> Memory range data are parsed from the ACPI MRRM table and exposed
> to userspace tools via sysfs [1]. Memory range data is read from:
> 
> 	/sys/firmware/acpi/memory_ranges/rangeX
> 
> With the following attributes:
> 
> 	u64 base;
> 	u64 length;
> 	int node;
> 	u8  local_region_id;
> 	u8  remote_region_id;
> 
> Read memory range data from sysfs if present and save it in
> the header of the perf data file under a new feature bit,
> HEADER_MEMORY_RANGES (35). Memory range data can be viewed with the
> --header or --header-only options of perf-report and perf-script.

Can you please add an example output here?

> 
> [1]: https://lore.kernel.org/lkml/20250505173819.419271-1-tony.luck@intel.com/
> 
> Assisted-by: Sashiko:gemini-3.1-pro-preview
> Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
> ---
> Changes in v2: 
> -- Added check for NULL return of sysfs__mountpoint() when parsing
>    memory ranges in sysfs
> -- increased MAX_MEMORY_RANGES sanity check from 64 to 256 based on 
>    ACPI MRRM table implementation in Linux kernel
> -- added comment for MAX_MEMORY_RANGES to clarify that it is a sanity check
>    for malformed perf.data files
> -- removed a line of code was removed from util/env.h but was added back in
>    v1 due to bad rebase
> ---
[SNIP]
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index e90e541f546b..0669898362d6 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -92,6 +92,7 @@
>  #define MAX_PMU_CAPS		512
>  #define MAX_PMU_MAPPINGS	4096
>  #define MAX_SCHED_DOMAINS	64
> +#define MAX_MEMORY_RANGES	256
>  
>  /*
>   * magic2 = "PERFILE2"
> @@ -1891,6 +1892,130 @@ static int write_cpu_domain_info(struct feat_fd *ff,
>  	return ret;
>  }
>  
> +static int memory_range__read(struct memory_range *range, int idx)
> +{
> +	char path[PATH_MAX], file[PATH_MAX];

I hope we can reduce the number of buffers for the pathnames as it
already has another in the caller.  Ideally it'd be great if it can use
openat() with an FD for the directory.


> +	struct stat st;
> +	int tmp;
> +
> +	scnprintf(path, PATH_MAX, "firmware/acpi/memory_ranges/range%d", idx);
> +	scnprintf(file, PATH_MAX, "%s/%s", sysfs__mountpoint(), path);
> +	if (stat(file, &st))
> +		return -1;

It seems this check is redundant.

> +
> +	scnprintf(file, PATH_MAX, "%s/base", path);
> +	if (sysfs__read_xll(file, (unsigned long long *) &range->base))
> +		return -1;
> +
> +	scnprintf(file, PATH_MAX, "%s/length", path);
> +	if (sysfs__read_xll(file, (unsigned long long *) &range->length))
> +		return -1;
> +
> +	scnprintf(file, PATH_MAX, "%s/node", path);
> +	if (sysfs__read_int(file, (int *) &range->node))
> +		return -1;
> +
> +	scnprintf(file, PATH_MAX, "%s/local_region_id", path);
> +	if (sysfs__read_int(file, &tmp))
> +		return -1;
> +
> +	if (tmp < 0 || tmp > 255)

Probably better to compare with MAX_MEMORY_REGIONS.


> +		return -1;
> +	range->local_region_id = tmp;
> +
> +	scnprintf(file, PATH_MAX, "%s/remote_region_id", path);
> +	if (sysfs__read_int(file, &tmp))
> +		return -1;
> +
> +	if (tmp < 0 || tmp > 255)

Ditto.


> +		return -1;
> +	range->remote_region_id = tmp;
> +
> +	return 0;
> +}
> +
> +static int memory_range__parse(struct memory_range **ranges)
> +{
> +	const char *sysfs = sysfs__mountpoint();
> +	int i, err, nr_memory_ranges = 0;
> +	char path[PATH_MAX];
> +	struct stat st;
> +
> +	if (!sysfs)
> +		return 0;
> +
> +	scnprintf(path, PATH_MAX, "%s/firmware/acpi/memory_ranges", sysfs);
> +	if (stat(path, &st))
> +		return 0;
> +
> +	while (1) {
> +		scnprintf(path, PATH_MAX, "%s/firmware/acpi/memory_ranges/range%d", sysfs, nr_memory_ranges);

This line looks too long.


> +		if (stat(path, &st))
> +			break;
> +
> +		nr_memory_ranges++;
> +	}
> +
> +	if (nr_memory_ranges == 0)
> +		return 0;
> +
> +	*ranges = zalloc(nr_memory_ranges * sizeof(struct memory_range));

I think calloc() is preferred when you multiply the size and the count.


> +	if (!(*ranges))
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr_memory_ranges; i++) {
> +		struct memory_range range;
> +
> +		err = memory_range__read(&range, i);
> +		if (err < 0)
> +			goto out_error;
> +
> +		(*ranges)[i] = range;
> +	}
> +
> +	return nr_memory_ranges;
> +
> +out_error:
> +	zfree(ranges);
> +	return -1;
> +}
> +
[SNIP] 
> +static int process_memory_ranges(struct feat_fd *ff, void *data __maybe_unused)
> +{
> +	struct perf_env *env = &ff->ph->env;
> +	struct memory_range *ranges, *r;
> +	u32 nr_memory_ranges, i;
> +
> +	if (do_read_u32(ff, &nr_memory_ranges))
> +		return -1;
> +
> +	if (!nr_memory_ranges) {
> +		pr_debug("memory ranges not available\n");
> +		return 0;
> +	}
> +
> +	/* According to version 1.1 of the ACPI MRRM table, the maximum
> +	 * number of memory regions can be at most 255. Do a sanity check
> +	 * here to guard against a malformed perf.data file.
> +	 */
> +	if (nr_memory_ranges >= MAX_MEMORY_RANGES) {
> +		pr_err("Invalid memory_ranges: nr_memory_ranges (%u) > %u\n",
> +		       nr_memory_ranges, MAX_MEMORY_RANGES);
> +		return -1;
> +	}
> +
> +	ranges = zalloc(nr_memory_ranges * sizeof(*ranges));

Please use calloc().

Thanks,
Namhyung


> +	if (!ranges)
> +		return -1;
> +
> +	for (i = 0; i < nr_memory_ranges; i++) {
> +		r = &ranges[i];
> +
> +		if (do_read_u64(ff, &r->base))
> +			goto error;
> +		if (do_read_u64(ff, &r->length))
> +			goto error;
> +		if (do_read_u32(ff, (u32 *) &r->node))
> +			goto error;
> +		if (__do_read(ff, &r->local_region_id, sizeof(u8)))
> +			goto error;
> +		if (__do_read(ff, &r->remote_region_id, sizeof(u8)))
> +			goto error;
> +	}
> +
> +	env->memory_ranges = ranges;
> +	env->nr_memory_ranges = nr_memory_ranges;
> +
> +	return 0;
> +error:
> +	zfree(&ranges);
> +	return -1;
> +}
> +
>  #define FEAT_OPR(n, func, __full_only) \
>  	[HEADER_##n] = {					\
>  		.name	    = __stringify(n),			\
> @@ -4265,6 +4459,7 @@ const struct perf_header_feature_ops feat_ops[HEADER_LAST_FEATURE] = {
>  	FEAT_OPR(CPU_DOMAIN_INFO,	cpu_domain_info,	true),
>  	FEAT_OPR(E_MACHINE,	e_machine,	false),
>  	FEAT_OPR(CLN_SIZE,	cln_size,	false),
> +	FEAT_OPR(MEMORY_RANGES,	memory_ranges,	false),
>  };
>  
>  struct header_print_data {
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index 5e03f884b7cc..765037762758 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -56,6 +56,7 @@ enum {
>  	HEADER_CPU_DOMAIN_INFO,
>  	HEADER_E_MACHINE,
>  	HEADER_CLN_SIZE,
> +	HEADER_MEMORY_RANGES,
>  	HEADER_LAST_FEATURE,
>  	HEADER_FEAT_BITS	= 256,
>  };
> -- 
> 2.43.0
>