From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <keith.wiles@intel.com>
Received: from mga18.intel.com (mga18.intel.com [134.134.136.126])
by dpdk.org (Postfix) with ESMTP id 8EA671B4ED;
Thu, 29 Nov 2018 16:47:17 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
29 Nov 2018 07:47:16 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.56,295,1539673200"; d="scan'208";a="254458117"
Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205])
by orsmga004.jf.intel.com with ESMTP; 29 Nov 2018 07:47:16 -0800
Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by
fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS)
id 14.3.408.0; Thu, 29 Nov 2018 07:47:16 -0800
Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.100]) by
fmsmsx101.amr.corp.intel.com ([169.254.1.4]) with mapi id 14.03.0415.000;
Thu, 29 Nov 2018 07:47:15 -0800
From: "Wiles, Keith" <keith.wiles@intel.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
CC: dpdk-dev <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH] malloc: fix duplicate mem event notification
Thread-Index: AQHUh+7vxWmcCy20T0GIeOQ8DhE0xaVnXQ8AgAALx4CAAAMCgA==
Date: Thu, 29 Nov 2018 15:47:15 +0000
Message-ID: <21758B66-053A-4E95-8AE9-9FB8EE7C81F1@intel.com>
References: <a3807707cec73b13b25cdd623440fbcb61f02134.1543501068.git.anatoly.burakov@intel.com>
<D8B27CF4-BE99-4A06-8A94-86853BD480F8@intel.com>
<55beddff-0c3a-63c0-e793-636265175c75@intel.com>
In-Reply-To: <55beddff-0c3a-63c0-e793-636265175c75@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.252.193.14]
Content-Type: text/plain; charset="us-ascii"
Content-ID: <15DC2E60ABE0C946B779A23195D56052@intel.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH] malloc: fix duplicate mem event notification
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>
X-List-Received-Date: Thu, 29 Nov 2018 15:47:18 -0000
> On Nov 29, 2018, at 9:36 AM, Burakov, Anatoly <anatoly.burakov@intel.com>=
wrote:
>=20
> On 29-Nov-18 2:54 PM, Wiles, Keith wrote:
>>> On Nov 29, 2018, at 8:21 AM, Anatoly Burakov <anatoly.burakov@intel.com=
> wrote:
>>>=20
>>> We already trigger a mem event notification inside the walk function,
>>> no need to do it twice.
>>>=20
>>> Fixes: f32c7c9de961 ("malloc: enable event callbacks for external memor=
y")
>>> Cc: stable@dpdk.org
>>>=20
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---
>>> lib/librte_eal/common/rte_malloc.c | 4 ----
>>> 1 file changed, 4 deletions(-)
>>>=20
>>> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common=
/rte_malloc.c
>>> index 0da5ad5e8..750a83c2c 100644
>>> --- a/lib/librte_eal/common/rte_malloc.c
>>> +++ b/lib/librte_eal/common/rte_malloc.c
>>> @@ -518,10 +518,6 @@ sync_memory(const char *heap_name, void *va_addr, =
size_t len, bool attach)
>>> rte_errno =3D -wa.result;
>>> ret =3D -1;
>>> } else {
>>> - /* notify all subscribers that a new memory area was added */
>>> - if (attach)
>>> - eal_memalloc_mem_event_notify(RTE_MEM_EVENT_ALLOC,
>>> - va_addr, len);
>>> ret =3D 0;
>>> }
>> This change leaves
>> else {
>> ret =3D 0;
>> }
>> Needs to be:
>> else
>> ret =3D 0;
>=20
> Checkpatch disagrees :P Brackets are needed everywhere if at least one of=
the branches is a multiline branch. No brackets needed only if all branche=
s are one-line branches.
>=20
> As a side note, I would also argue that we shouldn't leave bracket-less i=
f statements altogether, because it makes for extra effort whenever a singl=
e-line statement inevitably becomes a multiline one (e.g. could be as simpl=
e as putting in a debug printf - i now have to add brackets everywhere...).=
But that's a topic for another day :)
Well it seems to be a very questionable formatting to leave the else with b=
rackets in a single line style IMO.
Also look at section 1.6.2 in DPDK coding style as it states something diff=
erent.
* Closing and opening braces go on the same line as the else keyword.
* Braces that are not necessary should be left out.
if (test)
stmt;
else if (bar) { =20
stmt;
stmt;
} else
stmt;
Note the last else here. Looking at this code it appears check patch is wro=
ng here compared to the DPDK coding style.
>=20
>>> unlock:
>>> --=20
>>> 2.17.1
>> Regards,
>> Keith
>=20
>=20
> --=20
> Thanks,
> Anatoly
Regards,
Keith