Re: [PATCH] perf trace: Add --bitmask-list option and correct cpumask formatting

[Aaron Tomlin]

On Tue, Jul 14, 2026 at 12:29:47PM -0400, Aaron Tomlin wrote:
> Currently, dynamic non-array fields such as 'cpumask_t' are mishandled
> in 'perf trace', causing the raw length and offset descriptors to be
> interpreted and displayed as a literal integer (e.g., 'cpumask: 524320'
> instead of the actual mask data).
> 
> This patch corrects the parsing of dynamic fields that do not have the
> TEP_FIELD_IS_ARRAY flag set. By doing so, it ensures the pointer to the
> raw bits is properly resolved within the payload.
> 
> Furthermore, it improves the display formatting for 'cpumask' types:
>     - By default, the cpumask is now correctly output as a zero-suppressed
>       hexadecimal string, resolving the aforementioned integer display
>       anomaly
> 
>     - A new '--bitmask-list' command-line option has been introduced.
>       When specified, this delegates formatting to 'bitmap_scnprintf()',
>       enabling the cpumask to be rendered as a condensed, human-readable
>       list (e.g., '0,2-5,7')
> 
> Fixes: c5e006cdbd27 ("perf trace: Support tracepoint dynamic char arrays")
> Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
> ---
>  tools/perf/builtin-trace.c | 70 ++++++++++++++++++++++++++++++++------
>  tools/perf/util/evsel.c    | 29 ++++++++++++++++
>  tools/perf/util/evsel.h    |  3 ++
>  3 files changed, 92 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index ba0f8749fc7d..cd75ac4e5169 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -226,6 +226,7 @@ struct trace {
>  	bool			force;
>  	bool			vfs_getname;
>  	bool			force_btf;
> +	bool			bitmask_list;
>  	bool			summary_bpf;
>  	int			trace_pgfaults;
>  	char			*perfconfig_events;
> @@ -3216,7 +3217,7 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct perf_sample *
>  	const struct tep_event *tp_format = evsel__tp_format(evsel);
>  	struct tep_format_field *field = tp_format ? tp_format->format.fields : NULL;
>  	struct syscall_arg_fmt *arg = __evsel__syscall_arg_fmt(evsel);
> -	size_t printed = 0, btf_printed;
> +	size_t printed = 0, btf_printed, saved_printed;
>  	unsigned long val;
>  	u8 bit = 1;
>  	struct syscall_arg syscall_arg = {
> @@ -3238,17 +3239,65 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct perf_sample *
>  		syscall_arg.len = 0;
>  		syscall_arg.fmt = arg;
>  		if (field->flags & TEP_FIELD_IS_ARRAY) {
> -			int offset = field->offset;
> -
> -			if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> -				offset = format_field__intval(field, sample, evsel->needs_swap);
> -				syscall_arg.len = offset >> 16;
> -				offset &= 0xffff;
> -				if (tep_field_is_relative(field->flags))
> -					offset += field->offset + field->size;
> +			void *ptr = format_field__get_raw_data(field, sample,
> +							       evsel->needs_swap,
> +							       &syscall_arg.len);
> +
> +			if (!ptr) {
> +				pr_err("Problem processing %s field, skipping...\n", field->name);
> +				continue;
> +			}
> +			val = (uintptr_t)ptr;
> +		} else if ((field->flags & TEP_FIELD_IS_DYNAMIC) &&
> +			   strstr(field->type, "cpumask")) {
> +			void *ptr = format_field__get_raw_data(field, sample,
> +							       evsel->needs_swap,
> +							       &syscall_arg.len);
> +
> +			if (!ptr) {
> +				pr_err("Problem processing %s field, skipping...\n", field->name);
> +				continue;
>  			}
> +			val = (uintptr_t)ptr;
>  
> -			val = (uintptr_t)(sample->raw_data + offset);
> +			saved_printed = printed;
> +
> +			printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
> +			if (trace->show_arg_names)
> +				printed += scnprintf(bf + printed, size - printed, "%s: ", field->name);
> +
> +			if (trace->bitmask_list) {
> +				if (syscall_arg.len > 0) {
> +					unsigned long *mask = zalloc(BITS_TO_LONGS(syscall_arg.len * 8) * sizeof(unsigned long));
> +					if (!mask) {
> +						pr_err("Problem processing %s field, skipping...\n", field->name);
> +						printed = saved_printed;
> +						continue;
> +					}
> +					memcpy(mask, (void *)val, syscall_arg.len);
> +					printed += bitmap_scnprintf(mask, syscall_arg.len * 8,
> +								    bf + printed, size - printed);
> +					free(mask);
> +				}
> +			} else {
> +				unsigned char *b = (unsigned char *)val;
> +				int i;
> +				bool skip_zero = true;
> +
> +				printed += scnprintf(bf + printed, size - printed, "0x");
> +				/* Print in little-endian order */
> +				for (i = syscall_arg.len - 1; i >= 0; i--) {
> +					if (skip_zero && b[i] == 0 && i > 0)
> +						continue;
> +					if (skip_zero) {
> +						printed += scnprintf(bf + printed, size - printed, "%x", b[i]);
> +						skip_zero = false;
> +					} else {
> +						printed += scnprintf(bf + printed, size - printed, "%02x", b[i]);
> +					}
> +				}

Hi Arnaldo, Namhyung, Peter, Mark,

Please disregard this change.

I overlooked 'evsel->needs_swap' and neglected to account for big-endian
support.

I shall correct this and submit a v2.


Kind regards,
-- 
Aaron Tomlin