|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [UNIKRAFT PATCH, v2, 02/15] lib/ukring: {ring.c, ring.h} Fix checkpatch errors and spacing.
Converting tabs to spaces and increasing readability.
Also fixes checkpatch errors:
* return is not a function, parentheses are not required
* trailing whitespace
* space required before the open parenthesis '('
Signed-off-by: Alexander Jung <alexander.jung@xxxxxxxxx>
---
lib/ukring/include/uk/ring.h | 420 ++++++++++++++++++-----------------
lib/ukring/ring.c | 35 +--
2 files changed, 236 insertions(+), 219 deletions(-)
diff --git a/lib/ukring/include/uk/ring.h b/lib/ukring/include/uk/ring.h
index 899563c..4f0d80c 100644
--- a/lib/ukring/include/uk/ring.h
+++ b/lib/ukring/include/uk/ring.h
@@ -29,8 +29,8 @@
*
*/
-#ifndef _SYS_BUF_RING_H_
-#define _SYS_BUF_RING_H_
+#ifndef _SYS_BUF_RING_H_
+#define _SYS_BUF_RING_H_
#include <machine/cpu.h>
@@ -39,22 +39,24 @@
#include <sys/mutex.h>
#endif
+
struct buf_ring {
- volatile uint32_t br_prod_head;
- volatile uint32_t br_prod_tail;
- int br_prod_size;
- int br_prod_mask;
- uint64_t br_drops;
- volatile uint32_t br_cons_head __aligned(CACHE_LINE_SIZE);
- volatile uint32_t br_cons_tail;
- int br_cons_size;
- int br_cons_mask;
+ volatile uint32_t br_prod_head;
+ volatile uint32_t br_prod_tail;
+ int br_prod_size;
+ int br_prod_mask;
+ uint64_t br_drops;
+ volatile uint32_t br_cons_head __aligned(CACHE_LINE_SIZE);
+ volatile uint32_t br_cons_tail;
+ int br_cons_size;
+ int br_cons_mask;
#ifdef DEBUG_BUFRING
- struct mtx *br_lock;
-#endif
- void *br_ring[0] __aligned(CACHE_LINE_SIZE);
+ struct mtx *br_lock;
+#endif
+ void *br_ring[0] __aligned(CACHE_LINE_SIZE);
};
+
/*
* multi-producer safe lock-free ring buffer enqueue
*
@@ -62,54 +64,60 @@ struct buf_ring {
static __inline int
buf_ring_enqueue(struct buf_ring *br, void *buf)
{
- uint32_t prod_head, prod_next, cons_tail;
+ uint32_t prod_head, prod_next, cons_tail;
+
#ifdef DEBUG_BUFRING
- int i;
-
- /*
- * Note: It is possible to encounter an mbuf that was removed
- * via drbr_peek(), and then re-added via drbr_putback() and
- * trigger a spurious panic.
- */
- for (i = br->br_cons_head; i != br->br_prod_head;
- i = ((i + 1) & br->br_cons_mask))
- if(br->br_ring[i] == buf)
- panic("buf=%p already enqueue at %d prod=%d cons=%d",
- buf, i, br->br_prod_tail, br->br_cons_tail);
-#endif
- critical_enter();
- do {
- prod_head = br->br_prod_head;
- prod_next = (prod_head + 1) & br->br_prod_mask;
- cons_tail = br->br_cons_tail;
-
- if (prod_next == cons_tail) {
- rmb();
- if (prod_head == br->br_prod_head &&
- cons_tail == br->br_cons_tail) {
- br->br_drops++;
- critical_exit();
- return (ENOBUFS);
- }
- continue;
- }
- } while (!atomic_cmpset_acq_int(&br->br_prod_head, prod_head,
prod_next));
+ int i;
+
+ /*
+ * Note: It is possible to encounter an mbuf that was removed
+ * via drbr_peek(), and then re-added via drbr_putback() and
+ * trigger a spurious panic.
+ */
+ for (i = br->br_cons_head; i != br->br_prod_head;
+ i = ((i + 1) & br->br_cons_mask))
+ if (br->br_ring[i] == buf)
+ panic("buf=%p already enqueue at %d prod=%d cons=%d",
+ buf, i, br->br_prod_tail, br->br_cons_tail);
+#endif
+
+ critical_enter();
+
+ do {
+ prod_head = br->br_prod_head;
+ prod_next = (prod_head + 1) & br->br_prod_mask;
+ cons_tail = br->br_cons_tail;
+
+ if (prod_next == cons_tail) {
+ rmb();
+ if (prod_head == br->br_prod_head && cons_tail == br->br_cons_tail) {
+ br->br_drops++;
+ critical_exit();
+ return ENOBUFS;
+ }
+ continue;
+ }
+ } while (!atomic_cmpset_acq_int(&br->br_prod_head, prod_head, prod_next));
+
#ifdef DEBUG_BUFRING
- if (br->br_ring[prod_head] != NULL)
- panic("dangling value in enqueue");
-#endif
- br->br_ring[prod_head] = buf;
-
- /*
- * If there are other enqueues in progress
- * that preceded us, we need to wait for them
- * to complete
- */
- while (br->br_prod_tail != prod_head)
- cpu_spinwait();
- atomic_store_rel_int(&br->br_prod_tail, prod_next);
- critical_exit();
- return (0);
+ if (br->br_ring[prod_head] != NULL)
+ panic("dangling value in enqueue");
+#endif
+
+ br->br_ring[prod_head] = buf;
+
+ /*
+ * If there are other enqueues in progress
+ * that preceded us, we need to wait for them
+ * to complete
+ */
+ while (br->br_prod_tail != prod_head)
+ cpu_spinwait();
+
+ atomic_store_rel_int(&br->br_prod_tail, prod_next);
+ critical_exit();
+
+ return 0;
}
/*
@@ -119,36 +127,39 @@ buf_ring_enqueue(struct buf_ring *br, void *buf)
static __inline void *
buf_ring_dequeue_mc(struct buf_ring *br)
{
- uint32_t cons_head, cons_next;
- void *buf;
+ uint32_t cons_head, cons_next;
+ void *buf;
+
+ critical_enter();
+
+ do {
+ cons_head = br->br_cons_head;
+ cons_next = (cons_head + 1) & br->br_cons_mask;
- critical_enter();
- do {
- cons_head = br->br_cons_head;
- cons_next = (cons_head + 1) & br->br_cons_mask;
+ if (cons_head == br->br_prod_tail) {
+ critical_exit();
+ return NULL;
+ }
+ } while (!atomic_cmpset_acq_int(&br->br_cons_head, cons_head, cons_next));
- if (cons_head == br->br_prod_tail) {
- critical_exit();
- return (NULL);
- }
- } while (!atomic_cmpset_acq_int(&br->br_cons_head, cons_head,
cons_next));
+ buf = br->br_ring[cons_head];
- buf = br->br_ring[cons_head];
#ifdef DEBUG_BUFRING
- br->br_ring[cons_head] = NULL;
+ br->br_ring[cons_head] = NULL;
#endif
- /*
- * If there are other dequeues in progress
- * that preceded us, we need to wait for them
- * to complete
- */
- while (br->br_cons_tail != cons_head)
- cpu_spinwait();
-
- atomic_store_rel_int(&br->br_cons_tail, cons_next);
- critical_exit();
-
- return (buf);
+
+ /*
+ * If there are other dequeues in progress
+ * that preceded us, we need to wait for them
+ * to complete
+ */
+ while (br->br_cons_tail != cons_head)
+ cpu_spinwait();
+
+ atomic_store_rel_int(&br->br_cons_tail, cons_next);
+ critical_exit();
+
+ return buf;
}
/*
@@ -159,72 +170,76 @@ buf_ring_dequeue_mc(struct buf_ring *br)
static __inline void *
buf_ring_dequeue_sc(struct buf_ring *br)
{
- uint32_t cons_head, cons_next;
+ uint32_t cons_head, cons_next;
#ifdef PREFETCH_DEFINED
- uint32_t cons_next_next;
+ uint32_t cons_next_next;
#endif
- uint32_t prod_tail;
- void *buf;
-
- /*
- * This is a workaround to allow using buf_ring on ARM and ARM64.
- * ARM64TODO: Fix buf_ring in a generic way.
- * REMARKS: It is suspected that br_cons_head does not require
- * load_acq operation, but this change was extensively tested
- * and confirmed it's working. To be reviewed once again in
- * FreeBSD-12.
- *
- * Preventing following situation:
- * Core(0) - buf_ring_enqueue()
Core(1) - buf_ring_dequeue_sc()
- * -----------------------------------------
----------------------------------------------
- *
- *
cons_head = br->br_cons_head;
- * atomic_cmpset_acq_32(&br->br_prod_head, ...));
- *
buf = br->br_ring[cons_head]; <see <1>>
- * br->br_ring[prod_head] = buf;
- * atomic_store_rel_32(&br->br_prod_tail, ...);
- *
prod_tail = br->br_prod_tail;
- *
if (cons_head == prod_tail)
- *
return (NULL);
- *
<condition is false and code uses invalid(old) buf>`
- *
- * <1> Load (on core 1) from br->br_ring[cons_head] can be reordered
(speculative readed) by CPU.
- */
+ uint32_t prod_tail;
+ void *buf;
+
+ /*
+ * This is a workaround to allow using buf_ring on ARM and ARM64.
+ * ARM64TODO: Fix buf_ring in a generic way.
+ * REMARKS: It is suspected that br_cons_head does not require
+ * load_acq operation, but this change was extensively tested
+ * and confirmed it's working. To be reviewed once again in
+ * FreeBSD-12.
+ *
+ * Preventing following situation:
+ * Core(0) - buf_ring_enqueue()
Core(1) - buf_ring_dequeue_sc()
+ * -----------------------------------------
----------------------------------------------
+ *
+ *
cons_head = br->br_cons_head;
+ * atomic_cmpset_acq_32(&br->br_prod_head, ...));
+ *
buf = br->br_ring[cons_head]; <see <1>>
+ * br->br_ring[prod_head] = buf;
+ * atomic_store_rel_32(&br->br_prod_tail, ...);
+ *
prod_tail = br->br_prod_tail;
+ *
if (cons_head == prod_tail)
+ *
return (NULL);
+ *
<condition is false and code uses invalid(old) buf>`
+ *
+ * <1> Load (on core 1) from br->br_ring[cons_head] can be reordered
(speculative readed) by CPU.
+ */
#if defined(__arm__) || defined(__aarch64__)
- cons_head = atomic_load_acq_32(&br->br_cons_head);
+ cons_head = atomic_load_acq_32(&br->br_cons_head);
#else
- cons_head = br->br_cons_head;
+ cons_head = br->br_cons_head;
#endif
- prod_tail = atomic_load_acq_32(&br->br_prod_tail);
-
- cons_next = (cons_head + 1) & br->br_cons_mask;
+ prod_tail = atomic_load_acq_32(&br->br_prod_tail);
+
+ cons_next = (cons_head + 1) & br->br_cons_mask;
#ifdef PREFETCH_DEFINED
- cons_next_next = (cons_head + 2) & br->br_cons_mask;
+ cons_next_next = (cons_head + 2) & br->br_cons_mask;
#endif
-
- if (cons_head == prod_tail)
- return (NULL);
-
-#ifdef PREFETCH_DEFINED
- if (cons_next != prod_tail) {
- prefetch(br->br_ring[cons_next]);
- if (cons_next_next != prod_tail)
- prefetch(br->br_ring[cons_next_next]);
- }
+
+ if (cons_head == prod_tail)
+ return NULL;
+
+#ifdef PREFETCH_DEFINED
+ if (cons_next != prod_tail) {
+ prefetch(br->br_ring[cons_next]);
+ if (cons_next_next != prod_tail)
+ prefetch(br->br_ring[cons_next_next]);
+ }
#endif
- br->br_cons_head = cons_next;
- buf = br->br_ring[cons_head];
+
+ br->br_cons_head = cons_next;
+ buf = br->br_ring[cons_head];
#ifdef DEBUG_BUFRING
- br->br_ring[cons_head] = NULL;
- if (!mtx_owned(br->br_lock))
- panic("lock not held on single consumer dequeue");
- if (br->br_cons_tail != cons_head)
- panic("inconsistent list cons_tail=%d cons_head=%d",
- br->br_cons_tail, cons_head);
+ br->br_ring[cons_head] = NULL;
+
+ if (!mtx_owned(br->br_lock))
+ panic("lock not held on single consumer dequeue");
+
+ if (br->br_cons_tail != cons_head)
+ panic("inconsistent list cons_tail=%d cons_head=%d",
+ br->br_cons_tail, cons_head);
#endif
- br->br_cons_tail = cons_next;
- return (buf);
+
+ br->br_cons_tail = cons_next;
+ return buf;
}
/*
@@ -235,20 +250,23 @@ buf_ring_dequeue_sc(struct buf_ring *br)
static __inline void
buf_ring_advance_sc(struct buf_ring *br)
{
- uint32_t cons_head, cons_next;
- uint32_t prod_tail;
-
- cons_head = br->br_cons_head;
- prod_tail = br->br_prod_tail;
-
- cons_next = (cons_head + 1) & br->br_cons_mask;
- if (cons_head == prod_tail)
- return;
- br->br_cons_head = cons_next;
+ uint32_t cons_head, cons_next;
+ uint32_t prod_tail;
+
+ cons_head = br->br_cons_head;
+ prod_tail = br->br_prod_tail;
+ cons_next = (cons_head + 1) & br->br_cons_mask;
+
+ if (cons_head == prod_tail)
+ return;
+
+ br->br_cons_head = cons_next;
+
#ifdef DEBUG_BUFRING
- br->br_ring[cons_head] = NULL;
+ br->br_ring[cons_head] = NULL;
#endif
- br->br_cons_tail = cons_next;
+
+ br->br_cons_tail = cons_next;
}
/*
@@ -270,9 +288,9 @@ buf_ring_advance_sc(struct buf_ring *br)
static __inline void
buf_ring_putback_sc(struct buf_ring *br, void *new)
{
- KASSERT(br->br_cons_head != br->br_prod_tail,
- ("Buf-Ring has none in putback")) ;
- br->br_ring[br->br_cons_head] = new;
+ KASSERT(br->br_cons_head != br->br_prod_tail,
+ ("Buf-Ring has none in putback"));
+ br->br_ring[br->br_cons_head] = new;
}
/*
@@ -283,89 +301,85 @@ buf_ring_putback_sc(struct buf_ring *br, void *new)
static __inline void *
buf_ring_peek(struct buf_ring *br)
{
-
#ifdef DEBUG_BUFRING
- if ((br->br_lock != NULL) && !mtx_owned(br->br_lock))
- panic("lock not held on single consumer dequeue");
-#endif
- /*
- * I believe it is safe to not have a memory barrier
- * here because we control cons and tail is worst case
- * a lagging indicator so we worst case we might
- * return NULL immediately after a buffer has been enqueued
- */
- if (br->br_cons_head == br->br_prod_tail)
- return (NULL);
-
- return (br->br_ring[br->br_cons_head]);
+ if ((br->br_lock != NULL) && !mtx_owned(br->br_lock))
+ panic("lock not held on single consumer dequeue");
+#endif
+
+ /*
+ * I believe it is safe to not have a memory barrier
+ * here because we control cons and tail is worst case
+ * a lagging indicator so we worst case we might
+ * return NULL immediately after a buffer has been enqueued
+ */
+ if (br->br_cons_head == br->br_prod_tail)
+ return NULL;
+
+ return br->br_ring[br->br_cons_head];
}
static __inline void *
buf_ring_peek_clear_sc(struct buf_ring *br)
{
#ifdef DEBUG_BUFRING
- void *ret;
+ void *ret;
- if (!mtx_owned(br->br_lock))
- panic("lock not held on single consumer dequeue");
-#endif
+ if (!mtx_owned(br->br_lock))
+ panic("lock not held on single consumer dequeue");
+#endif
- if (br->br_cons_head == br->br_prod_tail)
- return (NULL);
+ if (br->br_cons_head == br->br_prod_tail)
+ return NULL;
#if defined(__arm__) || defined(__aarch64__)
- /*
- * The barrier is required there on ARM and ARM64 to ensure, that
- * br->br_ring[br->br_cons_head] will not be fetched before the above
- * condition is checked.
- * Without the barrier, it is possible, that buffer will be fetched
- * before the enqueue will put mbuf into br, then, in the meantime, the
- * enqueue will update the array and the br_prod_tail, and the
- * conditional check will be true, so we will return previously fetched
- * (and invalid) buffer.
- */
- atomic_thread_fence_acq();
+ /*
+ * The barrier is required there on ARM and ARM64 to ensure, that
+ * br->br_ring[br->br_cons_head] will not be fetched before the above
+ * condition is checked.
+ * Without the barrier, it is possible, that buffer will be fetched
+ * before the enqueue will put mbuf into br, then, in the meantime, the
+ * enqueue will update the array and the br_prod_tail, and the
+ * conditional check will be true, so we will return previously fetched
+ * (and invalid) buffer.
+ */
+ atomic_thread_fence_acq();
#endif
#ifdef DEBUG_BUFRING
- /*
- * Single consumer, i.e. cons_head will not move while we are
- * running, so atomic_swap_ptr() is not necessary here.
- */
- ret = br->br_ring[br->br_cons_head];
- br->br_ring[br->br_cons_head] = NULL;
- return (ret);
+ /*
+ * Single consumer, i.e. cons_head will not move while we are
+ * running, so atomic_swap_ptr() is not necessary here.
+ */
+ ret = br->br_ring[br->br_cons_head];
+ br->br_ring[br->br_cons_head] = NULL;
+
+ return ret;
#else
- return (br->br_ring[br->br_cons_head]);
+ return br->br_ring[br->br_cons_head];
#endif
}
static __inline int
buf_ring_full(struct buf_ring *br)
{
-
- return (((br->br_prod_head + 1) & br->br_prod_mask) ==
br->br_cons_tail);
+ return ((br->br_prod_head + 1) & br->br_prod_mask) == br->br_cons_tail;
}
static __inline int
buf_ring_empty(struct buf_ring *br)
{
-
- return (br->br_cons_head == br->br_prod_tail);
+ return br->br_cons_head == br->br_prod_tail;
}
static __inline int
buf_ring_count(struct buf_ring *br)
{
-
- return ((br->br_prod_size + br->br_prod_tail - br->br_cons_tail)
- & br->br_prod_mask);
+ return (br->br_prod_size + br->br_prod_tail - br->br_cons_tail)
+ & br->br_prod_mask;
}
struct buf_ring *buf_ring_alloc(int count, struct malloc_type *type, int flags,
struct mtx *);
void buf_ring_free(struct buf_ring *br, struct malloc_type *type);
-
-
#endif
\ No newline at end of file
diff --git a/lib/ukring/ring.c b/lib/ukring/ring.c
index 644c3be..ed5e8f7 100644
--- a/lib/ukring/ring.c
+++ b/lib/ukring/ring.c
@@ -39,27 +39,30 @@ __FBSDID("$FreeBSD$");
struct buf_ring *
buf_ring_alloc(int count, struct malloc_type *type, int flags, struct mtx
*lock)
{
- struct buf_ring *br;
+ struct buf_ring *br;
+
+ KASSERT(powerof2(count), ("buf ring must be size power of 2"));
+
+ br = malloc(sizeof(struct buf_ring) + count*sizeof(caddr_t),
+ type, flags|M_ZERO);
+
+ if (br == NULL)
+ return NULL;
- KASSERT(powerof2(count), ("buf ring must be size power of 2"));
-
- br = malloc(sizeof(struct buf_ring) + count*sizeof(caddr_t),
- type, flags|M_ZERO);
- if (br == NULL)
- return (NULL);
#ifdef DEBUG_BUFRING
- br->br_lock = lock;
-#endif
- br->br_prod_size = br->br_cons_size = count;
- br->br_prod_mask = br->br_cons_mask = count-1;
- br->br_prod_head = br->br_cons_head = 0;
- br->br_prod_tail = br->br_cons_tail = 0;
-
- return (br);
+ br->br_lock = lock;
+#endif
+
+ br->br_prod_size = br->br_cons_size = count;
+ br->br_prod_mask = br->br_cons_mask = count - 1;
+ br->br_prod_head = br->br_cons_head = 0;
+ br->br_prod_tail = br->br_cons_tail = 0;
+
+ return br;
}
void
buf_ring_free(struct buf_ring *br, struct malloc_type *type)
{
- free(br, type);
+ free(br, type);
}
\ No newline at end of file
--
2.20.1
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |