Re: [PATCH v2 2/2] trace: allocate space from temparary trace sequence buffer

[Linyu Yuan]

On 12/15/2022 12:54 PM, Steven Rostedt wrote:
> On Thu, 15 Dec 2022 12:33:27 +0800
> Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>
> 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,