Received-SPF: pass (google.com: domain of linux-kernel+bounces-91498-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <e34628b6-fdb3-420f-8563-4cf91ad3a04b@efficios.com>
Date: Mon, 4 Mar 2024 20:36:28 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] tracing: Have trace_marker writes be just half of
 TRACE_SEQ_SIZE
Content-Language: en-US
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
 Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
 Masami Hiramatsu <mhiramat@kernel.org>,
 Linus Torvalds <torvalds@linux-foundation.org>,
 Sachin Sant <sachinp@linux.ibm.com>
References: <20240304192710.4c99677c@gandalf.local.home>
 <469d31a7-f358-4547-bb17-0979b3515924@efficios.com>
 <20240304203516.45b7a551@gandalf.local.home>
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
In-Reply-To: <20240304203516.45b7a551@gandalf.local.home>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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