[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <42f4753a-f7db-49a3-ba40-8743a78684b4@rowland.harvard.edu>
Date: Fri, 25 Jul 2025 11:46:16 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: Olivier Tuchon <tcn@...gle.com>
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH] usb: gadget: Add gadgetmon traffic monitor
On Fri, Jul 25, 2025 at 05:25:49PM +0200, Olivier Tuchon wrote:
> The Linux kernel lacks a tool equivalent to usbmon for the gadget side,
> making it difficult to debug the lifecycle and performance of usb_requests.
> This forces developers into using ad-hoc printk statements for
> introspection.
>
> This commit introduces "gadgetmon", a comprehensive framework for
> monitoring USB gadget traffic. It consists of two main parts: a new
> monitoring interface in the UDC core and a loadable module that
> implements this interface to provide data to userspace.
>
> The UDC core is modified to add an optional monitoring interface defined
> by struct usb_gadget_mon_operations in <linux/usb/gadget.h>.
This does not appear in the patch. Was it left out by mistake?
> An
> RCU-protected global pointer, gadget_mon_ops, is defined and exported
> to allow monitoring modules to register. Hooks are placed in
> usb_ep_queue() and usb_gadget_giveback_request() to call this interface,
> capturing request submission and completion events.
Do you expect that other gadget monitoring modules will be written?
If they are, assignment to the global pointer should be handled by a
routine in the gadget core, not in the monitoring module as done here.
> + /*
> + * optimization: for an OUT submission (host-to-device), the data
> + * has not yet arrived from the host. The buffer is an empty
> + * placeholder, so its content is not captured to save space.
> + */
> + if (event_type == GADGETMON_EVENT_SUBMIT && hdr.dir == USB_DIR_OUT)
> + payload_len = 0;
There should be a similar optimization for IN givebacks. The data to
be transferred to the host was already recorded by the submission
hook, so you can save space by not copying it a second time during the
giveback.
> +
> + hdr.data_len = payload_len;
> + total_len = sizeof(hdr) + payload_len;
> +
> + /* lock and queue the event into the FIFO */
> + spin_lock_irqsave(&mon_lock, flags);
> +
> + if (kfifo_avail(&mon_fifo) < total_len) {
> + /* not enough space, drop the event silently */
Would it be better to keep the event but drop the tail end of the data?
Alan Stern
Powered by blists - more mailing lists