Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 14 Dec 2022 23:54:02 -0500
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Linyu Yuan <quic_linyyuan@quicinc.com>
Cc:     Masami Hiramatsu <mhiramat@kernel.org>,
        <linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] trace: allocate space from temparary trace
 sequence buffer
Message-ID: <20221214235402.5ea7489e@gandalf.local.home>
In-Reply-To: <1671078807-20748-2-git-send-email-quic_linyyuan@quicinc.com>
References: <1671078807-20748-1-git-send-email-quic_linyyuan@quicinc.com>
        <1671078807-20748-2-git-send-email-quic_linyyuan@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 15 Dec 2022 12:33:27 +0800
Linyu Yuan <quic_linyyuan@quicinc.com> wrote:

> there is one dwc3 trace event declare as below,
> DECLARE_EVENT_CLASS(dwc3_log_event,
> 	TP_PROTO(u32 event, struct dwc3 *dwc),
> 	TP_ARGS(event, dwc),
> 	TP_STRUCT__entry(
> 		__field(u32, event)
> 		__field(u32, ep0state)
> 		__dynamic_array(char, str, DWC3_MSG_MAX)
> 	),
> 	TP_fast_assign(
> 		__entry->event = event;
> 		__entry->ep0state = dwc->ep0state;
> 	),
> 	TP_printk("event (%08x): %s", __entry->event,
> 			dwc3_decode_event(__get_str(str), DWC3_MSG_MAX,
> 				__entry->event, __entry->ep0state))
> );
> the problem is when trace function called, it will allocate up to
> DWC3_MSG_MAX bytes from trace event buffer, but never fill the buffer
> during fast assignment, it only fill the buffer when output function are
> called, so this means if output function are not called, the buffer will
> never used.
> 
> add __get_buf(len) which allocate space from iter->tmp_seq when trace
> output function called, it allow user write any data to allocatd space.
> 
> the mentioned dwc3 trace event will changed as below,
> DECLARE_EVENT_CLASS(dwc3_log_event,
> 	TP_PROTO(u32 event, struct dwc3 *dwc),
> 	TP_ARGS(event, dwc),
> 	TP_STRUCT__entry(
> 		__field(u32, event)
> 		__field(u32, ep0state)
> 	),
> 	TP_fast_assign(
> 		__entry->event = event;
> 		__entry->ep0state = dwc->ep0state;
> 	),
> 	TP_printk("event (%08x): %s", __entry->event,
> 		dwc3_decode_event(__get_buf(DWC3_MSG_MAX), DWC3_MSG_MAX,
> 				__entry->event, __entry->ep0state))
> );.
> 
> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>


No!

Here, I did it for you:

-- Steve

diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 48b44b88dc25..81abdcbfc660 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -11,6 +11,7 @@
 #ifndef __DWC3_DEBUG_H
 #define __DWC3_DEBUG_H
 
+#include <linux/trace_seq.h>
 #include "core.h"
 
 /**
@@ -381,6 +382,19 @@ static inline const char *dwc3_decode_event(char *str, size_t size, u32 event,
 		return dwc3_ep_event_string(str, size, &evt.depevt, ep0state);
 }
 
+static inline const char *dwc3_decode_event_seq(struct trace_seq *p,
+						u32 event, u32 ep0state)
+{
+	char *str = trace_seq_buffer_ptr(p);
+	int size = seq_buf_buffer_left(&p->seq);
+
+	dwc3_decode_event(str, size, event, ep0state);
+
+	seq_buf_commit(&p->seq, strlen(str));
+
+	return str;
+}
+
 static inline const char *dwc3_ep_cmd_status_string(int status)
 {
 	switch (status) {
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index 1975aec8d36d..bee2589e85a8 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -54,15 +54,13 @@ DECLARE_EVENT_CLASS(dwc3_log_event,
 	TP_STRUCT__entry(
 		__field(u32, event)
 		__field(u32, ep0state)
-		__dynamic_array(char, str, DWC3_MSG_MAX)
 	),
 	TP_fast_assign(
 		__entry->event = event;
 		__entry->ep0state = dwc->ep0state;
 	),
 	TP_printk("event (%08x): %s", __entry->event,
-			dwc3_decode_event(__get_str(str), DWC3_MSG_MAX,
-					__entry->event, __entry->ep0state))
+			dwc3_decode_event_seq(p, __entry->event, __entry->ep0state))
 );
 
 DEFINE_EVENT(dwc3_log_event, dwc3_event,