linux-kernel - Re: [RFC] USB: core/xhci: add a buffer in urb for host controller private data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49a5d0b7.9095.196c9056aff.Coremail.00107082@163.com>
Date: Tue, 13 May 2025 17:41:37 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Mathias Nyman" <mathias.nyman@...el.com>
Cc: gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, surenb@...gle.com,
	kent.overstreet@...ux.dev
Subject: Re: [RFC] USB: core/xhci: add a buffer in urb for host controller
 private data



At 2025-05-13 17:27:34, "Mathias Nyman" <mathias.nyman@...el.com> wrote:
>Hi David
>
>On 12.5.2025 18.07, David Wang wrote:
>> ---
>> I was checking memory allocation behaviors (via memory profiling[1]),
>> when I notice a high frequent memory allocation in xhci_urb_enqueue, about
>> 250/s when using a USB webcam. If those alloced buffer could be kept and
>> reused, lots of memory allocations could be avoid over time.
>> 
>> This patch is just a POC, about 0/s memory allocation in xhci with this
>> patch, when I use my USB devices, webcam/keyboard/mouse.
>
>Thanks for looking at this, this is something that obviously needs more
>attention
>
>> 
>> A dynamic cached memory would be better: URB keep host controller's
>> private data, if larger size buffer needed for private data, old buffer
>> released and a larger buffer alloced.
>> 
>> I did not observe any nagative impact with xhci's 250/s allocations
>> when using my system, hence no measurement of how useful this changes
>> can make to user. Just want to collect feedbacks before putting more
>> effort.
>> 
>
>I think we can make a xhci internal solution that doesn't affect other hosts
>or usb core.

Yes,  my latest patches separates xhci changes from  usbcore .
>
>How about adding a list of struct urb_priv nodes for every usb device, or maybe
>even per endpoint? i.e. to struct xhci_virt_device or  struct xhci_virt_ep.
>
>Add size and list_head entries to struct urb_priv.
>
device level mempool could be very complicated, I think a single memory pool hold by URB
would be much simpler. (Could you help  take a look at this patch:
https://lore.kernel.org/lkml/20250513055447.5696-1-00107082@163.com/) 

>In xhci_urb_enqueue() pick the first urb_priv node from list if it exists and is
>large enough, otherwise just allocate a new one and set the size.
>
>When giving back the URB zero everything in the struct urb_priv except the size,
>and add it to the list.
>
>When the device is freed there shouldn't be any nodes left in the list, but if
>there are then warn and free them.
>
>Isoc transfers are the ones with odd urb_priv sizes. If we have a per device or
>per endpoint urb_priv list then sizes will match better.

If the mempool is in URB, then only largest size matters, since no two HC can own the URB
at the same time.

>
>Thanks
>Mathias
>

Thanks for the comments and suggestions.
David