Re: [PATCH v4 2/2] USB: xhci: use urb hcpriv mempool for private data

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, May 22, 2025 at 03:10:10PM +0800, David Wang wrote:
> xhci keeps alloc/free private data for each enqueue/dequeue cycles,
> when using a USB webcam, allocation rate is ~250/s;
> when using a USB mic, allocation rate reaches ~1k/s;
> The more usb device in use, the higher allocation rate.
> 
> URB objects have longer lifespan than private data, hand over ownership
> of private data to urb can save lots of memory allocations over time.
> With this change, no extra memory allocation is needed during usages of
> USB webcam/mic.
> 
> Signed-off-by: David Wang <00107082@163.com>
> ---
>  drivers/usb/host/xhci-mem.c  | 1 +
>  drivers/usb/host/xhci-ring.c | 3 +--
>  drivers/usb/host/xhci.c      | 8 +++-----
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index d698095fc88d..b19e41cf1c4c 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1745,6 +1745,7 @@ struct xhci_command *xhci_alloc_command_with_ctx(struct xhci_hcd *xhci,
>  
>  void xhci_urb_free_priv(struct urb_priv *urb_priv)
>  {
> +	WARN_ONCE(1, "xhci private data should be managed by urb");

You just crashed the kernel if this ever gets hit.  As you are saying
this should never be called, why is this function even present anymore?

This makes no sense :(

Again, NEVER add a WARN*() call to the kernel for something that it
should be handling properly on its own.  Otherwise you just lost all the
user's data when the box got rebooted (and if userspace can trigger
this, you just created a new CVE...)

thanks,

greg k-h