Re: [PATCH 1/2] mbuf: fix performance/cache resource issue with 128-byte cache line targets
From: Ananyev, Konstantin <hidden>
Date: 2015-12-07 15:21:36
Hi Jerin,
quoted hunk ↗ jump to hunk
-----Original Message----- From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] Sent: Sunday, December 06, 2015 3:59 PM To: dev@dpdk.org Cc: thomas.monjalon@6wind.com; Richardson, Bruce; olivier.matz@6wind.com; Dumitrescu, Cristian; Ananyev, Konstantin; Jerin Jacob Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix performance/cache resource issue with 128-byte cache line targets No need to split mbuf structure to two cache lines for 128-byte cache line size targets as it can fit on a single 128-byte cache line. Signed-off-by: Jerin Jacob <redacted> --- app/test/test_mbuf.c | 4 ++++ lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 4 ++++ lib/librte_mbuf/rte_mbuf.h | 2 ++ 3 files changed, 10 insertions(+)diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index b32bef6..5e21075 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c@@ -930,7 +930,11 @@ test_failing_mbuf_sanity_check(void) static int test_mbuf(void) { +#if RTE_CACHE_LINE_SIZE == 64 RTE_BUILD_BUG_ON(sizeof(struct rte_mbuf) != RTE_CACHE_LINE_SIZE * 2); +#elif RTE_CACHE_LINE_SIZE == 128 + RTE_BUILD_BUG_ON(sizeof(struct rte_mbuf) != RTE_CACHE_LINE_SIZE); +#endif /* create pktmbuf pool if it does not exist */ if (pktmbuf_pool == NULL) {diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h index bd1cc09..e724af7 100644--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h@@ -121,8 +121,12 @@ struct rte_kni_mbuf { uint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */ uint16_t data_len; /**< Amount of data in segment buffer. */ +#if RTE_CACHE_LINE_SIZE == 64 /* fields on second cache line */ char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_SIZE))); +#elif RTE_CACHE_LINE_SIZE == 128 + char pad3[24]; +#endif void *pool; void *next; };diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index f234ac9..0bf55e0 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h@@ -813,8 +813,10 @@ struct rte_mbuf { uint16_t vlan_tci_outer; /**< Outer VLAN Tag Control Identifier (CPU order) */ +#if RTE_CACHE_LINE_SIZE == 64 /* second cache line - fields only used in slow path or on TX */ MARKER cacheline1 __rte_cache_aligned; +#endif
I suppose you'll need to keep same space reserved for first 64B even on systems with 128B cache-line. Otherwise we can endup with different mbuf format for systems with 128B cache-line. Another thing - now we have __rte_cache_aligned all over the places, and I don't know is to double sizes of all these structures is a good idea. Again, #if RTE_CACHE_LINE_SIZE == 64 ... all over the places looks a bit clumsy. Wonder can we have __rte_cache_aligned/ RTE_CACHE_LINE_SIZE architecture specific, but introduce RTE_CACHE_MIN_LINE_SIZE(==64)/ __rte_cache_min_aligned and used it for mbuf (and might be other places). Konstantin