From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id F17BFCE7 for ; 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" To: Neil Horman , "Richardson, Bruce" 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" , Michael Qiu 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 =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=