DPDK patches and discussions
 help / color / mirror / Atom feed
From: gowrishankar muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
	Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	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
Date: Thu, 8 Sep 2016 22:22:17 +0530	[thread overview]
Message-ID: <0ae24536-c04c-dcf2-5cd9-cb4904a47bb0@linux.vnet.ibm.com> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D8912647A669E2@IRSMSX108.ger.corp.intel.com>

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.
>
>

  reply	other threads:[~2016-09-08 16:52 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
2016-09-08  9:36     ` Thomas Monjalon
2016-09-08  9:40       ` Dumitrescu, Cristian
2016-09-08 16:52         ` gowrishankar muthukrishnan [this message]
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=0ae24536-c04c-dcf2-5cd9-cb4904a47bb0@linux.vnet.ibm.com \
    --to=gowrishankar.m@linux.vnet.ibm.com \
    --cc=bruce.richardson@intel.com \
    --cc=chaozhu@linux.vnet.ibm.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --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).