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;
Message-ID: <fea0e9c9-18b6-0b6f-9e5e-5632d7ab6eea@quicinc.com>
Date:   Thu, 15 Dec 2022 13:08:21 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.0
Subject: Re: [PATCH v2 2/2] trace: allocate space from temparary trace
 sequence buffer
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Masami Hiramatsu <mhiramat@kernel.org>,
        <linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>
References: <1671078807-20748-1-git-send-email-quic_linyyuan@quicinc.com>
 <1671078807-20748-2-git-send-email-quic_linyyuan@quicinc.com>
 <20221214235402.5ea7489e@gandalf.local.home>
Content-Language: en-US
From:   Linyu Yuan <quic_linyyuan@quicinc.com>
In-Reply-To: <20221214235402.5ea7489e@gandalf.local.home>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/15/2022 12:54 PM, Steven Rostedt wrote:
> 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!
ok, so you don't like a common function in trace_seq.c ?
>
> 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,