[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[UNIKRAFT PATCH 3/7] lib/uk9p: Use fixed-size request buffers



Use fixed-size request buffers instead of dynamically allocating new
receive and transmit buffers for each new request. This reduces the
number of calls to uk_alloc from 3 to 1 per each uk_9pdev_req_create()
call.

Allocating a request is faster by roughly 2.5 microseconds. The
previous patch in this patch series shows a 3.7us delay between read
and request_allocated. The excerpt below shows roughly 1us delay.
This performance benefit is observed at a larger scale as well (in
latency and throughput benchmarks).

Excerpt from the output traces for a read() with in-depth traces:

882362702  uk_9p_trace_read               fid 2 offset 61440 count 4096
882363707  uk_9p_trace_request_allocated
882363736  uk_9p_trace_ready              tag 0
882366471  uk_9p_trace_sent               tag 0
882405347  uk_9p_trace_received           tag 0
882405387  uk_9p_trace_read_end           count 4096

Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
---
 lib/uk9p/9p.c               | 24 ++++++------
 lib/uk9p/9pdev.c            | 11 +++---
 lib/uk9p/9preq.c            | 76 ++++++++-----------------------------
 lib/uk9p/include/uk/9pdev.h |  5 +--
 lib/uk9p/include/uk/9preq.h | 31 +++++++++++++--
 5 files changed, 63 insertions(+), 84 deletions(-)

diff --git a/lib/uk9p/9p.c b/lib/uk9p/9p.c
index d6ba3cf..b49ac98 100644
--- a/lib/uk9p/9p.c
+++ b/lib/uk9p/9p.c
@@ -99,7 +99,7 @@ struct uk_9preq *uk_9p_version(struct uk_9pdev *dev,
        uk_9p_trace_version(dev->msize, requested);
        uk_9p_str_init(&requested_str, requested);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TVERSION, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TVERSION);
        if (PTRISERR(req))
                return req;
 
@@ -154,7 +154,7 @@ struct uk_9pfid *uk_9p_attach(struct uk_9pdev *dev, 
uint32_t afid,
 
        uk_9p_trace_attach(fid->fid, afid, uname, aname, n_uname);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TATTACH, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TATTACH);
        if (PTRISERR(req)) {
                uk_9pdev_fid_release(fid);
                return (void *)req;
@@ -193,7 +193,7 @@ int uk_9p_flush(struct uk_9pdev *dev, uint16_t oldtag)
 
        uk_9p_trace_flush(oldtag);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TFLUSH, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TFLUSH);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -234,7 +234,7 @@ struct uk_9pfid *uk_9p_walk(struct uk_9pdev *dev, struct 
uk_9pfid *fid,
        uk_9p_trace_walk(fid->fid, newfid->fid, name ? name : "<NULL>");
        nwname = name ? 1 : 0;
 
-       req = uk_9pdev_req_create(dev, UK_9P_TWALK, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TWALK);
        if (PTRISERR(req)) {
                rc = PTR2ERR(req);
                goto out;
@@ -297,7 +297,7 @@ int uk_9p_open(struct uk_9pdev *dev, struct uk_9pfid *fid, 
uint8_t mode)
        int rc = 0;
 
        uk_9p_trace_open(fid->fid, mode);
-       req = uk_9pdev_req_create(dev, UK_9P_TOPEN, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TOPEN);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -338,7 +338,7 @@ int uk_9p_create(struct uk_9pdev *dev, struct uk_9pfid *fid,
        uk_9p_str_init(&name_str, name);
        uk_9p_str_init(&extension_str, extension);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TCREATE, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TCREATE);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -376,7 +376,7 @@ int uk_9p_remove(struct uk_9pdev *dev, struct uk_9pfid *fid)
        /* The fid is considered invalid even if the remove fails. */
        fid->was_removed = 1;
 
-       req = uk_9pdev_req_create(dev, UK_9P_TREMOVE, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TREMOVE);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -405,7 +405,7 @@ int uk_9p_clunk(struct uk_9pdev *dev, struct uk_9pfid *fid)
        if (fid->was_removed)
                return 0;
 
-       req = uk_9pdev_req_create(dev, UK_9P_TCLUNK, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TCLUNK);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -436,7 +436,7 @@ int64_t uk_9p_read(struct uk_9pdev *dev, struct uk_9pfid 
*fid,
                count = MIN(count, fid->iounit);
        count = MIN(count, dev->msize - 11);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TREAD, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TREAD);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -474,7 +474,7 @@ int64_t uk_9p_write(struct uk_9pdev *dev, struct uk_9pfid 
*fid,
        count = MIN(count, fid->iounit);
        count = MIN(count, dev->msize - 23);
 
-       req = uk_9pdev_req_create(dev, UK_9P_TWRITE, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TWRITE);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
@@ -509,7 +509,7 @@ struct uk_9preq *uk_9p_stat(struct uk_9pdev *dev, struct 
uk_9pfid *fid,
        uint16_t dummy;
 
        uk_9p_trace_stat(fid->fid);
-       req = uk_9pdev_req_create(dev, UK_9P_TSTAT, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TSTAT);
        if (PTRISERR(req))
                return req;
 
@@ -542,7 +542,7 @@ int uk_9p_wstat(struct uk_9pdev *dev, struct uk_9pfid *fid,
        uint16_t *dummy;
 
        uk_9p_trace_wstat(fid->fid);
-       req = uk_9pdev_req_create(dev, UK_9P_TWSTAT, __PAGE_SIZE);
+       req = uk_9pdev_req_create(dev, UK_9P_TWSTAT);
        if (PTRISERR(req))
                return PTR2ERR(req);
 
diff --git a/lib/uk9p/9pdev.c b/lib/uk9p/9pdev.c
index 1d7e09b..40a4daa 100644
--- a/lib/uk9p/9pdev.c
+++ b/lib/uk9p/9pdev.c
@@ -288,8 +288,7 @@ void uk_9pdev_xmit_notify(struct uk_9pdev *dev)
 #endif
 }
 
-struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type,
-                               uint32_t size)
+struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type)
 {
        struct uk_9preq *req;
        int rc = 0;
@@ -298,14 +297,16 @@ struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev 
*dev, uint8_t type,
 
        UK_ASSERT(dev);
 
-       size = MIN(size, dev->msize);
-
-       req = uk_9preq_alloc(dev->a, size);
+       req = uk_9preq_alloc(dev->a);
        if (req == NULL) {
                rc = -ENOMEM;
                goto out;
        }
 
+       /* 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;
diff --git a/lib/uk9p/9preq.c b/lib/uk9p/9preq.c
index 997f772..d44e684 100644
--- a/lib/uk9p/9preq.c
+++ b/lib/uk9p/9preq.c
@@ -45,51 +45,28 @@
 #include <uk/wait.h>
 #endif
 
-static int _fcall_alloc(struct uk_alloc *a, struct uk_9preq_fcall *f,
-                       uint32_t size)
-{
-       UK_ASSERT(a);
-       UK_ASSERT(f);
-       UK_ASSERT(size > 0);
-
-       f->buf = uk_calloc(a, size, sizeof(char));
-       if (f->buf == NULL)
-               return -ENOMEM;
-
-       f->size = size;
-       f->offset = 0;
-       f->zc_buf = NULL;
-       f->zc_size = 0;
-       f->zc_offset = 0;
-
-       return 0;
-}
-
-static void _fcall_free(struct uk_alloc *a, struct uk_9preq_fcall *f)
-{
-       UK_ASSERT(a);
-       UK_ASSERT(f);
-
-       if (f->buf)
-               uk_free(a, f->buf);
-}
-
-struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a, uint32_t size)
+struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a)
 {
        struct uk_9preq *req;
-       int rc;
 
        req = uk_calloc(a, 1, sizeof(*req));
        if (req == NULL)
-               goto out;
+               return NULL;
 
-       rc = _fcall_alloc(a, &req->xmit, size);
-       if (rc < 0)
-               goto out_free;
+       req->xmit.buf = req->xmit_buf;
+       req->recv.buf = req->recv_buf;
+       req->xmit.size = req->recv.size = UK_9P_BUFSIZE;
+       req->xmit.zc_buf = req->recv.zc_buf = NULL;
+       req->xmit.zc_size = req->recv.zc_size = 0;
+       req->xmit.zc_offset = req->recv.zc_offset = 0;
 
-       rc = _fcall_alloc(a, &req->recv, MAX(size, UK_9P_RERROR_MAXSIZE));
-       if (rc < 0)
-               goto out_free;
+       /*
+        * Assume the header has already been written.
+        * The header itself will be written on uk_9preq_ready(), when the
+        * actual message size is known.
+        */
+       req->xmit.offset = UK_9P_HEADER_SIZE;
+       req->recv.offset = 0;
 
        UK_INIT_LIST_HEAD(&req->_list);
        req->_a = a;
@@ -98,28 +75,7 @@ struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a, uint32_t 
size)
        uk_waitq_init(&req->wq);
 #endif
 
-       /*
-        * Assume the header has already been written.
-        * The header itself will be written on uk_9preq_ready(), when the
-        * actual message size is known.
-        */
-       req->xmit.offset = UK_9P_HEADER_SIZE;
-
        return req;
-
-out_free:
-       _fcall_free(a, &req->recv);
-       _fcall_free(a, &req->xmit);
-       uk_free(a, req);
-out:
-       return NULL;
-}
-
-static void _req_free(struct uk_9preq *req)
-{
-       _fcall_free(req->_a, &req->recv);
-       _fcall_free(req->_a, &req->xmit);
-       uk_free(req->_a, req);
 }
 
 void uk_9preq_get(struct uk_9preq *req)
@@ -133,7 +89,7 @@ int uk_9preq_put(struct uk_9preq *req)
 
        last = uk_refcount_release(&req->refcount);
        if (last)
-               _req_free(req);
+               uk_free(req->_a, req);
 
        return last;
 }
diff --git a/lib/uk9p/include/uk/9pdev.h b/lib/uk9p/include/uk/9pdev.h
index 04ff523..1225336 100644
--- a/lib/uk9p/include/uk/9pdev.h
+++ b/lib/uk9p/include/uk/9pdev.h
@@ -114,15 +114,12 @@ void uk_9pdev_xmit_notify(struct uk_9pdev *dev);
  *   The Unikraft 9P Device.
  * @param type
  *   Transmit type of the request, e.g. Tversion, Tread, and so on.
- * @param size
- *   The maximum size for the receive and send buffers.
  * @return
  *   If not an error pointer, the created request.
  *   Otherwise, the error in creating the request:
  *   - ENOMEM: No memory for the request or no available tags.
  */
-struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type,
-                               uint32_t size);
+struct uk_9preq *uk_9pdev_req_create(struct uk_9pdev *dev, uint8_t type);
 
 /**
  * Looks up a request based on the given tag. This is generally used by
diff --git a/lib/uk9p/include/uk/9preq.h b/lib/uk9p/include/uk/9preq.h
index b9713e2..ed883d2 100644
--- a/lib/uk9p/include/uk/9preq.h
+++ b/lib/uk9p/include/uk/9preq.h
@@ -63,6 +63,16 @@ extern "C" {
  */
 #define UK_9P_RERROR_MAXSIZE            141U
 
