On Tue, Dec 23, 2025 at 01:34:52PM -0500, Steven Rostedt wrote: > On Mon, 22 Dec 2025 22:56:22 -0500 > Aaron Tomlin <[email protected]> wrote: > > > Add support for displaying bitmasks in human-readable list format (e.g., > > 0,2-5,7) in addition to the default hexadecimal bitmap representation. > > This is particularly useful when tracing CPU masks and other large > > bitmasks where individual bit positions are more meaningful than their > > hexadecimal encoding. > > > > When the "bitmask-list" option is enabled, the printk "%*pbl" format > > specifier is used to render bitmasks as comma-separated ranges, making > > trace output easier to interpret for complex CPU configurations and > > large bitmask values. > > Hmm, I have a couple of issues with this change. One, this is global. It > affects all instances. The other is that if this is going to be done, then > instead of adding a parameter to trace_seq_bitmask(), another trace_seq_* > API should be created. Perhaps trace_seq_bitmask_cnt()? And have > trace_print_bitmask_seq() call them separately. > > I'm still not convinced that this is needed. What examples do you see? > Should it be only for CPU bitmasks? > > I think a bit more thought needs to be made on a change like this. There's > other options that were added that I now regret. I don't want to add > another one I wish we didn't have. >
Hi Steven,
Regarding the scope, I take your point that a global flag is perhaps too
blunt an instrument. I can certainly refactor this to be instance-aware by
passing the trace_iterator through the call chain.
As for the use case, I find this particularly invaluable when debugging IPI
latency or affinity issues on high-core-count systems. I typically run this
with the "nop" tracer and enable only the events/ipi/ipi_send_cpumask/
event.
When dealing with 128+ logical cores, interpreting a raw hexadecimal bitmap
to identify targeted CPUs is taxing and prone to error. For example, if I
am investigating why CPU 6 is being interrupted, I might use a filter like
"cpumask & CPU{6}". Seeing the resulting output as a range list (e.g., 0-7)
rather than a hexadecimal bitmask allows one to deduce almost instantly
which cluster of CPUs is involved in the IPI broadcast.
To avoid duplication, I am of the opinion that we should maintain a single
trace_seq_bitmask() and simply extend it. However, I am happy to extend the
API if you prefer. Given that struct trace_iterator *iter is available in
the output path, how about the following (not yet fully tested):
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 3690221ba3d8..0a2b8229b999 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -38,7 +38,10 @@ const char *trace_print_symbols_seq_u64(struct trace_seq *p,
*symbol_array);
#endif
-const char *trace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
+struct trace_iterator;
+struct trace_event;
+
+const char *trace_print_bitmask_seq(struct trace_iterator *iter, void
*bitmask_ptr,
unsigned int bitmask_size);
const char *trace_print_hex_seq(struct trace_seq *p,
@@ -54,9 +57,6 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char
*prefix_str,
int prefix_type, int rowsize, int groupsize,
const void *buf, size_t len, bool ascii);
-struct trace_iterator;
-struct trace_event;
-
int trace_raw_output_prep(struct trace_iterator *iter,
struct trace_event *event);
extern __printf(2, 3)
diff --git a/include/trace/stages/stage3_trace_output.h
b/include/trace/stages/stage3_trace_output.h
index 1e7b0bef95f5..fce85ea2df1c 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -39,7 +39,7 @@
void *__bitmask = __get_dynamic_array(field); \
unsigned int __bitmask_size; \
__bitmask_size = __get_dynamic_array_len(field); \
- trace_print_bitmask_seq(p, __bitmask, __bitmask_size); \
+ trace_print_bitmask_seq(iter, __bitmask, __bitmask_size);
\
})
#undef __get_cpumask
@@ -51,7 +51,7 @@
void *__bitmask = __get_rel_dynamic_array(field);
\
unsigned int __bitmask_size; \
__bitmask_size = __get_rel_dynamic_array_len(field); \
- trace_print_bitmask_seq(p, __bitmask, __bitmask_size); \
+ trace_print_bitmask_seq(iter, __bitmask, __bitmask_size);
\
})
#undef __get_rel_cpumask
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index cc2d3306bb60..b312d67815fa 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -195,13 +195,18 @@ EXPORT_SYMBOL(trace_print_symbols_seq_u64);
#endif
const char *
-trace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
+trace_print_bitmask_seq(struct trace_iterator *iter, void *bitmask_ptr,
unsigned int bitmask_size)
{
- const char *ret = trace_seq_buffer_ptr(p);
-
- trace_seq_bitmask(p, bitmask_ptr, bitmask_size * 8);
- trace_seq_putc(p, 0);
+ struct trace_seq *s = &iter->seq;
+ const struct trace_array *tr = iter->tr;
+ const char *ret = trace_seq_buffer_ptr(s);
+ bool show_bitmask_list = tr->trace_flags &
+ TRACE_ITER(BITMASK_LIST);
+
+ trace_seq_bitmask(s, bitmask_ptr, bitmask_size * 8,
+ show_bitmask_list);
+ trace_seq_putc(s, 0);
return ret;
}
Kind regards,
--
Aaron Tomlin
signature.asc
Description: PGP signature
