mbox series

[v2,00/11] usb: gadget: reduce usb gadget trace event buffer usage

Message ID 20230911112446.1791-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. 11, 2023, 11:24 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.

v1: https://lore.kernel.org/linux-usb/20230911042843.2711-1-quic_linyyuan@quicinc.com/
v2: fix two compile issues that COMPILE_TEST not covered

Linyu Yuan (11):
  usb: gadget: add anonymous definition in struct usb_gadget
  usb: gadget: add anonymous definition in struct usb_request
  usb: gadget: add anonymous definition in struct usb_ep
  usb: udc: assign epnum for each usb endpoint
  usb: udc: trace: reduce buffer usage of trace event
  usb: cdns3: cdnsp: reduce buffer usage of trace event
  usb: cdns3: trace: reduce buffer usage of trace event
  usb: dwc3: trace: reduce buffer usage of trace event
  usb: cdns2: trace: reduce buffer usage of trace event
  usb: mtu3: trace: reduce buffer usage of trace event
  usb: musb: trace: reduce buffer usage of trace event

 drivers/usb/cdns3/cdns3-gadget.c            |   1 +
 drivers/usb/cdns3/cdns3-trace.h             |  93 +++++------
 drivers/usb/cdns3/cdnsp-gadget.c            |   1 +
 drivers/usb/cdns3/cdnsp-trace.h             |  45 +++---
 drivers/usb/chipidea/udc.c                  |   1 +
 drivers/usb/dwc2/gadget.c                   |   2 +-
 drivers/usb/dwc3/gadget.c                   |   1 +
 drivers/usb/dwc3/trace.h                    |  54 +++----
 drivers/usb/fotg210/fotg210-udc.c           |   1 +
 drivers/usb/gadget/udc/aspeed-vhub/epn.c    |   1 +
 drivers/usb/gadget/udc/aspeed_udc.c         |   1 +
 drivers/usb/gadget/udc/at91_udc.c           |   1 +
 drivers/usb/gadget/udc/atmel_usba_udc.c     |   1 +
 drivers/usb/gadget/udc/bcm63xx_udc.c        |   1 +
 drivers/usb/gadget/udc/bdc/bdc_ep.c         |   1 +
 drivers/usb/gadget/udc/cdns2/cdns2-gadget.c |   2 +-
 drivers/usb/gadget/udc/cdns2/cdns2-trace.h  |  77 +++++-----
 drivers/usb/gadget/udc/dummy_hcd.c          |   1 +
 drivers/usb/gadget/udc/fsl_qe_udc.c         |   1 +
 drivers/usb/gadget/udc/fsl_udc_core.c       |   1 +
 drivers/usb/gadget/udc/fusb300_udc.c        |   1 +
 drivers/usb/gadget/udc/goku_udc.c           |   1 +
 drivers/usb/gadget/udc/gr_udc.c             |   1 +
 drivers/usb/gadget/udc/lpc32xx_udc.c        |   1 +
 drivers/usb/gadget/udc/m66592-udc.c         |   1 +
 drivers/usb/gadget/udc/max3420_udc.c        |   1 +
 drivers/usb/gadget/udc/mv_u3d_core.c        |   2 +
 drivers/usb/gadget/udc/mv_udc_core.c        |   2 +
 drivers/usb/gadget/udc/net2272.c            |   1 +
 drivers/usb/gadget/udc/net2280.c            |   2 +
 drivers/usb/gadget/udc/omap_udc.c           |   1 +
 drivers/usb/gadget/udc/pch_udc.c            |   1 +
 drivers/usb/gadget/udc/pxa25x_udc.c         |   1 +
 drivers/usb/gadget/udc/pxa27x_udc.c         |   1 +
 drivers/usb/gadget/udc/r8a66597-udc.c       |   1 +
 drivers/usb/gadget/udc/renesas_usb3.c       |   1 +
 drivers/usb/gadget/udc/renesas_usbf.c       |   1 +
 drivers/usb/gadget/udc/snps_udc_core.c      |   1 +
 drivers/usb/gadget/udc/tegra-xudc.c         |   2 +
 drivers/usb/gadget/udc/trace.h              | 106 +++++--------
 drivers/usb/gadget/udc/udc-xilinx.c         |   1 +
 drivers/usb/isp1760/isp1760-udc.c           |   1 +
 drivers/usb/mtu3/mtu3_gadget.c              |   1 +
 drivers/usb/mtu3/mtu3_trace.h               |  42 +++--
 drivers/usb/musb/musb_gadget.c              |   1 +
 drivers/usb/musb/musb_trace.h               |  14 +-
 drivers/usb/renesas_usbhs/mod_gadget.c      |   1 +
 drivers/usb/usbip/vudc_dev.c                |   1 +
 include/linux/usb/gadget.h                  | 161 +++++++++++++++-----
 49 files changed, 344 insertions(+), 295 deletions(-)

Comments

Greg Kroah-Hartman Sept. 11, 2023, 1:48 p.m. UTC | #1 
On Mon, Sep 11, 2023 at 09:44:21PM +0800, Linyu Yuan wrote:
> 
> On 9/11/2023 9:32 PM, Greg Kroah-Hartman wrote:
> > On Mon, Sep 11, 2023 at 07:24:35PM +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.
> > Ok, but how much memory did you actually save here?  Is the memory saved
> > only if tracing is enbaled, or it is always?  Is there a speed penality
> > for these changes or is it the same?
> 
> yes, it is save trace ring buffer only when tracing enabled, as save less
> data, speed is higher.

Are you sure?  You now have to parse the data, so how is it faster?

> it is help when enable trace for a long period.

Help exactly how?  What is saved exactly, and what about cpu usage?

> for a single trace entry, take struct usb_gadget as example, at worst case,
> it save (19 dword  - 1 dword) / 19dword = 94% buffer.
> 
> for ep name, it only need 4 bytes/  (4bytes + 9bytes ) = 30%.

And this buffer size reduction shows up where?  Are you trading space
for CPU time?  Somehow you are ending up with the same information in
userspace, so where are the tradeoffs?

thanks,

greg k-h