> >-----Original Message----- > >From: Honnappa Nagarahalli [mailto:[email protected]] > >Sent: Thursday, October 11, 2018 11:32 PM > >To: Richardson, Bruce <[email protected]>; De Lara Guarch, > >Pablo <[email protected]> > >Cc: [email protected]; Wang, Yipeng1 <[email protected]>; > >[email protected]; [email protected]; > >[email protected]; [email protected] > >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 <[email protected]>
