From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: Chao Zhu <chaozhu@linux.vnet.ibm.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
Thomas Monjalon <thomas.monjalon@6wind.com>,
Pradeep <pradeep@us.ibm.com>
Subject: Re: [dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for cache line size
Date: Wed, 31 Aug 2016 17:29:45 +0000 [thread overview]
Message-ID: <3EB4FA525960D640B5BDFFD6A3D8912647A547BE@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <1471343279-24014-10-git-send-email-gowrishankar.m@linux.vnet.ibm.com>
> -----Original Message-----
> From: Gowrishankar Muthukrishnan
> [mailto:gowrishankar.m@linux.vnet.ibm.com]
> Sent: Tuesday, August 16, 2016 11:28 AM
> To: dev@dpdk.org
> Cc: Chao Zhu <chaozhu@linux.vnet.ibm.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; Pradeep <pradeep@us.ibm.com>
> Subject: [PATCH v6 9/9] table: align rte table hash structs for cache line size
>
> 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.
>
> Signed-off-by: Gowrishankar Muthukrishnan
> <gowrishankar.m@linux.vnet.ibm.com>
> ---
> lib/librte_table/rte_table_hash_key16.c | 4 ++--
> lib/librte_table/rte_table_hash_key32.c | 4 ++--
> lib/librte_table/rte_table_hash_key8.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/librte_table/rte_table_hash_key16.c
> b/lib/librte_table/rte_table_hash_key16.c
> index b7e000f..2102326 100644
> --- a/lib/librte_table/rte_table_hash_key16.c
> +++ b/lib/librte_table/rte_table_hash_key16.c
> @@ -68,10 +68,10 @@ struct rte_bucket_4_16 {
> uint64_t next_valid;
>
> /* Cache line 1 */
> - uint64_t key[4][2];
> + uint64_t key[4][2] __rte_cache_aligned;
>
> /* Cache line 2 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
> };
>
> struct rte_table_hash {
> diff --git a/lib/librte_table/rte_table_hash_key32.c
> b/lib/librte_table/rte_table_hash_key32.c
> index a7aba49..619f63a 100644
> --- a/lib/librte_table/rte_table_hash_key32.c
> +++ b/lib/librte_table/rte_table_hash_key32.c
> @@ -68,10 +68,10 @@ struct rte_bucket_4_32 {
> uint64_t next_valid;
>
> /* Cache lines 1 and 2 */
> - uint64_t key[4][4];
> + uint64_t key[4][4] __rte_cache_aligned;
>
> /* Cache line 3 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
> };
>
> struct rte_table_hash {
> diff --git a/lib/librte_table/rte_table_hash_key8.c
> b/lib/librte_table/rte_table_hash_key8.c
> index e2e2bdc..4d5e0cd 100644
> --- a/lib/librte_table/rte_table_hash_key8.c
> +++ b/lib/librte_table/rte_table_hash_key8.c
> @@ -68,7 +68,7 @@ struct rte_bucket_4_8 {
> uint64_t key[4];
>
> /* Cache line 1 */
> - uint8_t data[0];
> + uint8_t data[0] __rte_cache_aligned;
> };
>
> struct rte_table_hash {
> --
> 1.9.1
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.
Regards,
Cristian
next prev parent reply other threads:[~2016-08-31 17:29 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-16 10:27 [dpdk-dev] [PATCH v6 0/9] enable lpm, acl and other missing libraries in ppc64le Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 1/9] lpm: add altivec intrinsics for dpdk lpm on ppc_64 Gowrishankar Muthukrishnan
2016-09-07 9:21 ` Bruce Richardson
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 2/9] acl: add altivec intrinsics for dpdk acl " Gowrishankar Muthukrishnan
2016-08-31 13:08 ` Ananyev, Konstantin
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 3/9] l3fwd: add altivec support for em_hash_key Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 4/9] table: enable table library for ppc64le Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 5/9] sched: enable sched " Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 6/9] port: enable port " Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 7/9] pipeline: enable pipeline " Gowrishankar Muthukrishnan
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 8/9] ip_pipeline: fix lcore mapping for varying SMT threads as in ppc64 Gowrishankar Muthukrishnan
2016-08-31 17:33 ` Dumitrescu, Cristian
2016-08-16 10:27 ` [dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for cache line size Gowrishankar Muthukrishnan
2016-08-31 17:29 ` Dumitrescu, Cristian [this message]
2016-09-08 9:36 ` Thomas Monjalon
2016-09-08 9:40 ` Dumitrescu, Cristian
2016-09-08 16:52 ` gowrishankar muthukrishnan
2016-08-17 8:48 ` [dpdk-dev] [PATCH v6 0/9] enable lpm, acl and other missing libraries in ppc64le Chao Zhu
2016-08-26 10:55 ` [dpdk-dev] FW: " Chao Zhu
2016-08-26 15:29 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3EB4FA525960D640B5BDFFD6A3D8912647A547BE@IRSMSX108.ger.corp.intel.com \
--to=cristian.dumitrescu@intel.com \
--cc=bruce.richardson@intel.com \
--cc=chaozhu@linux.vnet.ibm.com \
--cc=dev@dpdk.org \
--cc=gowrishankar.m@linux.vnet.ibm.com \
--cc=konstantin.ananyev@intel.com \
--cc=pradeep@us.ibm.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).