Re: [PATCH v3 1/2] USB: core: add a memory pool to urb caching host-controller private data

["David Wang" <00107082@163.com>]

At 2025-05-21 20:28:17, "Mathias Nyman" <mathias.nyman@intel.com> wrote:
>On 17.5.2025 11.38, David Wang wrote:
>> ---
>> Changes since v2:
>> 1. activat the pool only when the urb object is created via
>> usb_alloc_urb()
>> Thanks to Oliver Neukum <oneukum@suse.com>'s review.
>> ---
>> URB objects have long lifecycle, an urb can be reused between
>> submit loops; The private data needed by some host controller
>> has very short lifecycle, the memory is alloced when enqueue, and
>> released when dequeue. For example, on a system with xhci, in
>> xhci_urb_enqueue:
>> Using a USB webcam would have ~250/s memory allocation;
>> Using a USB mic would have ~1K/s memory allocation;
>> 
>> High frequent allocations for host-controller private data can be
>> avoided if urb take over the ownership of memory, the memory then shares
>> the longer lifecycle with urb objects.
>> 
>> Add a mempool to urb for hcpriv usage, the mempool only holds one block
>> of memory and grows when larger size is requested.
>> 
>> The mempool is activated only when the URB object is created via
>> usb_alloc_urb() in case some drivers create a URB object by other
>> means and manage it lifecycle without corresponding usb_free_urb.
>
>Won't this still allocate a lot of unnecessary memory for the roothub urbs?
>i.e. the ones queued with rh_urb_enqueue(hcd, urb).
>The host drivers don't use the urb->hcpriv of those URBs.
>
The mempool slot is alloced on demand when hcd request private data with its urb.
If a urb is  ever used by hcd and the  hcd requests private data with it, a  memory would be alloced
and this memory will not be released until the urb is destroyed.

If those URB keeps being reused, than the mempool slot within it can be reused by hcds which get hold on it.

And yes, it a URB is used only once or used very unfrequently, and hcd needs private data with it, the mempool
slot would be alloced with it and would be a very useless mempool slot, wasting memory.

>This would be the roothub status URB, and every hub request sent
>during device enumeration for devices connected to the roothub.

>
>Is this whole URB hcpriv mempool addition an actual issue that needs fixing?

NO, not an issue, just meant to avoid memory allocations, lots of allocation, to theoretically  improve things..

>
>If yes then I still think it's better to do it in the host driver like I
>described earlier. I don't think it will be too complex
>
>Thanks
>-Mathias
>
>