On Wed, 8 Feb 2023 16:54:03 -0800
John Stultz [off-list ref] wrote:

quoted

Let me understand what you're saying...

The commit 3087c61ed2c4 did

-/* Task command name length: */
-#define TASK_COMM_LEN                  16
+/*
+ * Define the task command name length as enum, then it can be visible to
+ * BPF programs.
+ */
+enum {
+       TASK_COMM_LEN = 16,
+};


and that caused:

cat /sys/kernel/debug/tracing/events/task/task_newtask/format

to print
field:char comm[TASK_COMM_LEN];    offset:12;    size:16;    signed:0;

Yes because there's no easy way to automatically convert an enum to a
number. And this has been a macro for a very long time (so it works,
because macros convert to numbers).

And this breaks much more than android. It will break trace-cmd, rasdaemon
and perf (if it's not using BTF). This change very much "Breaks userspace!"
And requires a kernel workaround, not a user space one.

quoted

instead of
field:char comm[16];    offset:12;    size:16;    signed:0;

so the ftrace parsing android tracing tool had to do:

-  if (Match(type_and_name.c_str(), R"(char [a-zA-Z_]+\[[0-9]+\])")) {
+  if (Match(type_and_name.c_str(),
+            R"(char [a-zA-Z_][a-zA-Z_0-9]*\[[a-zA-Z_0-9]+\])")) {

to workaround this change.
Right?  

I believe so.

quoted

And what are you proposing?  

I'm not proposing anything. I was just wanting to understand more
context around this, as it outwardly appears to be a user-breaking
change, and that is usually not done, so I figured it was an issue
worth raising.

If the debug/tracing/*/format output is in the murky not-really-abi
space, that's fine, but I wanted to know if this was understood as
something that may require userland updates or if this was a
unexpected side-effect.

Linus has already said that /sys/kernel/tracing/* is an ABI (fyi, getting
to the tracing directory via debugfs is obsolete).

Usually, when a trace event uses an enum, it can do:

TRACE_DEFINE_ENUM(TASK_COMM_LEN);

But that's a very common define. It would require it updated for every trace
event, or the change needs to be reverted.

Not sure why BTF needs it like this, because it hasn't changed in years.
Can't it just hard code it?

For ftrace to change it, it requires reading the format files at boot up
and replacing the enums with the numbers, which does impact start up.

-- Steve