> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Andriy Berestovskyy > Sent: Friday, January 19, 2018 2:48 PM > To: dev@dpdk.org > Cc: Horton, Remy <remy.hor...@intel.com> > Subject: [dpdk-dev] [PATCH] keepalive: fix keepalive state alignment > > The __rte_cache_aligned was applied to the whole array, > not the array elements. This leads to a false sharing between > the monitored cores. > > Fixes: e70a61ad50ab ("keepalive: export states") > Cc: remy.hor...@intel.com > Signed-off-by: Andriy Berestovskyy <a...@semihalf.com> > --- > lib/librte_eal/common/rte_keepalive.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/lib/librte_eal/common/rte_keepalive.c > b/lib/librte_eal/common/rte_keepalive.c > index 7ddf201..a586e03 100644 > --- a/lib/librte_eal/common/rte_keepalive.c > +++ b/lib/librte_eal/common/rte_keepalive.c > @@ -13,8 +13,13 @@ > > struct rte_keepalive { > /** Core Liveness. */ > - enum rte_keepalive_state __rte_cache_aligned state_flags[ > - RTE_KEEPALIVE_MAXCORES]; > + struct { > + /* > + * Each element of the state_flags table must be cache aligned > + * to prevent false sharing. > + */ > + enum rte_keepalive_state s __rte_cache_aligned; > + } state_flags[RTE_KEEPALIVE_MAXCORES];
By aligning each item in the array, we do reduce false-sharing of the cache lines for all cores, however we pay the cost of increasing the footprint of the rte_keepalive struct. Note that the code iterates the full MAX_CORES in various loops in the monitoring core. Before (gdb) p sizeof(struct rte_keepalive) $1 = 1728 # 27 cache lines After (gdb) p sizeof(struct rte_keepalive) $1 = 9408 # 147 cache lines These changes do reduce false-sharing however is there actually a performance benefit? A lot of cache space will be taken up if each core requires its own cache line, which will reduce performance again.. it's a tradeoff. Little fix for a v2: "s" is not a good variable name for the rte_keepalive_state, please use something more descriptive. <snip>
