Re: [RFC][PATCH 0/5] tracing/events: stable tracepoints

Mathieu Desnoyers
2010-11-17T20:14:47Z

* Steven Rostedt (rostedt@goodmis.org) wrote:
> We also have name (redundant), ID (should be agnostic), and print_fmt
> (lots of issues).
>
> So the new format looks like this:
>
> [root@bxf ~]# cat /sys/kernel/event/sched_switch/format
> array:prev_comm type:char size:8 count:16 align:1 signed:1;
> field:prev_pid type:pid_t size:32 align:4 signed:1;
> field:prev_state type:char size:8 align:1 signed:1;
> array:next_comm type:char size:8 count:16 align:1 signed:1;
> field:next_pid type:pid_t size:32 align:4 signed:1;

Hrm, this is mixing field and type definitions. How about we organize this in
something that will be both parseable and extensible ?

First, I don't see what exporting the kernel-internal type "pid_t" in there
gives you. Userspace knows nothing about this, so it seems pretty useless.

What do you think of this alternative layout ?

Named types below:

% cat /sys/kernel/event/types/char
parent = integer;
size = 8;
signed = true;
align = 8;

% cat /sys/kernel/event/types/pid_t
parent = integer;
size = 32;
signed = true;
align = 32; /* Or 8 if the architecture supports unaligned writes
efficiently */

% cat /sys/kernel/event/sched_switch/format
type { /* Nameless type */
parent = struct;
fields = {
{
type { /* Nameless type */
parent = array;
length = 16;
elem_type = char; /* refers to named type */
},
prev_comm,
},
{ pid_t, prev_pid, },
{ char, prev_state, },
{
type { /* Nameless type */
parent = array;
length = 16;
elem_type = char; /* refers to named type */
},
next_comm,
},
{ pid_t, next_pid, },
};
}

With this layout, we can declare types like enumerations, e.g.

% cat /sys/kernel/event/types/trap_id_t
type {
parent = enum;
size = 5; /* 5-bit bitfield to hold the enumeration */
signed = false;
align = 1; /* bit-packed */
map = {
{ 0, "divide error" },
{ 2, "nmi stack" },
{ 4, "overflow" },
....
};
}

So we can refer to this "named type" in all events for which we want to save
trap ID ? We therefore get the mapping to a human-understandable name for free.

> Some notes:
>
> o The size is in bits.

Yep, this will immensely help when dealing with bitfields.

> o We added an align, that is the natural alignment for the arch of that
> type.

Just watch out, in your initial example, I think your align field is in bytes
rather than bits. Ideally we'd like everything to be consistent.

Thanks,

Mathieu

> o We added an "array" type, that specifies the size of an element as
> well as a "count", where total size can be align(size) * count.
> o We separated the field name from the type.
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.