|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [UNIKRAFT PATCH 4/7] lib/uk9p: Recycle uk_9preq allocations
Each 9p device keeps a free-list of 9p requests. This allows
allocations to take much less time, as usually the workflow of a 9p
device is synchronous: create request, wait for reply, process it,
repeat. This approach avoids the allocation performance hit on
subsequent request creations.
Results below indicate that request_allocated takes < 100ns,
as opposed to 1us (previous commit in this patch series).
In the sample below, the read() now spends most of the time (~99.7%)
either notifying the host or waiting for a reply. At this scale,
even uk_traces are probably noisy and the "real" time might be lower
by a relatively large amount.
Excerpt from the uk traces on a read() call with in-depth tracing:
1846467237 uk_9p_trace_read fid 2 offset 61440 count 4096
1846467321 uk_9p_trace_request_allocated
1846467350 uk_9p_trace_ready tag 0
1846470161 uk_9p_trace_sent tag 0
1846508851 uk_9p_trace_received tag 0
1846508889 uk_9p_trace_read_end count 4096
Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
---
lib/uk9p/9pdev.c | 78 +++++++++++++++++++++++++++++---
lib/uk9p/9preq.c | 14 ++----
lib/uk9p/include/uk/9pdev.h | 10 ++++
lib/uk9p/include/uk/9pdev_core.h | 2 +
lib/uk9p/include/uk/9preq.h | 15 +++---
5 files changed, 93 insertions(+), 26 deletions(-)
diff --git a/lib/uk9p/9pdev.c b/lib/uk9p/9pdev.c
index 40a4daa..42c5058 100644
--- a/lib/uk9p/9pdev.c
+++ b/lib/uk9p/9pdev.c
@@ -132,6 +132,7 @@ static void _req_mgmt_init(struct uk_9pdev_req_mgmt
*req_mgmt)
ukarch_spin_lock_init(&req_mgmt->spinlock);
uk_bitmap_zero(req_mgmt->tag_bm, UK_9P_NUMTAGS);
UK_INIT_LIST_HEAD(&req_mgmt->req_list);
+ UK_INIT_LIST_HEAD(&req_mgmt->req_free_list);
}
static void _req_mgmt_add_req_locked(struct uk_9pdev_req_mgmt *req_mgmt,
@@ -141,6 +142,21 @@ static void _req_mgmt_add_req_locked(struct
uk_9pdev_req_mgmt *req_mgmt,
uk_list_add(&req->_list, &req_mgmt->req_list);
}
+static struct uk_9preq *
+_req_mgmt_from_freelist_locked(struct uk_9pdev_req_mgmt *req_mgmt)
+{
+ struct uk_9preq *req;
+
+ if (uk_list_empty(&req_mgmt->req_free_list))
+ return NULL;
+
+ req = uk_list_first_entry(&req_mgmt->req_free_list,
+ struct uk_9preq, _list);
+ uk_list_del(&req->_list);
+
+ return req;
+}
+
static void _req_mgmt_del_req_locked(struct uk_9pdev_req_mgmt *req_mgmt,
struct uk_9preq *req)
{
@@ -148,6 +164,12 @@ static void _req_mgmt_del_req_locked(struct
uk_9pdev_req_mgmt *req_mgmt,
uk_list_del(&req->_list);
}
+static void _req_mgmt_req_to_freelist_locked(struct uk_9pdev_req_mgmt
*req_mgmt,
+ struct uk_9preq *req)
+{
+ uk_list_add(&req->_list, &req_mgmt->req_free_list);
+}
+
static uint16_t _req_mgmt_next_tag_locked(struct uk_9pdev_req_mgmt *req_mgmt)
{
return uk_find_next_zero_bit(req_mgmt->tag_bm, UK_9P_NUMTAGS, 0);
@@ -164,10 +186,24 @@ static void _req_mgmt_cleanup(struct uk_9pdev_req_mgmt
*req_mgmt __unused)
tag = req->tag;
_req_mgmt_del_req_locked(req_mgmt, req);
if (!uk_9preq_put(req)) {
- uk_pr_warn("Tag %d still has references on cleanup.\n",
+ /* If in the future these references get released, mark
+ * _dev as NULL so uk_9pdev_req_to_freelist doesn't
+ * attempt to place them in an invalid memory region.
+ *
+ * As _dev is not used for any other purpose, this
+ * doesn't impact any other logic related to 9p request
+ * processing.
+ */
+ req->_dev = NULL;
+ uk_pr_err("Tag %d still has references on cleanup.\n",
tag);
}
}
+ uk_list_for_each_entry_safe(req, reqn, &req_mgmt->req_free_list,
+ _list) {
+ uk_list_del(&req->_list);
+ uk_free(req->_a, req);
+ }
ukplat_spin_unlock_irqrestore(&req_mgmt->spinlock, flags);
}
@@ -297,17 +333,35 @@ struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev
*dev, uint8_t type)
UK_ASSERT(dev);
- req = uk_9preq_alloc(dev->a);
- if (req == NULL) {
- rc = -ENOMEM;
- goto out;
+ ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
+ if (!(req = _req_mgmt_from_freelist_locked(&dev->_req_mgmt))) {
+ /* Don't allocate with the spinlock held. */
+ ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, flags);
+ req = uk_calloc(dev->a, 1, sizeof(*req));
+ if (req == NULL) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ req->_dev = dev;
+ /*
+ * Duplicate this, instead of using req->_dev, as we can't rely
+ * on the value of _dev at time of free. Check comment in
+ * _req_mgmt_cleanup.
+ */
+ req->_a = dev->a;
+ ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
}
+ uk_9preq_init(req);
+
+ /* If request was from the free list, it should already belong to the
+ * dev. */
+ UK_ASSERT(req->_dev == dev);
+
/* Shouldn't exceed the msize on non-zerocopy buffers, just in case. */
req->recv.size = MIN(req->recv.size, dev->msize);
req->xmit.size = MIN(req->xmit.size, dev->msize);
- ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
if (type == UK_9P_TVERSION)
tag = UK_9P_NOTAG;
else
@@ -360,6 +414,18 @@ int uk_9pdev_req_remove(struct uk_9pdev *dev, struct
uk_9preq *req)
return uk_9preq_put(req);
}
+void uk_9pdev_req_to_freelist(struct uk_9pdev *dev, struct uk_9preq *req)
+{
+ unsigned long flags;
+
+ if (!dev)
+ return;
+
+ ukplat_spin_lock_irqsave(&dev->_req_mgmt.spinlock, flags);
+ _req_mgmt_req_to_freelist_locked(&dev->_req_mgmt, req);
+ ukplat_spin_unlock_irqrestore(&dev->_req_mgmt.spinlock, flags);
+}
+
struct uk_9pfid *uk_9pdev_fid_create(struct uk_9pdev *dev)
{
struct uk_9pfid *fid = NULL;
diff --git a/lib/uk9p/9preq.c b/lib/uk9p/9preq.c
index d44e684..edc462c 100644
--- a/lib/uk9p/9preq.c
+++ b/lib/uk9p/9preq.c
@@ -35,6 +35,7 @@
#include <string.h>
#include <uk/config.h>
#include <uk/9preq.h>
+#include <uk/9pdev.h>
#include <uk/9p_core.h>
#include <uk/list.h>
#include <uk/refcount.h>
@@ -45,14 +46,8 @@
#include <uk/wait.h>
#endif
-struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a)
+void uk_9preq_init(struct uk_9preq *req)
{
- struct uk_9preq *req;
-
- req = uk_calloc(a, 1, sizeof(*req));
- if (req == NULL)
- return NULL;
-
req->xmit.buf = req->xmit_buf;
req->recv.buf = req->recv_buf;
req->xmit.size = req->recv.size = UK_9P_BUFSIZE;
@@ -69,13 +64,10 @@ struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a)
req->recv.offset = 0;
UK_INIT_LIST_HEAD(&req->_list);
- req->_a = a;
uk_refcount_init(&req->refcount, 1);
#if CONFIG_LIBUKSCHED
uk_waitq_init(&req->wq);
#endif
-
- return req;
}
void uk_9preq_get(struct uk_9preq *req)
@@ -89,7 +81,7 @@ int uk_9preq_put(struct uk_9preq *req)
last = uk_refcount_release(&req->refcount);
if (last)
- uk_free(req->_a, req);
+ uk_9pdev_req_to_freelist(req->_dev, req);
return last;
}
diff --git a/lib/uk9p/include/uk/9pdev.h b/lib/uk9p/include/uk/9pdev.h
index 1225336..560ba8f 100644
--- a/lib/uk9p/include/uk/9pdev.h
+++ b/lib/uk9p/include/uk/9pdev.h
@@ -149,6 +149,16 @@ struct uk_9preq *uk_9pdev_req_lookup(struct uk_9pdev *dev,
uint16_t tag);
*/
int uk_9pdev_req_remove(struct uk_9pdev *dev, struct uk_9preq *req);
+/**
+ * Places the given request on the 9p device's request freelist.
+ *
+ * @param dev
+ * The Unikraft 9P Device. If NULL, doesn't place the request on the
freelist.
+ * @param req
+ * The request to be placed on the freelist.
+ */
+void uk_9pdev_req_to_freelist(struct uk_9pdev *dev, struct uk_9preq *req);
+
/**
* Creates a FID associated with the given 9P device.
*
diff --git a/lib/uk9p/include/uk/9pdev_core.h b/lib/uk9p/include/uk/9pdev_core.h
index 38864ac..fcad1ef 100644
--- a/lib/uk9p/include/uk/9pdev_core.h
+++ b/lib/uk9p/include/uk/9pdev_core.h
@@ -120,6 +120,8 @@ struct uk_9pdev_req_mgmt {
unsigned long tag_bm[UK_BITS_TO_LONGS(UK_9P_NUMTAGS)];
/* List of requests allocated and not yet removed. */
struct uk_list_head req_list;
+ /* Free-list of requests. */
+ struct uk_list_head req_free_list;
};
/**
diff --git a/lib/uk9p/include/uk/9preq.h b/lib/uk9p/include/uk/9preq.h
index ed883d2..aad8d42 100644
--- a/lib/uk9p/include/uk/9preq.h
+++ b/lib/uk9p/include/uk/9preq.h
@@ -162,6 +162,8 @@ struct uk_9preq {
uint16_t tag;
/* Entry into the list of requests (API-internal). */
struct uk_list_head _list;
+ /* @internal 9P device this request belongs to. */
+ struct uk_9pdev *_dev;
/* @internal Allocator used to allocate this request. */
struct uk_alloc *_a;
/* Tracks the number of references to this structure. */
@@ -176,16 +178,10 @@ UK_CTASSERT(sizeof(struct uk_9preq) <= __PAGE_SIZE);
/**
* @internal
- * Allocates a 9p request.
+ * Initializes a 9P request.
* Should not be used directly, use uk_9pdev_req_create() instead.
- *
- * @param a
- * Allocator to use.
- * @return
- * - (==NULL): Out of memory.
- * - (!=NULL): Successful.
*/
-struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a);
+void uk_9preq_init(struct uk_9preq *req);
/**
* Gets the 9p request, incrementing the reference count.
@@ -197,7 +193,8 @@ void uk_9preq_get(struct uk_9preq *req);
/**
* Puts the 9p request, decrementing the reference count.
- * If this was the last live reference, the memory will be freed.
+ * If this was the last live reference, it will be placed on the asociated
+ * device's request freelist.
*
* @param req
* Reference to the 9p request.
--
2.26.2
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |