2021-10-14 15:58:28

by Steven Rostedt

[permalink] [raw]
Subject: Re: [sched.h] 317419b91e: perf-sanity-tests.Parse_sched_tracepoints_fields.fail

On Thu, 14 Oct 2021 22:40:04 +0800
Yafang Shao <[email protected]> wrote:

> > mount -t tracefs nodev /sys/kernel/tracing
> > cat /sys/kernel/tracing/events/sched/sched_switch/format
> >
> > name: sched_switch
> > ID: 314
> > format:
> > [...]
> > field:char prev_comm[16]; offset:8; size:16; signed:1;
> > [...]
> > field:char next_comm[16]; offset:40; size:16; signed:1;
> >
> > Both of those fields expose a fixed-size of 16 bytes.
> >
> > AFAIK Steven's intent was that by parsing this file, trace viewers could adapt to
> > changes in the event field layout. Unfortunately, there have been cases where
> > trace viewers had a hard expectation on the field layout. Hopefully those have
> > all been fixed a while ago.
> >
>
> I don't have a clear idea what will happen to trace viewers if we
> extend task comm.

There shouldn't be any doing a hard coded read of the events. That happened
once with powertop, but they broke when they ran 32 bit userspace on a 64
bit kernel, and they switched to libtraceevent to fix it. Which handles
these updates.

>
> Steven, do you have any suggestions ?

The "Don't break user space" is a "tree in the forest" argument. We break
user space all the time. But if no user space tool is around to hear it,
did it really break? The answer is "no".

-- Steve


2021-10-14 18:16:40

by Yafang Shao

[permalink] [raw]
Subject: Re: [sched.h] 317419b91e: perf-sanity-tests.Parse_sched_tracepoints_fields.fail

On Thu, Oct 14, 2021 at 10:48 PM Steven Rostedt <[email protected]> wrote:
>
> On Thu, 14 Oct 2021 22:40:04 +0800
> Yafang Shao <[email protected]> wrote:
>
> > > mount -t tracefs nodev /sys/kernel/tracing
> > > cat /sys/kernel/tracing/events/sched/sched_switch/format
> > >
> > > name: sched_switch
> > > ID: 314
> > > format:
> > > [...]
> > > field:char prev_comm[16]; offset:8; size:16; signed:1;
> > > [...]
> > > field:char next_comm[16]; offset:40; size:16; signed:1;
> > >
> > > Both of those fields expose a fixed-size of 16 bytes.
> > >
> > > AFAIK Steven's intent was that by parsing this file, trace viewers could adapt to
> > > changes in the event field layout. Unfortunately, there have been cases where
> > > trace viewers had a hard expectation on the field layout. Hopefully those have
> > > all been fixed a while ago.
> > >
> >
> > I don't have a clear idea what will happen to trace viewers if we
> > extend task comm.
>
> There shouldn't be any doing a hard coded read of the events. That happened
> once with powertop, but they broke when they ran 32 bit userspace on a 64
> bit kernel, and they switched to libtraceevent to fix it. Which handles
> these updates.
>

Thanks for the explanation.

> >
> > Steven, do you have any suggestions ?
>
> The "Don't break user space" is a "tree in the forest" argument. We break
> user space all the time. But if no user space tool is around to hear it,
> did it really break? The answer is "no".
>

Got it!

--
Thanks
Yafang