mbox series

[v6,0/5] usb: gadget: configfs: new trace events

Message ID 1649294865-4388-1-git-send-email-quic_linyyuan@quicinc.com
Headers show
Series usb: gadget: configfs: new trace events | expand

Message

Linyu Yuan April 7, 2022, 1:27 a.m. UTC
Last year I try to add trace event support for usb gadget configfs [1],
this time the idea is change a lot, the purpose is trace all user space
operation to gadget configuration, include gadget and it's function.

In usb gadget configfs, mainly user can do mkdir/rmdir a group,
link/unlink a function, change gadget/function attributes,
each operation will touch a struct config_item.

It only have one trace event entry which store string,
provide several API which represent user operation and generate string
from  struct config_item.

example output,
   mkdir-80      [000] .....    44.555106: gadget_configfs: mkdir dummy
      sh-76      [000] .....    44.562609: gadget_configfs: echo dummy/idVendor 0x05C6

   mkdir-81      [000] .....    44.569795: gadget_configfs: mkdir dummy/functions/eem.0
      sh-76      [000] .....    44.600221: gadget_configfs: echo dummy/functions/eem.0/dev_addr 1e:77:46:4b:1e:96

   mkdir-82      [000] .....    44.615542: gadget_configfs: mkdir dummy/configs/dummy.1
      ln-83      [000] .....    44.628997: gadget_configfs: link dummy/configs/dummy.1 dummy/functions/eem.0
      sh-76      [000] .....    44.634506: gadget_configfs: echo dummy/configs/dummy.1/MaxPower 500

   mkdir-84      [000] .....    44.642265: gadget_configfs: mkdir dummy/configs/dummy.1/strings/0x409
      sh-76      [000] .....    44.663886: gadget_configfs: echo dummy/configs/dummy.1/strings/0x409/configuration dummy

   rmdir-85      [000] .....    64.255507: gadget_configfs: rmdir dummy/configs/dummy.1/strings/0x409
      rm-86      [000] .....    64.263926: gadget_configfs: unlink dummy/configs/dummy.1 dummy/functions/eem.0
   rmdir-87      [000] .....    64.279768: gadget_configfs: rmdir dummy/configs/dummy.1
   rmdir-88      [000] .....    64.328124: gadget_configfs: rmdir dummy/functions/eem.0
   rmdir-89      [000] .....    64.992085: gadget_configfs: rmdir dummy


As it is different from last year change, start a new thread.

[1] https://lore.kernel.org/linux-usb/1635229309-2821-1-git-send-email-quic_linyyuan@quicinc.com/

V2: add example output
V3: add trace.c and move all APIs to it
v4: fix memory leak in v3
v5: change return value report by kernel test robot <lkp@intel.com> and
    Dan Carpenter <dan.carpenter@oracle.com>
v6: fix checkpatch warning

Linyu Yuan (5):
  usb: gadget: remove gadgets_type storage type 'static'
  usb: gadget: add trace event of configfs operation
  usb: gadget: add trace event of configfs group operation
  usb: gadget: add trace event of configfs link/unlink operation
  usb: gadget: add trace event of configfs write attributes operation

 drivers/usb/gadget/Makefile                    |   2 +
 drivers/usb/gadget/configfs.c                  |  45 ++++++-
 drivers/usb/gadget/function/f_acm.c            |   1 +
 drivers/usb/gadget/function/f_hid.c            |   4 +
 drivers/usb/gadget/function/f_loopback.c       |   4 +
 drivers/usb/gadget/function/f_mass_storage.c   |  20 +++
 drivers/usb/gadget/function/f_midi.c           |   6 +
 drivers/usb/gadget/function/f_printer.c        |   4 +
 drivers/usb/gadget/function/f_serial.c         |   1 +
 drivers/usb/gadget/function/f_sourcesink.c     |  16 +++
 drivers/usb/gadget/function/f_uac1.c           |   6 +
 drivers/usb/gadget/function/f_uac1_legacy.c    |   4 +
 drivers/usb/gadget/function/f_uac2.c           |   8 ++
 drivers/usb/gadget/function/u_ether_configfs.h |  10 ++
 drivers/usb/gadget/function/uvc_configfs.c     |  42 +++++++
 drivers/usb/gadget/trace.c                     | 163 +++++++++++++++++++++++++
 drivers/usb/gadget/trace.h                     |  39 ++++++
 include/linux/usb/composite.h                  |  18 +++
 include/linux/usb/gadget_configfs.h            |   4 +
 19 files changed, 396 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/gadget/trace.c
 create mode 100644 drivers/usb/gadget/trace.h

