mbox series

[v6,0/3] usb: gadget: reduce usb gadget trace event buffer usage

Message ID 20230915052716.28540-1-quic_linyyuan@quicinc.com (mailing list archive)
Headers show
Series usb: gadget: reduce usb gadget trace event buffer usage | expand

Message

Linyu Yuan Sept. 15, 2023, 5:27 a.m. UTC
some trace event use an interger to to save a bit field info of gadget,
also some trace save endpoint name in string forat, it all can be
chagned to other way at trace event store phase.

bit field can be replace with a union interger member which include
multiple bit fields.

ep name stringe can be replace to a interger which contaion number
and dir info.

in order to avoid big endian issue, save interger data into ring
buffer in __le32 format.

backgroud:
the benefit is when user not increase system trace event buffer space,
or in lower system trace event buffer space, it allow more trace event
entries to be saved.


in normal condition, the usb request is most frequent things after
enumeration with useful operation,
so take below trace event class for explanation,


DECLARE_EVENT_CLASS(udc_log_req,
    TP_PROTO(struct usb_ep *ep, struct usb_request *req, int ret),
    TP_ARGS(ep, req, ret),
    TP_STRUCT__entry(
        __string(name, ep->name)
        __field(unsigned, length)
        __field(unsigned, actual)
        __field(unsigned, num_sgs)
        __field(unsigned, num_mapped_sgs)
        __field(unsigned, stream_id)
        __field(unsigned, no_interrupt)
        __field(unsigned, zero)
        __field(unsigned, short_not_ok)
        __field(int, status)
        __field(int, ret)
        __field(struct usb_request *, req)
    ),
    TP_fast_assign(
        __assign_str(name, ep->name);
        __entry->length = req->length;
        __entry->actual = req->actual;
        __entry->num_sgs = req->num_sgs;
        __entry->num_mapped_sgs = req->num_mapped_sgs;
        __entry->stream_id = req->stream_id;
        __entry->no_interrupt = req->no_interrupt;
        __entry->zero = req->zero;
        __entry->short_not_ok = req->short_not_ok;
        __entry->status = req->status;
        __entry->ret = ret;
        __entry->req = req;
    ),
    TP_printk("%s: req %p length %d/%d sgs %d/%d stream %d %s%s%s status %d --> %d",
        __get_str(name),__entry->req,  __entry->actual, __entry->length,
        __entry->num_mapped_sgs, __entry->num_sgs, __entry->stream_id,
        __entry->zero ? "Z" : "z",
        __entry->short_not_ok ? "S" : "s",
        __entry->no_interrupt ? "i" : "I",
        __entry->status, __entry->ret
    )
);


consider 32 bit ARCH,
without change, one trace entry size is:
4 (ring buffer event header ) + 8 (trace event header ) +
48 (trace class header) + 9 (ep string name) = 69 bytes.

with change,
4 (ring buffer event header ) + 8 (trace event header ) +
36 (trace class header)  = 48 bytes.

consider there is 1MB trace buffer space,
without change, it can save 15196 entries,
with change, it can save 21845 entries. 


v1: https://lore.kernel.org/linux-usb/20230911042843.2711-1-quic_linyyuan@quicinc.com/
v2: fix two compile issues that COMPILE_TEST not covered
    https://lore.kernel.org/linux-usb/20230911112446.1791-1-quic_linyyuan@quicinc.com/
v3: fix reviewer comments, allow bit fields work on both little and big endian
    https://lore.kernel.org/linux-usb/20230912104455.7737-1-quic_linyyuan@quicinc.com/
v4: add DECLARE_EVENT_CLASS_PRINT_INIT() new trace class and use it
    https://lore.kernel.org/linux-usb/20230914100302.30274-1-quic_linyyuan@quicinc.com/
v5: use cpu_to_le32() at fast assign stage to fix endian issue
    https://lore.kernel.org/linux-usb/20230915051123.26486-1-quic_linyyuan@quicinc.com/
v6: missing three cpu_to_le32() usage in dwc3 trace

Linyu Yuan (3):
  usb: gadget: add anonymous definition in some struct for trace purpose
  usb: udc: trace: reduce buffer usage of trace event
  usb: dwc3: trace: reduce buffer usage of trace event

 drivers/usb/dwc3/trace.h       |  63 +++++------
 drivers/usb/gadget/udc/trace.h | 114 +++++--------------
 include/linux/usb/gadget.h     | 201 ++++++++++++++++++++++++++-------
 3 files changed, 219 insertions(+), 159 deletions(-)

Comments

Alan Stern Sept. 15, 2023, 2:13 p.m. UTC | #1
On Fri, Sep 15, 2023 at 01:27:13PM +0800, Linyu Yuan wrote:
> some trace event use an interger to to save a bit field info of gadget,
> also some trace save endpoint name in string forat, it all can be
> chagned to other way at trace event store phase.
> 
> bit field can be replace with a union interger member which include
> multiple bit fields.
> 
> ep name stringe can be replace to a interger which contaion number
> and dir info.
> 
> in order to avoid big endian issue, save interger data into ring
> buffer in __le32 format.

This won't do what you want.  cpu_to_le32() puts the _bytes_ in order 
from least significant to most significant.  But what you want is to put 
the _bits_ in order.

For example, suppose sg_supported ends up sitting in BIT(31) and it is 
the only flag set.  The value of g->dw1 would be 0x80000000.  Then 
cpu_to_le32(g->dw1) would be 0x00000080, not 0x00000001.

You should do what I wrote earlier:

	__entry->dw1 = (g->sg_supported << 0) |
			(g->is_otg << 1) |
			...

Alan Stern