Re: [PATCH v3 1/2] perf trace: Correct default cpumask formatting to hexadecimal

[Namhyung Kim <namhyung@kernel.org>]

Hello,

On Sat, Jul 18, 2026 at 08:15:09PM -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).
> 
> Correct the parsing of dynamic fields that do not have the
> TEP_FIELD_IS_ARRAY flag set by introducing helper functions
> format_field__get_raw_data() and format_field__get_cpumask().
> Using these helpers, resolve the pointer to the raw bits within the
> payload and format the cpumask as a zero-padded hexadecimal string by default.
> 
> Fixes: c5e006cdbd27 ("perf trace: Support tracepoint dynamic char arrays")
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  tools/perf/builtin-trace.c | 70 +++++++++++++++++++++++++----
>  tools/perf/util/evsel.c    | 90 ++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/evsel.h    |  6 +++
>  3 files changed, 157 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index ba0f8749fc7d..f8b8431f9543 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -3207,6 +3207,21 @@ static void bpf_output__fprintf(struct trace *trace,
>  	++trace->nr_events_printed;
>  }
>  
> +static unsigned char bitmap_byte(const unsigned long *mask, int byte_idx)
> +{
> +	unsigned char b_val = 0;
> +	int bit_in_byte;
> +
> +	for (bit_in_byte = 0; bit_in_byte < 8; bit_in_byte++) {
> +		int b_idx = byte_idx * 8 + bit_in_byte;
> +		int host_w_idx = b_idx / BITS_PER_LONG;
> +		int host_bit_in_word = b_idx % BITS_PER_LONG;

Better to add a blank line.


> +		if (mask[host_w_idx] & (1UL << host_bit_in_word))
> +			b_val |= (1 << bit_in_byte);
> +	}
> +	return b_val;
> +}
> +
>  static size_t trace__fprintf_tp_fields(struct trace *trace, struct perf_sample *sample,
>  				       struct thread *thread, void *augmented_args, int augmented_args_size)
>  {
> @@ -3238,17 +3253,54 @@ 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")) {
> +			unsigned long *mask = format_field__get_cpumask(field, sample,
> +									evsel->needs_swap,
> +									&syscall_arg.len);
> +
> +			if (!mask) {
> +				pr_err("Problem processing %s field, skipping...\n", field->name);
> +				continue;
>  			}
>  
> -			val = (uintptr_t)(sample->raw_data + offset);
> +			printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : "");
> +			if (trace->show_arg_names)
> +				printed += scnprintf(bf + printed, size - printed, "%s: ", field->name);
> +
> +			if (syscall_arg.len == 0) {
> +				printed += scnprintf(bf + printed, size - printed, "0");
> +			} else {
> +				int i;
> +				bool skip_zero = true;
> +
> +				printed += scnprintf(bf + printed, size - printed, "0x");
> +				/* Print bytes from most significant to least significant */
> +				for (i = syscall_arg.len - 1; i >= 0; i--) {
> +					unsigned char b_val = bitmap_byte(mask, i);
> +
> +					if (skip_zero && b_val == 0 && i > 0)
> +						continue;
> +
> +					if (skip_zero) {
> +						printed += scnprintf(bf + printed, size - printed, "%x", b_val);
> +						skip_zero = false;
> +					} else {
> +						printed += scnprintf(bf + printed, size - printed, "%02x", b_val);
> +					}
> +				}
> +			}
> +			free(mask);
> +			continue;
>  		} else
>  			val = format_field__intval(field, sample, evsel->needs_swap);
>  		/*
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ea9fa04429f0..912d77044141 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -16,9 +16,11 @@
>  #include <errno.h>
>  #include <inttypes.h>
>  #include <stdlib.h>
> +#include <string.h>
>  
>  #include <dirent.h>
>  #include <linux/bitops.h>
> +#include <linux/bitmap.h>
>  #include <linux/compiler.h>
>  #include <linux/ctype.h>
>  #include <linux/err.h>
> @@ -3933,6 +3935,94 @@ void *perf_sample__rawptr(struct perf_sample *sample, const char *name)
>  	return sample->raw_data + offset;
>  }
>  
> +void *format_field__get_raw_data(struct tep_format_field *field, struct
> +				 perf_sample *sample, bool needs_swap,
> +				 u16 *len_out)
> +{
> +	int offset = field->offset;
> +	int size = field->size;
> +
> +	if (field->flags & TEP_FIELD_IS_DYNAMIC) {
> +		unsigned int dynamic_data;
> +
> +		if (out_of_bounds(field, field->offset, field->size, sample->raw_size))
> +			return NULL;
> +
> +		dynamic_data = format_field__intval(field, sample, needs_swap);
> +
> +		offset = dynamic_data & 0xffff;
> +		size = (dynamic_data >> 16) & 0xffff;
> +
> +		if (tep_field_is_relative(field->flags))
> +			offset += field->offset + field->size;
> +	}
> +
> +	if (out_of_bounds(field, offset, size, sample->raw_size))
> +		return NULL;
> +
> +	*len_out = size;
> +	return sample->raw_data + offset;
> +}
> +
> +unsigned long *format_field__get_cpumask(struct tep_format_field *field,
> +					 struct perf_sample *sample,
> +					 bool needs_swap, u16 *len_out)
> +{
> +	u16 len;
> +	void *ptr = format_field__get_raw_data(field, sample, needs_swap, &len);
> +	unsigned long *mask;
> +	struct perf_env *env;
> +	bool target_is_64;
> +	int target_word_size;
> +	int nr_words;
> +	int bit_idx;
> +	int nbits;
> +
> +	if (!ptr)
> +		return NULL;
> +
> +	nbits = len * 8;
> +	mask = bitmap_zalloc(nbits ?: 1);
> +	if (!mask)
> +		return NULL;
> +
> +	env = evsel__env(sample->evsel);
> +	target_is_64 = env ? perf_env__kernel_is_64_bit(env) : (sizeof(void *) == 8);
> +	target_word_size = target_is_64 ? 8 : 4;
> +	nr_words = len / target_word_size;
> +
> +	for (bit_idx = 0; bit_idx < nbits; bit_idx++) {
> +		int w_idx = bit_idx / (target_word_size * 8);
> +		int bit_in_word = bit_idx % (target_word_size * 8);
> +		bool set = false;
> +
> +		if (w_idx < nr_words) {

Nit: Can you change it to something like below to reduce indentation?

		if (w_idx >= nr_words)
			break;

Thanks,
Namhyung


> +			if (target_is_64) {
> +				u64 word;
> +				memcpy(&word, (unsigned char *)ptr + w_idx * 8, 8);
> +				if (needs_swap)
> +					word = bswap_64(word);
> +				set = (word & (1ULL << bit_in_word)) != 0;
> +			} else {
> +				u32 word32;
> +				memcpy(&word32, (unsigned char *)ptr + w_idx * 4, 4);
> +				if (needs_swap)
> +					word32 = bswap_32(word32);
> +				set = (word32 & (1U << bit_in_word)) != 0;
> +			}
> +		}
> +
> +		if (set) {
> +			int host_w_idx = bit_idx / BITS_PER_LONG;
> +			int host_bit_in_word = bit_idx % BITS_PER_LONG;
> +			mask[host_w_idx] |= (1UL << host_bit_in_word);
> +		}
> +	}
> +
> +	*len_out = len;
> +	return mask;
> +}
> +
>  u64 format_field__intval(struct tep_format_field *field, struct perf_sample *sample,
>  			 bool needs_swap)
>  {
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 163fc2b6a7ea..02129a022ea3 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -400,6 +400,12 @@ static inline char *perf_sample__strval(struct perf_sample *sample, const char *
>  
>  struct tep_format_field;
>  
> +void *format_field__get_raw_data(struct tep_format_field *field,
> +				 struct perf_sample *sample,
> +				 bool needs_swap, u16 *len_out);
> +unsigned long *format_field__get_cpumask(struct tep_format_field *field,
> +					 struct perf_sample *sample,
> +					 bool needs_swap, u16 *len_out);
>  u64 format_field__intval(struct tep_format_field *field, struct perf_sample *sample, bool needs_swap);
>  
>  #ifdef HAVE_LIBTRACEEVENT
> -- 
> 2.54.0
>