[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH v4 04/11] lib/uknetdev: Netbuf refcounting and releasing
On 11.10.18 13:17, Sharan Santhanam wrote:
Hello Simon,
Please find the comments inline.
On 10/10/2018 02:00 PM, Simon Kuenzer wrote:
Introduce reference counting and buffer releasing with Netbufs.
Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
lib/uknetdev/exportsyms.uk | 3 ++
lib/uknetdev/include/uk/netbuf.h | 60
++++++++++++++++++++++++++++++++++++++++
lib/uknetdev/netbuf.c | 55
++++++++++++++++++++++++++++++++++++
3 files changed, 118 insertions(+)
diff --git a/lib/uknetdev/exportsyms.uk b/lib/uknetdev/exportsyms.uk
index 3153a5d..591d46c 100644
--- a/lib/uknetdev/exportsyms.uk
+++ b/lib/uknetdev/exportsyms.uk
@@ -2,6 +2,9 @@ uk_netbuf_init_indir
uk_netbuf_alloc_indir
uk_netbuf_alloc_buf
uk_netbuf_prepare_buf
+uk_netbuf_ref
+uk_netbuf_free_single
+uk_netbuf_free
uk_netbuf_disconnect
uk_netbuf_connect
uk_netbuf_append
diff --git a/lib/uknetdev/include/uk/netbuf.h
b/lib/uknetdev/include/uk/netbuf.h
index c448c6a..5b0907c 100644
--- a/lib/uknetdev/include/uk/netbuf.h
+++ b/lib/uknetdev/include/uk/netbuf.h
@@ -283,6 +283,43 @@ struct uk_netbuf *uk_netbuf_prepare_buf(void
*mem, size_t size,
size_t privlen, uk_netbuf_dtor_t dtor);
The function uk_netbuf_ref can also be defined as inline function.
/**
Sure.
+ * Increase reference count of a netbuf including each successive netbuf
+ * element of a chain. Note, preceding chain elements are not visited.
+ * @param m
+ * head of the uk_netbuf chain to increase refcount of
+ * @returns
+ * Reference to m
+ */
+struct uk_netbuf *uk_netbuf_ref(struct uk_netbuf *m);
+
+/**
+ * Increase reference count of a single netbuf (irrespective of a chain)
+ * @param m
+ * uk_netbuf to increase refcount of
+ * @returns
+ * Reference to m
+ */
+static inline struct uk_netbuf *uk_netbuf_ref_single(struct uk_netbuf
*m)
+{
+ UK_ASSERT(m);
+
+ uk_refcount_acquire(&m->refcount);
+ return m;
+}
+
Maybe rename uk_netbuf_refcount_single with uk_netbuf_refcount_single_get
Okay, fine.
+/**
+ * Returns the current reference count of a single netbuf
+ * @param m
+ * uk_netbuf to return the reference count of
+ */
+static inline uint32_t uk_netbuf_refcount_single(struct uk_netbuf *m)
+{
+ UK_ASSERT(m);
+
+ return uk_refcount_read(&m->refcount);
+}
+
+/**
* Connects two netbuf chains
* @param headtail
* Last netbuf element of chain that should come first.
@@ -317,6 +354,29 @@ void uk_netbuf_append(struct uk_netbuf *head,
*/
struct uk_netbuf *uk_netbuf_disconnect(struct uk_netbuf *m);
+/**
+ * Decreases the reference count of each element of the chain.
+ * If a refcount becomes 0, the netbuf is disconnected from its chain,
+ * its destructor is called and the memory is free'd according to its
+ * allocation.
+ * @param m
+ * head of uk_netbuf chain to release
+ * @returns
+ * Reference to m
+ */
+void uk_netbuf_free(struct uk_netbuf *m);
+
+/**
+ * Decreases the reference count of a single netbuf. If refcount
becomes 0,
+ * the netbuf is disconnected from its chain, its destructor is
called and
+ * the memory is free'd according to its allocation.
+ * @param m
+ * uk_netbuf to release
+ * @returns
+ * Reference to m
+ */
+void uk_netbuf_free_single(struct uk_netbuf *m);
+
/*
* Iterator helpers for netbuf chains
*/
diff --git a/lib/uknetdev/netbuf.c b/lib/uknetdev/netbuf.c
index 5bc2312..b624364 100644
--- a/lib/uknetdev/netbuf.c
+++ b/lib/uknetdev/netbuf.c
@@ -189,6 +189,18 @@ struct uk_netbuf *uk_netbuf_prepare_buf(void
*mem, size_t size,
return m;
}
+struct uk_netbuf *uk_netbuf_ref(struct uk_netbuf *m)
+{
+ struct uk_netbuf *iter;
+
+ UK_ASSERT(m);
+
+ UK_NETBUF_CHAIN_FOREACH(iter, m)
+ uk_netbuf_ref_single(iter);
+
+ return m;
+}
+
struct uk_netbuf *uk_netbuf_disconnect(struct uk_netbuf *m)
{
struct uk_netbuf *remhead = NULL;
@@ -243,3 +255,46 @@ void uk_netbuf_append(struct uk_netbuf *head,
headtail->next = tail;
tail->prev = headtail;
}
+
+void uk_netbuf_free_single(struct uk_netbuf *m)
+{
+ struct uk_alloc *a;
+
+ UK_ASSERT(m);
+
+ /* Decrease refcount and call destructor and free up memory
+ * when last reference was released.
+ */
+ if (uk_refcount_release(&m->refcount) == 1) {
+ /* Disconnect this netbuf from the chain. */
Additional note: We need to check in the uk_netbuf_disconnect for
refcount to be one. We should avoid disconnecting buffer with multi-user
reference. In the free function it done correctly but the
uk_netbuf_disconnect is exposed to the user directly.
Probably it wise to add a note on
* uk_netbuf_disconnect
* uk_netbuf_connect
* uk_netbuf_append
regarding the refcount and reference to uk_netbuf
Yes, I will do.
+ uk_netbuf_disconnect(m);
+
+ /* Copy the reference of the allocator in case
+ * the destructor is free'ing up our memory
+ * (e.g., uk_netbuf_init_indir() used).
+ * In such a case `a` should be (NULL), however
+ * we need to access it for a check after we have
+ * called the destructor.
+ */
+ a = m->_a;
+
+ if (m->dtor)
+ m->dtor(m);
+ if (a)
+ uk_free(a, m);
+ }
+}
+
+void uk_netbuf_free(struct uk_netbuf *m)
+{
+ struct uk_netbuf *n;
+
+ UK_ASSERT(m);
+ UK_ASSERT(!m->prev);
+
+ while (m != NULL) {
While freeing the buffers in the chain, there maybe buffers with higher
reference count. We need to atleast put define a way to inform the user
that we have not freed some buffers in the chain.
I will add a debug message.
+ n = m->next;
+ uk_netbuf_free_single(m);
+ m = n;
+ }
+}
Thanks & Regards
Sharan
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
