[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH v2 6/8] lib/uk9p: Add 9P fid abstraction
On 05.07.19 00:01, Simon Kuenzer wrote:
On 29.06.19 10:56, Cristian Banu wrote:
This patch adds the uk_9pfid struct, which abstracts the 4-byte fid
number used by the 9p protocol to associate numbers with filesystem
entries (files, directories, etc.)
This patch also adds a fid cache for each 9p device, enabling reuse of
the fids and reducing the amount of memory allocations done.
Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
---
lib/uk9p/9pdev.c | 121
++++++++++++++++++++++++++++++++++++++-
lib/uk9p/9pfid.c | 71 +++++++++++++++++++++++
lib/uk9p/Makefile.uk | 1 +
lib/uk9p/exportsyms.uk | 9 ++-
lib/uk9p/include/uk/9pdev.h | 23 ++++++++
lib/uk9p/include/uk/9pdev_core.h | 22 ++++++-
lib/uk9p/include/uk/9pfid.h | 115
+++++++++++++++++++++++++++++++++++++
7 files changed, 359 insertions(+), 3 deletions(-)
create mode 100644 lib/uk9p/9pfid.c
create mode 100644 lib/uk9p/include/uk/9pfid.h
diff --git a/lib/uk9p/9pdev.c b/lib/uk9p/9pdev.c
index 05a581eafbdb..11ca4d965893 100644
--- a/lib/uk9p/9pdev.c
+++ b/lib/uk9p/9pdev.c
@@ -45,11 +45,91 @@
#include <uk/9pdev.h>
#include <uk/9pdev_trans.h>
#include <uk/9preq.h>
+#include <uk/9pfid.h>
#if CONFIG_LIBUKSCHED
#include <uk/sched.h>
#include <uk/wait.h>
#endif
+static int _fid_mgmt_init(struct uk_9pdev_fid_mgmt *fid_mgmt)
+{
+ ukarch_spin_lock_init(&fid_mgmt->spinlock);
+ fid_mgmt->next_fid = 0;
+ UK_INIT_LIST_HEAD(&fid_mgmt->fid_free_list);
+ UK_INIT_LIST_HEAD(&fid_mgmt->fid_active_list);
+
+ return 0;
+}
Can be a void function, right?
+
+static int _fid_mgmt_next_fid_locked(struct uk_9pdev_fid_mgmt *fid_mgmt,
+ struct uk_9pdev *dev,
+ struct uk_9pfid **fid)
+{
+ struct uk_9pfid *result = NULL;
+
+ if (!uk_list_empty(&fid_mgmt->fid_free_list)) {
+ result = uk_list_first_entry(&fid_mgmt->fid_free_list,
+ struct uk_9pfid, _list);
+ uk_list_del(&result->_list);
+ } else {
+ result = uk_9pfid_alloc(dev);
+ if (!result)
+ return -ENOMEM;
+ result->fid = fid_mgmt->next_fid++;
+ }
+
+ uk_refcount_init(&result->refcount, 1);
+ result->was_removed = 0;
+ *fid = result;
+
+ return 0;
+}
+
+static int _fid_mgmt_add_fid_locked(struct uk_9pdev_fid_mgmt *fid_mgmt,
+ struct uk_9pfid *fid)
+{
+ uk_list_add(&fid->_list, &fid_mgmt->fid_active_list);
+
+ return 0;
+}
void here, too?
+
+static void _fid_mgmt_del_fid_locked(struct uk_9pdev_fid_mgmt *fid_mgmt,
+ struct uk_9pfid *fid,
+ int move_to_freelist)
+{
+ uk_list_del(&fid->_list);
+
+ if (move_to_freelist)
+ uk_list_add(&fid->_list, &fid_mgmt->fid_free_list);
+ else {
+ /*
+ * Free the memory associated. This fid will never be used
+ * again.
+ */
+ uk_pr_warn("Could not move fid to freelist, freeing memory.\n");
+ uk_free(fid->_dev->a, fid);
+ }
+}
+
+static void _fid_mgmt_cleanup(struct uk_9pdev_fid_mgmt *fid_mgmt)
+{
+ unsigned long flags;
+ struct uk_9pfid *fid, *fidn;
+
+ ukplat_spin_lock_irqsave(&fid_mgmt->spinlock, flags);
+ /*
+ * Every fid should have been clunked *before* destroying the
+ * connection.
+ */
+ UK_ASSERT(uk_list_empty(&fid_mgmt->fid_active_list));
+ uk_list_for_each_entry_safe(fid, fidn, &fid_mgmt->fid_free_list,
+ _list) {
+ uk_list_del(&fid->_list);
+ uk_free(fid->_dev->a, fid);
+ }
+ ukplat_spin_unlock_irqrestore(&fid_mgmt->spinlock, flags);
+}
+
static int _req_mgmt_init(struct uk_9pdev_req_mgmt *req_mgmt)
{
ukarch_spin_lock_init(&req_mgmt->spinlock);
@@ -130,15 +210,21 @@ struct uk_9pdev *uk_9pdev_connect(const struct
uk_9pdev_trans *trans,
if (rc < 0)
goto free_dev;
- rc = dev->ops->connect(dev, device_identifier, mount_args);
+ rc = _fid_mgmt_init(&dev->_fid_mgmt);
if (rc < 0)
goto free_req;
+ rc = dev->ops->connect(dev, device_identifier, mount_args);
+ if (rc < 0)
+ goto free_fid;
+
UK_ASSERT(dev->msize != 0);
dev->state = UK_9PDEV_CONNECTED;
goto out;
+free_fid:
+ _fid_mgmt_cleanup(&dev->_fid_mgmt);
free_req:
_req_mgmt_cleanup(&dev->_req_mgmt);
free_dev:
@@ -159,6 +245,7 @@ int uk_9pdev_disconnect(struct uk_9pdev *dev)
dev->state = UK_9PDEV_DISCONNECTING;
/* Clean up the requests before closing the channel. */
+ _fid_mgmt_cleanup(&dev->_fid_mgmt);
_req_mgmt_cleanup(&dev->_req_mgmt);
rc = dev->ops->disconnect(dev);
@@ -320,6 +407,38 @@ int uk_9pdev_req_remove(struct uk_9pdev *dev,
struct uk_9preq *req)
return uk_9preq_put(req);
}
+struct uk_9pfid *uk_9pdev_fid_create(struct uk_9pdev *dev)
+{
+ struct uk_9pfid *fid = NULL;
+ int rc = 0;
+ unsigned long flags;
+
+ ukplat_spin_lock_irqsave(&dev->_fid_mgmt.spinlock, flags);
+ rc = _fid_mgmt_next_fid_locked(&dev->_fid_mgmt, dev, &fid);
+ if (rc < 0)
+ goto out;
+
+ rc = _fid_mgmt_add_fid_locked(&dev->_fid_mgmt, fid);
+ if (rc < 0)
+ goto out;
+
+out:
+ ukplat_spin_unlock_irqrestore(&dev->_fid_mgmt.spinlock, flags);
+ if (rc == 0)
+ return fid;
+ return ERR2PTR(rc);
+}
+
+void uk_9pdev_fid_release(struct uk_9pfid *fid)
+{
+ struct uk_9pdev *dev = fid->_dev;
+ unsigned long flags;
+
+ ukplat_spin_lock_irqsave(&dev->_fid_mgmt.spinlock, flags);
+ _fid_mgmt_del_fid_locked(&dev->_fid_mgmt, fid, 1);
+ ukplat_spin_unlock_irqrestore(&dev->_fid_mgmt.spinlock, flags);
+}
+
void uk_9pdev_adjust_msize(struct uk_9pdev *dev, uint32_t msize)
{
dev->msize = MIN(dev->msize, msize);
diff --git a/lib/uk9p/9pfid.c b/lib/uk9p/9pfid.c
new file mode 100644
index 000000000000..85d14c66078a
--- /dev/null
+++ b/lib/uk9p/9pfid.c
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Cristian Banu <cristb@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, University Politehnica of Bucharest. All
rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
the
+ * documentation and/or other materials provided with the
distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived
from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#include <uk/config.h>
+#include <uk/refcount.h>
+#include <uk/alloc.h>
+#include <uk/9pdev.h>
+#include <uk/9pfid.h>
+
+struct uk_9pfid *uk_9pfid_alloc(struct uk_9pdev *dev)
+{
+ struct uk_9pfid *fid;
+
+ fid = uk_calloc(dev->a, 1, sizeof(*fid));
+ if (fid == NULL)
+ goto out;
+
+ fid->_dev = dev;
+
+ return fid;
+
+out:
+ return NULL;
+}
+
+void uk_9pfid_get(struct uk_9pfid *fid)
+{
+ uk_refcount_acquire(&fid->refcount);
+}
+
+int uk_9pfid_put(struct uk_9pfid *fid)
+{
+ int last;
+
+ last = uk_refcount_release(&fid->refcount);
+ if (last)
+ uk_9pdev_fid_release(fid);
+
+ return last;
+}
diff --git a/lib/uk9p/Makefile.uk b/lib/uk9p/Makefile.uk
index 34cc987a2f9c..cd4bf4b8a033 100644
--- a/lib/uk9p/Makefile.uk
+++ b/lib/uk9p/Makefile.uk
@@ -6,3 +6,4 @@ CXXINCLUDES-$(CONFIG_LIBUK9P) +=
-I$(LIBUK9P_BASE)/include
LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pdev_trans.c
LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9preq.c
LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pdev.c
+LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pfid.c
diff --git a/lib/uk9p/exportsyms.uk b/lib/uk9p/exportsyms.uk
index c373308fb0a2..5cd7817bf5c8 100644
--- a/lib/uk9p/exportsyms.uk
+++ b/lib/uk9p/exportsyms.uk
@@ -1,6 +1,7 @@
uk_9pdev_trans_register
uk_9pdev_trans_by_name
uk_9pdev_trans_default
+
I guess splitting those entries in groups should probably part of
earlier patches. But this is really minor...
uk_9preq_get
uk_9preq_put
uk_9preq_vserialize
@@ -12,12 +13,18 @@ uk_9preq_copy_from
uk_9preq_receive_cb
uk_9preq_waitreply
uk_9preq_error
+
uk_9pdev_connect
uk_9pdev_disconnect
uk_9pdev_request
uk_9pdev_xmit_ready
uk_9pdev_call
+uk_9pdev_adjust_msize
+
uk_9pdev_req_create
uk_9pdev_req_lookup
uk_9pdev_req_remove
-uk_9pdev_adjust_msize
+
+uk_9pdev_fid_create
+uk_9pfid_get
+uk_9pfid_put
diff --git a/lib/uk9p/include/uk/9pdev.h b/lib/uk9p/include/uk/9pdev.h
index f2210bbffc92..b8f400cea710 100644
--- a/lib/uk9p/include/uk/9pdev.h
+++ b/lib/uk9p/include/uk/9pdev.h
@@ -172,6 +172,29 @@ 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);
/**
+ * Creates a FID associated with the given 9P device.
+ *
+ * @param dev
+ * The Unikraft 9P Device.
+ * @return
+ * If not an error pointer, the created fid.
+ * Otherwise, the error in creating the fid:
+ * - ENOMEM: No memory for the request or no available tags.
+ */
+struct uk_9pfid *uk_9pdev_fid_create(struct uk_9pdev *dev);
+
+/**
+ * @internal
+ * Releases a FID when its reference count goes to 0.
+ *
+ * Should not be called directly, but rather via uk_9pfid_put().
+ *
+ * @param fid
+ * The FID to be released.
+ */
+void uk_9pdev_fid_release(struct uk_9pfid *fid);
+
+/**
* Adjusts the message size to the given maximum size.
*
* @param dev
diff --git a/lib/uk9p/include/uk/9pdev_core.h
b/lib/uk9p/include/uk/9pdev_core.h
index d7ef341e12d6..1c494732337a 100644
--- a/lib/uk9p/include/uk/9pdev_core.h
+++ b/lib/uk9p/include/uk/9pdev_core.h
@@ -124,6 +124,24 @@ struct uk_9pdev_req_mgmt {
/**
* @internal
+ * A structure used to describe the availability of 9P fids.
+ */
+struct uk_9pdev_fid_mgmt {
+ /* Spinlock protecting fids. */
+ spinlock_t spinlock;
+ /* Next available fid. */
+ uint32_t next_fid;
+ /* Free-list of fids that can be reused. */
+ struct uk_list_head fid_free_list;
+ /*
+ * List of fids that are currently active, to be clunked at the
end of
+ * a 9pfs session.
+ */
+ struct uk_list_head fid_active_list;
+};
+
+/**
+ * @internal
* 9PDEV transport state
*
* - CONNECTED: Default state after initialization and during normal
operation.
@@ -150,7 +168,9 @@ struct uk_9pdev {
/* Maximum size of a message. */
uint32_t msize;
/* Transport-allocated data. */
- void *priv;
+ void *priv;
Be careful when your editor changes the whitespaces. Either keep spaces,
or use tabs.
+ /* @internal Fid management. */
+ struct uk_9pdev_fid_mgmt _fid_mgmt;
/* @internal Request management. */
struct uk_9pdev_req_mgmt _req_mgmt;
#if CONFIG_LIBUKSCHED
diff --git a/lib/uk9p/include/uk/9pfid.h b/lib/uk9p/include/uk/9pfid.h
new file mode 100644
index 000000000000..7ec75bb4a508
--- /dev/null
+++ b/lib/uk9p/include/uk/9pfid.h
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Cristian Banu <cristb@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, University Politehnica of Bucharest. All
rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
the
+ * documentation and/or other materials provided with the
distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived
from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#ifndef __UK_9PFID__
+#define __UK_9PFID__
+
+#include <stdbool.h>
+#include <inttypes.h>
+#include <uk/config.h>
+#include <uk/9p_core.h>
+#include <uk/alloc.h>
+#include <uk/essentials.h>
+#include <uk/list.h>
+#include <uk/refcount.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Structure describing a managed fid via reference counting.
+ */
+struct uk_9pfid {
+ /* Fid number. */
+ uint32_t fid;
+ /* Associated server qid. */
+ struct uk_9p_qid qid;
+ /* I/O unit. */
+ uint32_t iounit;
+ /*
+ * If removed, no clunk is necessary, as the remove operation
+ * implicitly clunks the fid.
+ */
+ bool was_removed;
+ /* Tracks the number of live references. */
+ __atomic refcount;
+ /* @internal Associated 9P device. */
+ struct uk_9pdev *_dev;
+ /*
+ * @internal
+ * List on which this fid currently is. See uk_9pdev_fid_mgmt for
+ * details.
+ */
+ struct uk_list_head _list;
+};
+
+/**
+ * @internal
+ * Allocates a 9p fid.
+ * Should not be used directly, use uk_9pdev_fid_create() instead.
+ *
+ * @param a
+ * Allocator to use.
+ * @return
+ * - NULL: Out of memory.
+ * - (!=NULL): Successful.
+ */
+struct uk_9pfid *uk_9pfid_alloc(struct uk_9pdev *dev);
Do you need this function then be exported?
+
+/**
+ * Gets the 9p fid, incrementing the reference count.
+ *
+ * @param fid
+ * Reference to the 9p fid.
+ */
+void uk_9pfid_get(struct uk_9pfid *fid);
+
+/**
+ * Puts the 9p fid, decrementing the reference count.
+ * If this was the last live reference, the memory will be freed.
+ *
+ * @param fid
+ * Reference to the 9p fid.
+ * @return
+ * - 0: This was not the last live reference.
+ * - 1: This was the last live reference.
+ */
+int uk_9pfid_put(struct uk_9pfid *fid);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __UK_9PFID__ */
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
