Message ID | 1471343279-24014-10-git-send-email-gowrishankar.m@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id A1B916CAB; Tue, 16 Aug 2016 12:28:37 +0200 (CEST) Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by dpdk.org (Postfix) with ESMTP id A66096C99 for <dev@dpdk.org>; Tue, 16 Aug 2016 12:28:34 +0200 (CEST) Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u7GAOtAt055884 for <dev@dpdk.org>; Tue, 16 Aug 2016 06:28:33 -0400 Received: from e28smtp05.in.ibm.com (e28smtp05.in.ibm.com [125.16.236.5]) by mx0a-001b2d01.pphosted.com with ESMTP id 24sxunw3dp-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for <dev@dpdk.org>; Tue, 16 Aug 2016 06:28:33 -0400 Received: from localhost by e28smtp05.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for <dev@dpdk.org> from <gowrishankar.m@linux.vnet.ibm.com>; Tue, 16 Aug 2016 15:58:30 +0530 Received: from d28dlp01.in.ibm.com (9.184.220.126) by e28smtp05.in.ibm.com (192.168.1.135) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 16 Aug 2016 15:58:19 +0530 X-IBM-Helo: d28dlp01.in.ibm.com X-IBM-MailFrom: gowrishankar.m@linux.vnet.ibm.com X-IBM-RcptTo: dev@dpdk.org Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 8B20AE0060 for <dev@dpdk.org>; Tue, 16 Aug 2016 16:02:50 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay03.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u7GASIki11075876 for <dev@dpdk.org>; Tue, 16 Aug 2016 15:58:18 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u7GASHBL021206 for <dev@dpdk.org>; Tue, 16 Aug 2016 15:58:18 +0530 Received: from localhost.localdomain ([9.193.77.130]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u7GAS1ow020395; Tue, 16 Aug 2016 15:58:16 +0530 From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com> To: dev@dpdk.org Cc: Chao Zhu <chaozhu@linux.vnet.ibm.com>, Bruce Richardson <bruce.richardson@intel.com>, Konstantin Ananyev <konstantin.ananyev@intel.com>, Thomas Monjalon <thomas.monjalon@6wind.com>, Cristian Dumitrescu <cristian.dumitrescu@intel.com>, Pradeep <pradeep@us.ibm.com> Date: Tue, 16 Aug 2016 15:57:59 +0530 X-Mailer: git-send-email 1.9.1 In-Reply-To: <1471343279-24014-1-git-send-email-gowrishankar.m@linux.vnet.ibm.com> References: <1471343279-24014-1-git-send-email-gowrishankar.m@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16081610-0016-0000-0000-000002F44601 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16081610-0017-0000-0000-00002580D227 Message-Id: <1471343279-24014-10-git-send-email-gowrishankar.m@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-08-16_07:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608160134 Subject: [dpdk-dev] [PATCH v6 9/9] table: align rte table hash structs for cache line size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Gowrishankar
Aug. 16, 2016, 10:27 a.m. UTC
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(-)
Comments
> -----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
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?
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, September 8, 2016 10:36 AM > To: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com> > Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; > Chao Zhu <chaozhu@linux.vnet.ibm.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Pradeep <pradeep@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.
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@6wind.com] >> Sent: Thursday, September 8, 2016 10:36 AM >> To: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com> >> Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; >> Chao Zhu <chaozhu@linux.vnet.ibm.com>; Richardson, Bruce >> <bruce.richardson@intel.com>; Ananyev, Konstantin >> <konstantin.ananyev@intel.com>; Pradeep <pradeep@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. > >
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 {