From mboxrd@z Thu Jan  1 00:00:00 1970
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Subject: Re: [PATCH v3 3/7] hash: correct key store element
	alignment
Date: Tue, 16 Oct 2018 23:26:58 +0000
Message-ID: <DB7PR08MB3674B7270C0F5A9EC9CFE4B398FE0@DB7PR08MB3674.eurprd08.prod.outlook.com>
References: <1539325918-125438-1-git-send-email-honnappa.nagarahalli@arm.com>
 <1539325918-125438-4-git-send-email-honnappa.nagarahalli@arm.com>
 <D2C4A16CA39F7F4E8E384D204491D7A6614EFB5E@FMSMSX151.amr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Cc: "dev@dpdk.org" <dev@dpdk.org>, Dharmik Thakkar <Dharmik.Thakkar@arm.com>,
 "Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>, nd <nd@arm.com>,
 "Gobriel, Sameh" <sameh.gobriel@intel.com>
To: "Wang, Yipeng1" <yipeng1.wang@intel.com>, "Richardson, Bruce"
 <bruce.richardson@intel.com>, "De Lara Guarch, Pablo"
 <pablo.de.lara.guarch@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from EUR02-VE1-obe.outbound.protection.outlook.com
 (mail-eopbgr20046.outbound.protection.outlook.com [40.107.2.46])
 by dpdk.org (Postfix) with ESMTP id 78B324F90
 for <dev@dpdk.org>; Wed, 17 Oct 2018 01:26:59 +0200 (CEST)
In-Reply-To: <D2C4A16CA39F7F4E8E384D204491D7A6614EFB5E@FMSMSX151.amr.corp.intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

> >-----Original Message-----
> >From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@arm.com]
> >Sent: Thursday, October 11, 2018 11:32 PM
> >To: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch,
> >Pablo <pablo.de.lara.guarch@intel.com>
> >Cc: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>;
> >honnappa.nagarahalli@arm.com; dharmik.thakkar@arm.com;
> >gavin.hu@arm.com; nd@arm.com
> >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, or=
iginal 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 bo=
undary. 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 mak=
es
> the access of pdata non-atomic which may have performance implications.
>=20
I will add more explanation related to atomic access.

> Otherwise
> Reviewed-by: Yipeng Wang <yipeng1.wang@intel.com>