Re: [PATCH] tracing: Have trace_marker writes be just half of TRACE_SEQ_SIZE

[Mathieu Desnoyers]

On 2024-03-04 20:35, Steven Rostedt wrote:
> On Mon, 4 Mar 2024 20:15:57 -0500
> Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>
> > On 2024-03-04 19:27, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
> > >
> > > Since the size of trace_seq's buffer is the max an event can output, have
> > > the trace_marker be half of the entire TRACE_SEQ_SIZE, which is 4K. That
> > > will keep writes that has meta data written from being dropped (but
> > > reported), because the total output of the print event is greater than
> > > what the trace_seq can hold.
> >
> > Defining the trace_mark limit in terms of "TRACE_SEQ_SIZE / 2"
> > seems backwards. It's basically using a define of the maximum
> > buffer size for the pretty-printing output and defining the maximum
> > input size of a system call to half of that.
> >
> > I'd rather see, in a header file shared between tracing mark
> > write implementation and output implementation:
> >
> > #define TRACING_MARK_MAX_SIZE	4096
> >
> > and then a static validation that this input fits within your
> > pretty printing output in the output implementation file:
> >
> > BUILD_BUG_ON(TRACING_MARK_MAX_SIZE + sizeof(meta data stuff...) > TRACE_SEQ_SIZE);
>
> That's not the meta size I'm worried about. The sizeof(meta data) is the
> raw event binary data, which is not related to the size of the event output.
>
>   # cd /sys/kernel/tracing
>   # echo hello > trace_marker
>   # cat trace
> [..]
>             <...>-999     [001] .....  2296.140373: tracing_mark_write: hello
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     This is the meta data that is added to trace_seq

If this header has a known well-defined upper-limit length, then use
that in the BUILD_BUG_ON().

Thanks,

Mathieu

> -- Steve
> --
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com