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 22:00:14, "David Wang" <00107082@163.com> wrote:
>
>When looking in codes, I notice xen-hcd have already implemented a mempool, for example:
>drivers/usb/host/xen-hcd.c
>1323 static int xenhcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb
>...
>1334         urbp = kmem_cache_zalloc(xenhcd_urbp_cachep, mem_flags);
>1335         if (!urbp)
>1336                 return -ENOMEM;
>1337 
>1338         spin_lock_irqsave(&info->lock, flags);
>1339 
>1340         urbp->urb = urb;
>1341         urb->hcpriv = urbp;
>1342         urbp->req_id = ~0;
>
>But most others still use kzalloc.   
>Between kmem_cache and a mem slot in URB, I want say the mem slot in URB is managed more efficiently/balanced,
>and would it be  healthy to add kmem_cache in each hcds as they see the needs, we would have separated kmem_cache
>everywhere.
>
>
>Thanks
>David

hcds found using kmem_cache  for hc private data:

./drivers/usb/dwc2/hcd.c:		hsotg->desc_gen_cache = kmem_cache_create("dwc2-gen-desc",
./drivers/usb/dwc2/hcd.c:		hsotg->desc_hsisoc_cache = kmem_cache_create("dwc2-hsisoc-desc",
./drivers/usb/dwc2/hcd.c:		hsotg->unaligned_cache = kmem_cache_create("dwc2-unaligned-dma",
./drivers/usb/isp1760/isp1760-hcd.c:	urb_listitem_cachep = kmem_cache_create("isp1760_urb_listitem",
./drivers/usb/isp1760/isp1760-hcd.c:	qtd_cachep = kmem_cache_create("isp1760_qtd",
./drivers/usb/isp1760/isp1760-hcd.c:	qh_cachep = kmem_cache_create("isp1760_qh", sizeof(struct isp1760_qh),
./drivers/usb/host/uhci-hcd.c:	uhci_up_cachep = kmem_cache_create("uhci_urb_priv",
./drivers/usb/host/xen-hcd.c:	xenhcd_urbp_cachep = kmem_cache_create("xenhcd_urb_priv",

I think those hcds have already considered the high frequency of memory allocation for private data  as an issue and
address it via a const sized memory pool..
xhci could not follow this pattern since it needs a variable length memory.
But is this a good pattern to follow? A single slot of memory in URB can handle all those request.


Thanks
David