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

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

At 2025-05-13 22:25:50, "Alan Stern" <stern@rowland.harvard.edu> wrote:
>On Tue, May 13, 2025 at 07:38:17PM +0800, David Wang wrote:
>> ---
>> Changes:
>> 1. Use caller's mem_flags if a larger memory is needed.
>> Thanks to Oliver Neukum <oneukum@suse.com>'s review.
>> ---
>> URB objects have long lifecycle, an urb can be reused between
>> enqueue-dequeue 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, several
>> minutes of usage of webcam/keyboard/mouse have memory alloc counts:
>>   drivers/usb/core/urb.c:75 [usbcore] func:usb_alloc_urb 661
>>   drivers/usb/host/xhci.c:1555 [xhci_hcd] func:xhci_urb_enqueue 424863
>> Memory allocation frequency for host-controller private data can reach
>> ~1k/s and above.
>> 
>> 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.
>> 
>> Signed-off-by: David Wang <00107082@163.com>
>
>It should be possible to do what you want without touching the USB core 
>code at all, changing only xhci-hcd.  That's what Mathias is suggesting.
>
>Instead of having an URB keep ownership of its extra memory after it 
>completes, xhci-hcd can put the memory area onto a free list.  Then 
>memory areas on the free list can be reused with almost no overhead when 
>URBs are enqueued later on.

I have to disagree,  your suggestion has no much difference from requesting memory from
system, locks and memory pool managements, all the same are needed, why bother?

The reason I choose URB is that  URB life cycle contains the private data's lifecycle, 
and no two HCD can take over the same URB as the same time.

I think the memory pool here is not actually a pool,  or I should say the mempool consists of
pool of URBs, and each URB have only "single one" slot of mem pool in it.


>
>This idea can become more elaborate if you maintain separate free lists 
>for different devices, or even for different endpoints, or sort the free 
>list by the size of the memory areas.  But the basic idea is always the 
>same: Don't change usbcore (including struct urb), and make getting and 
>releasing the extra memory areas have extremely low overhead.

Why implements a device level memory pool would have extremely low overhead?
Why  making changes to usb core is bad? The idea in this thread is meant for all kinds of
host controllers, xhci is what I have in my system; i think similar changes would benefit other
HCs


>
>Alan Stern