+/*
+ * The transmit and receive buffer size.
+ *
+ * The buffer size is not expected to exceed 1K: reads and writes are
+ * zero-copy; on-the-wire size of a stat structure should not exceed
+ * 1K -- its base size is 61, along with the lengths for name, uid,
+ * gid, muid and extension strings.
+ */
+#define UK_9P_BUFSIZE                  1024U
+
 /**
  * @internal
  *
@@ -125,8 +135,23 @@ enum uk_9preq_state {
  *  referenced anymore. A call to uk_9pdev_req_remove() is mandatory to
  *  correctly free this and remove it from the list of requests managed
  *  by the 9p device.
+ *
+ *  Should fit within one page.
  */
 struct uk_9preq {
+       /*
+        * Fixed-size buffer for transmit, used for most messages.
+        * Large messages will always zero-copy from the user-provided
+        * buffer (on Twrite).
+        */
+       uint8_t                         xmit_buf[UK_9P_BUFSIZE];
+       /*
+        * Fixed-size buffer for receive, used for most messages.
+        * Large messages will always zero-copy into the user-provided
+        * buffer (on Tread).
+        */
+       uint8_t                         recv_buf[UK_9P_BUFSIZE];
+       /* 2 KB offset in the structure here. */
        /* Transmit fcall. */
        struct uk_9preq_fcall           xmit;
        /* Receive fcall. */
@@ -147,6 +172,8 @@ struct uk_9preq {
 #endif
 };
 
+UK_CTASSERT(sizeof(struct uk_9preq) <= __PAGE_SIZE);
+
 /**
  * @internal
  * Allocates a 9p request.
@@ -154,13 +181,11 @@ struct uk_9preq {
  *
  * @param a
  *   Allocator to use.
- * @param size
- *   Minimum size of the receive and transmit buffers.
  * @return
  *   - (==NULL): Out of memory.
  *   - (!=NULL): Successful.
  */
-struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a, uint32_t size);
+struct uk_9preq *uk_9preq_alloc(struct uk_alloc *a);
 
 /**
  * Gets the 9p request, incrementing the reference count.
-- 
2.26.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.