From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <michael.qiu@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id F17BFCE7
 for <dev@dpdk.org>; Mon,  8 Dec 2014 03:46:56 +0100 (CET)
Received: from orsmga003.jf.intel.com ([10.7.209.27])
 by orsmga101.jf.intel.com with ESMTP; 07 Dec 2014 18:46:55 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.04,691,1406617200"; d="scan'208";a="495198463"
Received: from pgsmsx107.gar.corp.intel.com ([10.221.44.105])
 by orsmga003.jf.intel.com with ESMTP; 07 Dec 2014 18:43:18 -0800
Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by
 PGSMSX107.gar.corp.intel.com (10.221.44.105) with Microsoft SMTP Server (TLS)
 id 14.3.195.1; Mon, 8 Dec 2014 10:46:53 +0800
Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.110]) by
 shsmsx102.ccr.corp.intel.com ([169.254.2.216]) with mapi id 14.03.0195.001;
 Mon, 8 Dec 2014 10:46:51 +0800
From: "Qiu, Michael" <michael.qiu@intel.com>
To: Neil Horman <nhorman@tuxdriver.com>, "Richardson, Bruce"
 <bruce.richardson@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform
Thread-Index: AQHQEFE2U0vIPmApvkOkCk8Kal0Dyw==
Date: Mon, 8 Dec 2014 02:46:51 +0000
Message-ID: <533710CFB86FA344BFBF2D6802E60286C9D736@SHSMSX101.ccr.corp.intel.com>
References: <533710CFB86FA344BFBF2D6802E60286C9CB12@SHSMSX101.ccr.corp.intel.com>
 <1417684369-21330-1-git-send-email-michael.qiu@intel.com>
 <54816D73.1020906@linux.vnet.ibm.com>
 <20141205142205.GB29245@hmsreliant.think-freely.org>
 <20141205150233.GA9704@bricha3-MOBL3>
 <20141205152406.GD29245@hmsreliant.think-freely.org>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Cc: "dev@dpdk.org" <dev@dpdk.org>, Michael Qiu <qdy220091330@gmail.com>
Subject: Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform
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>
X-List-Received-Date: Mon, 08 Dec 2014 02:46:58 -0000

On 12/5/2014 11:25 PM, Neil Horman wrote:=0A=
> On Fri, Dec 05, 2014 at 03:02:33PM +0000, Bruce Richardson wrote:=0A=
>> On Fri, Dec 05, 2014 at 09:22:05AM -0500, Neil Horman wrote:=0A=
>>> On Fri, Dec 05, 2014 at 04:31:47PM +0800, Chao Zhu wrote:=0A=
>>>> On 2014/12/4 17:12, Michael Qiu wrote:=0A=
>>>>> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison=0A=
>>>>> is always false due to limited range of data type [-Werror=3Dtype-lim=
its]=0A=
>>>>>     || (hugepage_sz =3D=3D RTE_PGSIZE_16G)) {=0A=
>>>>>     ^=0A=
>>>>> cc1: all warnings being treated as errors=0A=
>>>>>=0A=
>>>>> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer=0A=
>>>>> conversion from "long long" to "void *" may lose significant bits=0A=
>>>>>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);=0A=
>>>>>=0A=
>>>>> This was introuduced by commit b77b5639:=0A=
>>>>>         mem: add huge page sizes for IBM Power=0A=
>>>>>=0A=
>>>>> The root cause is that size_t and uintptr_t are 32-bit in i686=0A=
>>>>> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.=0A=
>>>>>=0A=
>>>>> Define RTE_PGSIZE_16G only in 64 bit platform to avoid=0A=
>>>>> this issue.=0A=
>>>>>=0A=
>>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>=0A=
>>>>> ---=0A=
>>>>>  v3 ---> v2=0A=
>>>>> 	Change RTE_PGSIZE_16G from ULL to UL=0A=
>>>>> 	to keep all entries consistent=0A=
>>>>>=0A=
>>>>>  V2 ---> v1=0A=
>>>>> 	Change two type entries to one, and=0A=
>>>>> 	leave RTE_PGSIZE_16G only valid for=0A=
>>>>> 	64-bit platform=0A=
>>>>>=0A=
>>> NACK, this is the wrong way to fix this problem.  Pagesizes are indepen=
dent of=0A=
>>> architecutre.  While a system can't have a hugepage size that exceeds i=
ts=0A=
>>> virtual address limit, theres no need to do per-architecture special ca=
sing of=0A=
>>> page sizes here.  Instead of littering the code with ifdef RTE_ARCH_64=
=0A=
>>> everytime you want to check a page size, just convert the size_t to a u=
int64_t=0A=
>>> and you can allow all of the enumerated page types on all architecutres=
, and=0A=
>>> save yourself some ifdeffing in the process.=0A=
>>>=0A=
>>> Neil=0A=
>> While I get your point, I find it distasteful to use a uint64_t for memo=
ry sizes,=0A=
>> when there is the size_t type defined for that particular purpose.=0A=
>> However, I suppose that reducing the number of #ifdefs compared to using=
 the=0A=
>> "correct" datatypes for objects is a more practical optino - however dis=
tastful=0A=
>> I find it.=0A=
> size_t isn't defined for memory sizes in the sense that we're using them =
here.=0A=
> size_t is meant to address the need for a type to describe object sizes o=
n a=0A=
> particular system, and it itself is sized accordingly (32 bits on a 32 bi=
t arch,=0A=
> 64 on 64), so that you can safely store a size that the system in questio=
n might=0A=
> maximally allocate/return.  In this situation we are describing memory si=
zes=0A=
> that might occur no a plurality of arches, and so size_t is inappropriate=
=0A=
> because it as a type is not sized for anything other than the arch it is =
being=0A=
> built for.  The pragmatic benefits of ennumerating page sizes in a single=
=0A=
> canonical location far outweigh the desire to use a misappropriated type =
to=0A=
> describe them.=0A=
=0A=
Neil,=0A=
=0A=
This patch fix two compile issues, and we need to do *dpdk testing=0A=
affairs*,  if it is blocked in build stage, we can do *nothing* for testing=
.=0A=
=0A=
I've get you mind and your concern. But we should take care of changing=0A=
the type of "hugepage_sz", because lots of places using it.=0A=
=0A=
Would you mind if we consider this as hot fix, and we can post a better=0A=
fix later(like in dpdk 2.0)? Otherwise all test cycle are blocked.=0A=
=0A=
Thanks,=0A=
Michael=0A=
> Neil=0A=
>=0A=
>=0A=
=0A=