Comments

Greg KH April 22, 2022, 8:42 a.m. UTC | #1
On Thu, Apr 07, 2022 at 09:27:40AM +0800, Linyu Yuan wrote:
> Last year I try to add trace event support for usb gadget configfs [1],
> this time the idea is change a lot, the purpose is trace all user space
> operation to gadget configuration, include gadget and it's function.

But why?  Who will use this, and what for?

> In usb gadget configfs, mainly user can do mkdir/rmdir a group,
> link/unlink a function, change gadget/function attributes,
> each operation will touch a struct config_item.

As userspace is the thing doing this, why do you need to tell userspace
again that this happened?

> It only have one trace event entry which store string,
> provide several API which represent user operation and generate string
> from  struct config_item.
> 
> example output,
>    mkdir-80      [000] .....    44.555106: gadget_configfs: mkdir dummy
>       sh-76      [000] .....    44.562609: gadget_configfs: echo dummy/idVendor 0x05C6
> 
>    mkdir-81      [000] .....    44.569795: gadget_configfs: mkdir dummy/functions/eem.0
>       sh-76      [000] .....    44.600221: gadget_configfs: echo dummy/functions/eem.0/dev_addr 1e:77:46:4b:1e:96
> 
>    mkdir-82      [000] .....    44.615542: gadget_configfs: mkdir dummy/configs/dummy.1
>       ln-83      [000] .....    44.628997: gadget_configfs: link dummy/configs/dummy.1 dummy/functions/eem.0
>       sh-76      [000] .....    44.634506: gadget_configfs: echo dummy/configs/dummy.1/MaxPower 500
> 
>    mkdir-84      [000] .....    44.642265: gadget_configfs: mkdir dummy/configs/dummy.1/strings/0x409
>       sh-76      [000] .....    44.663886: gadget_configfs: echo dummy/configs/dummy.1/strings/0x409/configuration dummy
> 
>    rmdir-85      [000] .....    64.255507: gadget_configfs: rmdir dummy/configs/dummy.1/strings/0x409
>       rm-86      [000] .....    64.263926: gadget_configfs: unlink dummy/configs/dummy.1 dummy/functions/eem.0
>    rmdir-87      [000] .....    64.279768: gadget_configfs: rmdir dummy/configs/dummy.1
>    rmdir-88      [000] .....    64.328124: gadget_configfs: rmdir dummy/functions/eem.0
>    rmdir-89      [000] .....    64.992085: gadget_configfs: rmdir dummy

As I said in other places, why not just add this to configfs directly
instead of all over the individual users of this one subsystem?

And again, why?

thanks,

greg k-h
Linyu Yuan April 29, 2022, 12:46 a.m. UTC | #2
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Friday, April 22, 2022 8:17 PM
> To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com>
> Cc: Felipe Balbi <balbi@kernel.org>; linux-usb@vger.kernel.org; Jack Pham
> (QUIC) <quic_jackp@quicinc.com>
> Subject: Re: [PATCH v6 0/5] usb: gadget: configfs: new trace events
> 
> On Fri, Apr 22, 2022 at 09:01:13AM +0000, Linyu Yuan (QUIC) wrote:
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Sent: Friday, April 22, 2022 4:43 PM
> > > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com>
> > > Cc: Felipe Balbi <balbi@kernel.org>; linux-usb@vger.kernel.org; Jack
> Pham
> > > (QUIC) <quic_jackp@quicinc.com>
> > > Subject: Re: [PATCH v6 0/5] usb: gadget: configfs: new trace events
> > >
> > > On Thu, Apr 07, 2022 at 09:27:40AM +0800, Linyu Yuan wrote:
> > > > Last year I try to add trace event support for usb gadget configfs [1],
> > > > this time the idea is change a lot, the purpose is trace all user space
> > > > operation to gadget configuration, include gadget and it's function.
> > >
> > > But why?  Who will use this, and what for?
> >
> > Thanks for review it.
> > It is not used by user space, just for kernel gadget issue debugging.
> 
> So you use it in userspace?  How can you use a tracepoint from somewhere
> else in the kernel?
> 
> > Like android, the user space is complicate to kernel developers,
> > With this trace events, kernel development will understand
> > What is simplified action happen from user space.
> 
> They do not need this, they can just use ftrace today.  Most of these
> tracepoints you are putting in here are just for a "got to this
> function!" type of thing, which ftrace can show you already.
> 
> What is the added benefit of these over ftrace?

Thanks, please discard this changes,   will use kprobe event.

> 
> thanks,
> 
> greg k-h