Hi Jerin, > -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com] > Sent: Sunday, December 06, 2015 3:59 PM > To: dev at dpdk.org > Cc: thomas.monjalon at 6wind.com; Richardson, Bruce; olivier.matz at > 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 <jerin.jacob at caviumnetworks.com> > --- > 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
