DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: dev@dpdk.org
Cc: bruce.richardson@dpdk.org, konstantin.ananyev@intel.com,
	daniel.verkamp@intel.com
Subject: [dpdk-dev] [RFC] ring: relax alignment constraint on ring structure
Date: Fri, 30 Jun 2017 16:26:09 +0200	[thread overview]
Message-ID: <20170630142609.6180-1-olivier.matz@6wind.com> (raw)

The initial objective of
commit d9f0d3a1ffd4 ("ring: remove split cacheline build setting")
was to add an empty cache line betwee, the producer and consumer
data (on platform with cache line size = 64B), preventing from
having them on adjacent cache lines.

Following discussion on the mailing list, it appears that this
also imposes an alignment constraint that is not required.

This patch removes the extra alignment constraint and adds the
empty cache lines using padding fields in the structure. The
size of rte_ring structure and the offset of the fields remain
the same on platforms with cache line size = 64B:

  rte_ring = 384
  rte_ring.name = 0
  rte_ring.flags = 32
  rte_ring.memzone = 40
  rte_ring.size = 48
  rte_ring.mask = 52
  rte_ring.prod = 128
  rte_ring.cons = 256

But it has an impact on platform where cache line size is 128B:

  rte_ring = 384        -> 768
  rte_ring.name = 0
  rte_ring.flags = 32
  rte_ring.memzone = 40
  rte_ring.size = 48
  rte_ring.mask = 52
  rte_ring.prod = 128   -> 256
  rte_ring.cons = 256   -> 512

Link: http://dpdk.org/dev/patchwork/patch/25039/
Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

I'm sending this patch to throw the discussion again, but since it
breaks the ABI on platform with cache lines = 128B, I think we should
follow the usual ABI breakage process.

If everybody agree, I'll send a notice and resend a similar patch after
17.08.

Olivier



 lib/librte_ring/rte_ring.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index 1beb781b4..135b83df0 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -116,14 +116,6 @@ enum rte_ring_queue_behavior {
 
 struct rte_memzone; /* forward declaration, so as not to require memzone.h */
 
-#if RTE_CACHE_LINE_SIZE < 128
-#define PROD_ALIGN (RTE_CACHE_LINE_SIZE * 2)
-#define CONS_ALIGN (RTE_CACHE_LINE_SIZE * 2)
-#else
-#define PROD_ALIGN RTE_CACHE_LINE_SIZE
-#define CONS_ALIGN RTE_CACHE_LINE_SIZE
-#endif
-
 /* structure to hold a pair of head/tail values and other metadata */
 struct rte_ring_headtail {
 	volatile uint32_t head;  /**< Prod/consumer head. */
@@ -155,11 +147,15 @@ struct rte_ring {
 	uint32_t mask;           /**< Mask (size-1) of ring. */
 	uint32_t capacity;       /**< Usable size of ring */
 
+	char pad0 __rte_cache_aligned; /**< empty cache line */
+
 	/** Ring producer status. */
-	struct rte_ring_headtail prod __rte_aligned(PROD_ALIGN);
+	struct rte_ring_headtail prod __rte_cache_aligned;
+	char pad1 __rte_cache_aligned; /**< empty cache line */
 
 	/** Ring consumer status. */
-	struct rte_ring_headtail cons __rte_aligned(CONS_ALIGN);
+	struct rte_ring_headtail cons __rte_cache_aligned;
+	char pad2 __rte_cache_aligned; /**< empty cache line */
 };
 
 #define RING_F_SP_ENQ 0x0001 /**< The default enqueue is "single-producer". */
-- 
2.11.0

             reply	other threads:[~2017-06-30 14:26 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-30 14:26 Olivier Matz [this message]
2017-07-20  8:52 ` Olivier Matz
2018-04-03 13:26 ` [dpdk-dev] [PATCH] " Olivier Matz
2018-04-03 15:07   ` Jerin Jacob
2018-04-03 15:25     ` Olivier Matz
2018-04-03 15:37       ` Jerin Jacob
2018-04-03 15:56         ` Olivier Matz
2018-04-03 16:42           ` Jerin Jacob
2018-04-04 23:38             ` Ananyev, Konstantin
2018-04-05  8:01               ` Jerin Jacob
2018-04-05 13:49                 ` Ananyev, Konstantin
2018-04-06  1:26                   ` Jerin Jacob
2018-04-11  0:33                     ` Ananyev, Konstantin
2018-04-11  2:48                       ` Jerin Jacob
2018-04-11  8:40                         ` Ananyev, Konstantin
2018-04-17 22:15                     ` Thomas Monjalon
2018-05-25 10:59   ` Burakov, Anatoly
2018-05-25 12:18     ` Burakov, Anatoly
2018-05-25 14:57       ` Burakov, Anatoly
2018-05-25 15:17         ` Olivier Matz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170630142609.6180-1-olivier.matz@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=bruce.richardson@dpdk.org \
    --cc=daniel.verkamp@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).