Thanks Cristian and Thomas for your feedback. I have taken your suggestions and sent out v7. Please check if the new patch is fine.
Thanks, Gowrishankar On Thursday 08 September 2016 03:10 PM, Dumitrescu, Cristian wrote: > >> -----Original Message----- >> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] >> Sent: Thursday, September 8, 2016 10:36 AM >> To: Gowrishankar Muthukrishnan <gowrishankar.m at linux.vnet.ibm.com> >> Cc: dev at dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu at intel.com>; >> Chao Zhu <chaozhu at linux.vnet.ibm.com>; Richardson, Bruce >> <bruce.richardson at intel.com>; Ananyev, Konstantin >> <konstantin.ananyev at intel.com>; Pradeep <pradeep at us.ibm.com> >> Subject: Re: [dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs >> for >> cache line size >> >> 2016-08-31 17:29, Dumitrescu, Cristian: >>> From: Gowrishankar Muthukrishnan >>>> rte table hash structs rte_bucket_4_8, rte_bucket_4_16 and >>>> rte_bucket_4_32 have >>>> to be cache aligned as required by their corresponding hash create >> functions >>>> rte_table_hash_create_key8_lru etc. >>> Hi Gowrishankar, >>> >>> My understanding is you are trying to work around the check invoked by >> the hash table create functions that verifies the size of the bucket header >> structure is a multiple of the cache line, right? >>> Given that the size of this structure is 1x, 2x or 3x times 64 bytes, the >>> check >> passes on IA CPUs (cache line of 64 bytes; explicit alignment to cache line >> size >> is not needed in order to make the size of the structure a multiple of cache >> line), but not on PPC CPUs (cache line of 128 bytes), correct? >>> I don't think your proposal provides the best way to fix this issue, since >> your code leads to a considerable increase in the memory consumption used >> per bucket in most cases: >>> - 100% more memory for 8-byte key hash table >>> - 0% more for 16-byte key hash table (code does not fix anything, >> explicit alignment is not needed) >>> - 50% more for 32-byte key hash table >>> >>> I suggest you simply change the check: instead of validating this data >> structure is a multiple of cache line size, validate it is a multiple of 64 >> bytes. >> >> Any news please? >> The whole series is blocked for this patch. >> Should we expect a v7? > Yes, I think we should. Small fix for a considerable benefit. > >
