> >-----Original Message----- > >From: Honnappa Nagarahalli [mailto:honnappa.nagaraha...@arm.com] > >Sent: Thursday, October 11, 2018 11:32 PM > >To: Richardson, Bruce <bruce.richard...@intel.com>; De Lara Guarch, > >Pablo <pablo.de.lara.gua...@intel.com> > >Cc: dev@dpdk.org; Wang, Yipeng1 <yipeng1.w...@intel.com>; > >honnappa.nagaraha...@arm.com; dharmik.thak...@arm.com; > >gavin...@arm.com; n...@arm.com > >Subject: [PATCH v3 3/7] hash: correct key store element alignment > [Wang, Yipeng] "correct" -> "improve"? I think 'fix' is the right word to use. If we look at the existing code, original author seems to have tried to align it on certain boundary: struct rte_hash_key { union { uintptr_t idata; void *pdata; }; /* Variable key size */ char key[0]; } __attribute__((aligned(KEY_ALIGNMENT)));
But, this does not align every element of the key-store on the alignment boundary. This patch fixes it. I think what is missing is the "Fixes" tag. I will add that. I found this bug because I made the store/load of 'pdata' atomic. > > > >Correct the key store array element alignment. This is required to make > >'pdata' in 'struct rte_hash_key' align on the correct boundary. > [Wang, Yipeng] > More explanation in commit message is appreciated, because people may not > understand what is "correct" boundary. > e.g. Previously pdata could spread across multiple cache lines, which makes > the access of pdata non-atomic which may have performance implications. > I will add more explanation related to atomic access. > Otherwise > Reviewed-by: Yipeng Wang <yipeng1.w...@intel.com>