| 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
| ||
|
Message-Id: <a235e322e270942dc3d607d4b46ff7db29abeb2d.1747897366.git.00107082@163.com>
Date: Thu, 22 May 2025 15:09:43 +0800
From: David Wang <00107082@....com>
To: gregkh@...uxfoundation.org,
mathias.nyman@...el.com
Cc: oneukum@...e.com,
stern@...land.harvard.edu,
hminas@...opsys.com,
rui.silva@...aro.org,
jgross@...e.com,
linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org,
David Wang <00107082@....com>
Subject: [PATCH v4 1/2] USB: core: add a memory pool to urb caching host-controller private data
When using USB devices, there are lots of memory allocations for
host controller's private data. For example, on a system with xhci,
using a webcam and a mic would needs ~250 and ~1k kzallocs per
second, respectively.
Some HCDs use kmem_cache to handle the high frequency of memory
allocation during device usages. There are several drawbacks:
1. The lifespan of kmem_cache entry has weak correlation with
device usages: when devices are in idle, those memory is still
kept in slab until system decide to reclaim it, if reclaimable.
2. kmem_cache is only used for fix-sized memory allocation, if a
HCD needs private data of m different sizes, m different kmem_cache
would be needed, and the overhead of kmem_cache is multiplied by m.
When m is large, as some HCD needs variable length private data,
kmem_cache approach is inflexible to use.
3. A system may have multiple HCDs in use, then the overhead of
kmem_cache would be multiplied again by the number of HCDs.
URB objects have long lifespans, an urb can be reused between
submit loops while the devices are being used. And with following
premise:
1. Each URB cannot be used by more than one HCDs at the same time.
2. The lifespan of HC private data is contained in its URB object's
lifespan.
high frequent allocations for HCD private data can be avoided if
urb take over the ownership of memory, the memory then shares
the longer lifespan with urb objects.
The memory pool in URB works this way:
1. A URB object holds only one slot of memory pool.
2. When created, memory pool is NULL. If no private data requested,
no memory pool alloced for the URB.
3. The memory pool slot will grow when larger size is requested.
4. Memory pool slot will be released only when URB is destroyed.
Normally the hcpriv pointer in URB structure is the same as memory
pool slot, but considering some drivers may still have its own
management of urb->hcpriv, a hcpriv_mempool pointer is added, to make
sure no impact on existing drivers.
A hcpriv_mempool_size field is needed for making sure that the mempool
slot is big enough, if not, more memory would be requested from system.
Existing drivers may have it own allocation of URB object, not via
usb_alloc_urb(), this may cause memory leak if URB has a active mempool
slot but it is not released via usb_free_urb(); A hcpriv_mempool_activated
flag is added to address this issue, it is set only in usb_alloc_urb().
When requesting a hcpriv memory to a URB which has hcpriv_mempool_activated
not set, the memory will be requested from system directly.
One of the drawbacks is URB object now have 3 more fields even if the
object would never be requested for HCD private data.
>From an end-user's perspective, the performance difference with this change
is insignificant when system is under no memory pressure, and when under
heavy memory pressure. When system is under heavy memory pressure,
everything is slow. There could be a point in-between no memory pressure
and heavy memory pressure where these 1k+/s memory allocations would
dominate the performance, but very hard to pinpoint it.
Signed-off-by: David Wang <00107082@....com>
---
drivers/usb/core/urb.c | 45 ++++++++++++++++++++++++++++++++++++++++++
include/linux/usb.h | 5 +++++
2 files changed, 50 insertions(+)
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 5e52a35486af..2cf218d4fb73 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -23,6 +23,8 @@ static void urb_destroy(struct kref *kref)
if (urb->transfer_flags & URB_FREE_BUFFER)
kfree(urb->transfer_buffer);
+ if (urb->hcpriv_mempool_activated)
+ kfree(urb->hcpriv_mempool);
kfree(urb);
}
@@ -77,6 +79,8 @@ struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags)
if (!urb)
return NULL;
usb_init_urb(urb);
+ /* activate hcpriv mempool when urb is created via usb_alloc_urb */
+ urb->hcpriv_mempool_activated = true;
return urb;
}
EXPORT_SYMBOL_GPL(usb_alloc_urb);
@@ -1037,3 +1041,44 @@ int usb_anchor_empty(struct usb_anchor *anchor)
EXPORT_SYMBOL_GPL(usb_anchor_empty);
+/**
+ * urb_hcpriv_mempool_zalloc - alloc memory from mempool for hcpriv
+ * @urb: pointer to URB being used
+ * @size: memory size requested by current host controller
+ * @mem_flags: the type of memory to allocate
+ *
+ * Return: NULL if out of memory, otherwise memory are zeroed
+ */
+void *urb_hcpriv_mempool_zalloc(struct urb *urb, size_t size, gfp_t mem_flags)
+{
+ if (!urb->hcpriv_mempool_activated)
+ return kzalloc(size, mem_flags);
+
+ if (urb->hcpriv_mempool_size < size) {
+ kfree(urb->hcpriv_mempool);
+ urb->hcpriv_mempool_size = size;
+ urb->hcpriv_mempool = kmalloc(size, mem_flags);
+ }
+ if (urb->hcpriv_mempool)
+ memset(urb->hcpriv_mempool, 0, size);
+ else
+ urb->hcpriv_mempool_size = 0;
+ return urb->hcpriv_mempool;
+}
+EXPORT_SYMBOL_GPL(urb_hcpriv_mempool_zalloc);
+
+/**
+ * urb_free_hcpriv - free hcpriv data if necessary
+ * @urb: pointer to URB being used
+ *
+ * If mempool is activated, private data's lifecycle
+ * is managed by urb object.
+ */
+void urb_free_hcpriv(struct urb *urb)
+{
+ if (urb->hcpriv != urb->hcpriv_mempool) {
+ kfree(urb->hcpriv);
+ urb->hcpriv = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(urb_free_hcpriv);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index b46738701f8d..27bc394b8141 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1602,6 +1602,9 @@ struct urb {
struct kref kref; /* reference count of the URB */
int unlinked; /* unlink error code */
void *hcpriv; /* private data for host controller */
+ void *hcpriv_mempool; /* a single slot of cache for HCD's private data */
+ size_t hcpriv_mempool_size; /* current size of the memory pool */
+ bool hcpriv_mempool_activated; /* flag the mempool usage */
atomic_t use_count; /* concurrent submissions counter */
atomic_t reject; /* submissions will fail */
@@ -1790,6 +1793,8 @@ extern int usb_wait_anchor_empty_timeout(struct usb_anchor *anchor,
extern struct urb *usb_get_from_anchor(struct usb_anchor *anchor);
extern void usb_scuttle_anchored_urbs(struct usb_anchor *anchor);
extern int usb_anchor_empty(struct usb_anchor *anchor);
+extern void *urb_hcpriv_mempool_zalloc(struct urb *urb, size_t size, gfp_t mem_flags);
+extern void urb_free_hcpriv(struct urb *urb);
#define usb_unblock_urb usb_unpoison_urb
--
2.39.2
Powered by blists - more mailing lists