[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <du3t5j4fg6nvftxjscyoxi4coan4cf6b7pvfnikmjkhdk5kusr@4xtzzrkjhmzh>
Date: Tue, 23 Dec 2025 19:41:59 -0500
From: Aaron Tomlin <atomlin@...mlin.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: mhiramat@...nel.org, mark.rutland@....com,
mathieu.desnoyers@...icios.com, corbet@....net, sean@...e.io, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH] tracing: Add bitmask-list option for human-readable
bitmask display
On Tue, Dec 23, 2025 at 01:34:52PM -0500, Steven Rostedt wrote:
> On Mon, 22 Dec 2025 22:56:22 -0500
> Aaron Tomlin <atomlin@...mlin.com> 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
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists