[dpdk-dev,v1,02/14] ring: create common structure for prod and cons metadata

Message ID 20170223172407.27664-3-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Bruce Richardson Feb. 23, 2017, 5:23 p.m. UTC
  create a common structure to hold the metadata for the producer and
the consumer, since both need essentially the same information - the
head and tail values, the ring size and mask.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_ring/rte_ring.h | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)
  

Comments

Olivier Matz March 1, 2017, 10:22 a.m. UTC | #1
On Thu, 23 Feb 2017 17:23:55 +0000, Bruce Richardson
<bruce.richardson@intel.com> wrote:
> create a common structure to hold the metadata for the producer and
> the consumer, since both need essentially the same information - the
> head and tail values, the ring size and mask.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_ring/rte_ring.h | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index 04fe667..0c8defd 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or
> without @@ -139,6 +139,19 @@ struct rte_ring_debug_stats {
>  
>  struct rte_memzone; /* forward declaration, so as not to require
> memzone.h */ 
> +/* structure to hold a pair of head/tail values and other metadata */
> +struct rte_ring_ht_ptr {

Just wondering if we can find a better name for this structure. I'm not
sure '_ptr' is really relevant. What do you think of:

rte_ring_endpoint
rte_ring_ht
rte_ring_headtail



Olivier
  
Bruce Richardson March 1, 2017, 10:33 a.m. UTC | #2
On Wed, Mar 01, 2017 at 11:22:43AM +0100, Olivier Matz wrote:
> On Thu, 23 Feb 2017 17:23:55 +0000, Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > create a common structure to hold the metadata for the producer and
> > the consumer, since both need essentially the same information - the
> > head and tail values, the ring size and mask.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/librte_ring/rte_ring.h | 32 ++++++++++++++++----------------
> >  1 file changed, 16 insertions(+), 16 deletions(-)
> > 
> > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> > index 04fe667..0c8defd 100644
> > --- a/lib/librte_ring/rte_ring.h
> > +++ b/lib/librte_ring/rte_ring.h
> > @@ -1,7 +1,7 @@
> >  /*-
> >   *   BSD LICENSE
> >   *
> > - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> > + *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
> >   *   All rights reserved.
> >   *
> >   *   Redistribution and use in source and binary forms, with or
> > without @@ -139,6 +139,19 @@ struct rte_ring_debug_stats {
> >  
> >  struct rte_memzone; /* forward declaration, so as not to require
> > memzone.h */ 
> > +/* structure to hold a pair of head/tail values and other metadata */
> > +struct rte_ring_ht_ptr {
> 
> Just wondering if we can find a better name for this structure. I'm not
> sure '_ptr' is really relevant. What do you think of:
> 
> rte_ring_endpoint
> rte_ring_ht
> rte_ring_headtail
> 
I'll use one of the latter two in next version.

/Bruce
  

Patch

diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index 04fe667..0c8defd 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -1,7 +1,7 @@ 
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -139,6 +139,19 @@  struct rte_ring_debug_stats {
 
 struct rte_memzone; /* forward declaration, so as not to require memzone.h */
 
+/* structure to hold a pair of head/tail values and other metadata */
+struct rte_ring_ht_ptr {
+	volatile uint32_t head;  /**< Prod/consumer head. */
+	volatile uint32_t tail;  /**< Prod/consumer tail. */
+	uint32_t size;           /**< Size of ring. */
+	uint32_t mask;           /**< Mask (size-1) of ring. */
+	union {
+		uint32_t sp_enqueue; /**< True, if single producer. */
+		uint32_t sc_dequeue; /**< True, if single consumer. */
+	};
+	uint32_t watermark;      /**< Max items before EDQUOT in producer. */
+};
+
 /**
  * An RTE ring structure.
  *
@@ -161,23 +174,10 @@  struct rte_ring {
 			/**< Memzone, if any, containing the rte_ring */
 
 	/** Ring producer status. */
-	struct prod {
-		uint32_t watermark;      /**< Maximum items before EDQUOT. */
-		uint32_t sp_enqueue;     /**< True, if single producer. */
-		uint32_t size;           /**< Size of ring. */
-		uint32_t mask;           /**< Mask (size-1) of ring. */
-		volatile uint32_t head;  /**< Producer head. */
-		volatile uint32_t tail;  /**< Producer tail. */
-	} prod __rte_aligned(RTE_CACHE_LINE_SIZE * 2);
+	struct rte_ring_ht_ptr prod __rte_aligned(RTE_CACHE_LINE_SIZE * 2);
 
 	/** Ring consumer status. */
-	struct cons {
-		uint32_t sc_dequeue;     /**< True, if single consumer. */
-		uint32_t size;           /**< Size of the ring. */
-		uint32_t mask;           /**< Mask (size-1) of ring. */
-		volatile uint32_t head;  /**< Consumer head. */
-		volatile uint32_t tail;  /**< Consumer tail. */
-	} cons __rte_aligned(RTE_CACHE_LINE_SIZE * 2);
+	struct rte_ring_ht_ptr cons __rte_aligned(RTE_CACHE_LINE_SIZE * 2);
 
 #ifdef RTE_LIBRTE_RING_DEBUG
 	struct rte_ring_debug_stats stats[RTE_MAX_LCORE];