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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2024-03-04 20:35, Steven Rostedt wrote:
> On Mon, 4 Mar 2024 20:15:57 -0500
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> On 2024-03-04 19:27, Steven Rostedt wrote:
>>> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>>>
>>> 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