Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

[Qiu, Michael]

Hi all,

Any comments on this patch?

It is really a big issue in dpdk1.8-rc2 and blocked the testing on i686
platform.

Thanks,
Michael
On 12/3/2014 4:11 PM, Michael Qiu wrote:
> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> is always false due to limited range of data type [-Werror=type-limits]
>     || (hugepage_sz == RTE_PGSIZE_16G)) {
>     ^
> cc1: all warnings being treated as errors
>
> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> conversion from "long long" to "void *" may lose significant bits
>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>
> This was introuduced by commit b77b5639:
>         mem: add huge page sizes for IBM Power
>
> The root cause is that size_t and uintptr_t are 32-bit in i686
> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>
> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> this issue.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  app/test/test_memzone.c                    | 18 ++++++++++++------
>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>  4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
> index 5da6903..7bab8b5 100644
> --- a/app/test/test_memzone.c
> +++ b/app/test/test_memzone.c
> @@ -145,8 +145,10 @@ test_memzone_reserve_flags(void)
>  			hugepage_1GB_avail = 1;
>  		if (ms[i].hugepage_sz == RTE_PGSIZE_16M)
>  			hugepage_16MB_avail = 1;
> +#ifdef RTE_ARCH_64
>  		if (ms[i].hugepage_sz == RTE_PGSIZE_16G)
>  			hugepage_16GB_avail = 1;
> +#endif
>  	}
>  	/* Display the availability of 2MB ,1GB, 16MB, 16GB pages */
>  	if (hugepage_2MB_avail)
> @@ -234,8 +236,8 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails unless
> -		 * HINT flag is indicated
> +		/* Check if 2MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_2MB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
> @@ -295,8 +297,9 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +#ifdef RTE_ARCH_64
> +		/* Check if 16GB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_16GB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
> @@ -318,7 +321,9 @@ test_memzone_reserve_flags(void)
>  				return -1;
>  			}
>  		}
> +#endif
>  	}
> +#ifdef RTE_ARCH_64
>  	/*As with 16MB tests above for 16GB huge page requests*/
>  	if (hugepage_16GB_avail) {
>  		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
> @@ -343,8 +348,8 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +		/* Check if 16MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_16MB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
> @@ -376,6 +381,7 @@ test_memzone_reserve_flags(void)
>  			}
>  		}
>  	}
> +#endif
>  	return 0;
>  }
>  
> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index b5a5d72..ee233ad 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -221,12 +221,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
>  		if ((flags & RTE_MEMZONE_1GB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_2M)
>  			continue;
> +#ifdef RTE_ARCH_64
>  		if ((flags & RTE_MEMZONE_16MB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_16G)
>  			continue;
>  		if ((flags & RTE_MEMZONE_16GB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_16M)
>  			continue;
> +#endif
>  
>  		/* this segment is the best until now */
>  		if (memseg_idx == -1) {
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 1990833..6bcb92b 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -53,12 +53,14 @@ extern "C" {
>  #endif
>  
>  enum rte_page_sizes {
> -	RTE_PGSIZE_4K = 1ULL << 12,
> -	RTE_PGSIZE_2M = 1ULL << 21,
> -	RTE_PGSIZE_1G = 1ULL << 30,
> -	RTE_PGSIZE_64K = 1ULL << 16,
> -	RTE_PGSIZE_16M = 1ULL << 24,
> -	RTE_PGSIZE_16G = 1ULL << 34
> +	RTE_PGSIZE_4K	= 1UL << 12,
> +	RTE_PGSIZE_2M	= 1UL << 21,
> +	RTE_PGSIZE_1G	= 1UL << 30,
> +	RTE_PGSIZE_64K	= 1UL << 16,
> +	RTE_PGSIZE_16M	= 1UL << 24,
> +#ifdef RTE_ARCH_64
> +	RTE_PGSIZE_16G	= 1ULL << 34
> +#endif
>  };
>  
>  #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index e6cb919..833670c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -317,11 +317,10 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>  			hugepg_tbl[i].filepath[sizeof(hugepg_tbl[i].filepath) - 1] = '\0';
>  		}
>  #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G and 16G pages, just reuse
> -		 * original map address as final map address.
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined),
> +		 * just reuse original map address as final map address.
>  		 */
> -		else if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		else if (hugepage_sz == RTE_PGSIZE_1G) {
>  			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>  			hugepg_tbl[i].orig_va = NULL;
>  			continue;
> @@ -422,11 +421,10 @@ remap_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
>  	while (i < hpi->num_pages[0]) {
>  
>  #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G pages and 16G pages,
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined,
>  		 * just reuse original map address as final map address.
>  		 */
> -		if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		if (hugepage_sz == RTE_PGSIZE_1G) {
>  			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>  			hugepg_tbl[i].orig_va = NULL;
>  			i++;

Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

[Bruce Richardson]

On Wed, Dec 03, 2014 at 04:10:23PM +0800, Michael Qiu wrote:
> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> is always false due to limited range of data type [-Werror=type-limits]
>     || (hugepage_sz == RTE_PGSIZE_16G)) {
>     ^
> cc1: all warnings being treated as errors
> 
> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> conversion from "long long" to "void *" may lose significant bits
>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
> 
> This was introuduced by commit b77b5639:
>         mem: add huge page sizes for IBM Power
> 
> The root cause is that size_t and uintptr_t are 32-bit in i686
> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
> 
> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> this issue.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
Minor comment below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/test_memzone.c                    | 18 ++++++++++++------
>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>  4 files changed, 27 insertions(+), 19 deletions(-)
> 
... snip ...
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -53,12 +53,14 @@ extern "C" {
>  #endif
>  
>  enum rte_page_sizes {
> -	RTE_PGSIZE_4K = 1ULL << 12,
> -	RTE_PGSIZE_2M = 1ULL << 21,
> -	RTE_PGSIZE_1G = 1ULL << 30,
> -	RTE_PGSIZE_64K = 1ULL << 16,
> -	RTE_PGSIZE_16M = 1ULL << 24,
> -	RTE_PGSIZE_16G = 1ULL << 34
> +	RTE_PGSIZE_4K	= 1UL << 12,
> +	RTE_PGSIZE_2M	= 1UL << 21,
> +	RTE_PGSIZE_1G	= 1UL << 30,
> +	RTE_PGSIZE_64K	= 1UL << 16,
> +	RTE_PGSIZE_16M	= 1UL << 24,
> +#ifdef RTE_ARCH_64
> +	RTE_PGSIZE_16G	= 1ULL << 34
you don't need the "LL" here as long type is 64-bits on 64-bit systems. Changing
it to 1UL << 34 will keep all entries consistent.

Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

[Qiu, Michael]

On 12/3/2014 11:40 PM, Richardson, Bruce wrote:
> On Wed, Dec 03, 2014 at 04:10:23PM +0800, Michael Qiu wrote:
>> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
>> is always false due to limited range of data type [-Werror=type-limits]
>>     || (hugepage_sz == RTE_PGSIZE_16G)) {
>>     ^
>> cc1: all warnings being treated as errors
>>
>> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
>> conversion from "long long" to "void *" may lose significant bits
>>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>>
>> This was introuduced by commit b77b5639:
>>         mem: add huge page sizes for IBM Power
>>
>> The root cause is that size_t and uintptr_t are 32-bit in i686
>> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>>
>> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
>> this issue.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> Minor comment below.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
>> ---
>>  app/test/test_memzone.c                    | 18 ++++++++++++------
>>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
>>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>>  4 files changed, 27 insertions(+), 19 deletions(-)
>>
> ... snip ...
>> --- a/lib/librte_eal/common/include/rte_memory.h
>> +++ b/lib/librte_eal/common/include/rte_memory.h
>> @@ -53,12 +53,14 @@ extern "C" {
>>  #endif
>>  
>>  enum rte_page_sizes {
>> -	RTE_PGSIZE_4K = 1ULL << 12,
>> -	RTE_PGSIZE_2M = 1ULL << 21,
>> -	RTE_PGSIZE_1G = 1ULL << 30,
>> -	RTE_PGSIZE_64K = 1ULL << 16,
>> -	RTE_PGSIZE_16M = 1ULL << 24,
>> -	RTE_PGSIZE_16G = 1ULL << 34
>> +	RTE_PGSIZE_4K	= 1UL << 12,
>> +	RTE_PGSIZE_2M	= 1UL << 21,
>> +	RTE_PGSIZE_1G	= 1UL << 30,
>> +	RTE_PGSIZE_64K	= 1UL << 16,
>> +	RTE_PGSIZE_16M	= 1UL << 24,
>> +#ifdef RTE_ARCH_64
>> +	RTE_PGSIZE_16G	= 1ULL << 34
> you don't need the "LL" here as long type is 64-bits on 64-bit systems. Changing
> it to 1UL << 34 will keep all entries consistent.

Hi Thomas,

Should I resend V3 patch to modify this or you can do it when you plan
to merge this patch?

Thanks,
Michael
>

Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

[Thomas Monjalon]

2014-12-04 02:49, Qiu, Michael:
> On 12/3/2014 11:40 PM, Richardson, Bruce wrote:
> > On Wed, Dec 03, 2014 at 04:10:23PM +0800, Michael Qiu wrote:
> >> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> >> is always false due to limited range of data type [-Werror=type-limits]
> >>     || (hugepage_sz == RTE_PGSIZE_16G)) {
> >>     ^
> >> cc1: all warnings being treated as errors
> >>
> >> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> >> conversion from "long long" to "void *" may lose significant bits
> >>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
> >>
> >> This was introuduced by commit b77b5639:
> >>         mem: add huge page sizes for IBM Power
> >>
> >> The root cause is that size_t and uintptr_t are 32-bit in i686
> >> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
> >>
> >> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> >> this issue.
> >>
> >> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > Minor comment below.
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> >> ---
> >>  app/test/test_memzone.c                    | 18 ++++++++++++------
> >>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
> >>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
> >>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
> >>  4 files changed, 27 insertions(+), 19 deletions(-)
> >>
> > ... snip ...
> >> --- a/lib/librte_eal/common/include/rte_memory.h
> >> +++ b/lib/librte_eal/common/include/rte_memory.h
> >> @@ -53,12 +53,14 @@ extern "C" {
> >>  #endif
> >>  
> >>  enum rte_page_sizes {
> >> -	RTE_PGSIZE_4K = 1ULL << 12,
> >> -	RTE_PGSIZE_2M = 1ULL << 21,
> >> -	RTE_PGSIZE_1G = 1ULL << 30,
> >> -	RTE_PGSIZE_64K = 1ULL << 16,
> >> -	RTE_PGSIZE_16M = 1ULL << 24,
> >> -	RTE_PGSIZE_16G = 1ULL << 34
> >> +	RTE_PGSIZE_4K	= 1UL << 12,
> >> +	RTE_PGSIZE_2M	= 1UL << 21,
> >> +	RTE_PGSIZE_1G	= 1UL << 30,
> >> +	RTE_PGSIZE_64K	= 1UL << 16,
> >> +	RTE_PGSIZE_16M	= 1UL << 24,
> >> +#ifdef RTE_ARCH_64
> >> +	RTE_PGSIZE_16G	= 1ULL << 34
> > you don't need the "LL" here as long type is 64-bits on 64-bit systems. Changing
> > it to 1UL << 34 will keep all entries consistent.
> 
> Hi Thomas,
> 
> Should I resend V3 patch to modify this or you can do it when you plan
> to merge this patch?

I could do the change by myself. But given that you had some problems to send
the patch to the mailing list, please send the v3 to everyone.
Then a review from Chao would be appreciated.

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Michael Qiu]

lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
is always false due to limited range of data type [-Werror=type-limits]
    || (hugepage_sz == RTE_PGSIZE_16G)) {
    ^
cc1: all warnings being treated as errors

lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
conversion from "long long" to "void *" may lose significant bits
   RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);

This was introuduced by commit b77b5639:
        mem: add huge page sizes for IBM Power

The root cause is that size_t and uintptr_t are 32-bit in i686
platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.

Define RTE_PGSIZE_16G only in 64 bit platform to avoid
this issue.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 v3 ---> v2
	Change RTE_PGSIZE_16G from ULL to UL
	to keep all entries consistent

 V2 ---> v1
	Change two type entries to one, and
	leave RTE_PGSIZE_16G only valid for
	64-bit platform

 app/test/test_memzone.c                    | 18 ++++++++++++------
 lib/librte_eal/common/eal_common_memzone.c |  2 ++
 lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
 lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
 4 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
index 5da6903..7bab8b5 100644
--- a/app/test/test_memzone.c
+++ b/app/test/test_memzone.c
@@ -145,8 +145,10 @@ test_memzone_reserve_flags(void)
 			hugepage_1GB_avail = 1;
 		if (ms[i].hugepage_sz == RTE_PGSIZE_16M)
 			hugepage_16MB_avail = 1;
+#ifdef RTE_ARCH_64
 		if (ms[i].hugepage_sz == RTE_PGSIZE_16G)
 			hugepage_16GB_avail = 1;
+#endif
 	}
 	/* Display the availability of 2MB ,1GB, 16MB, 16GB pages */
 	if (hugepage_2MB_avail)
@@ -234,8 +236,8 @@ test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		/* Check if 1GB huge pages are unavailable, that function fails unless
-		 * HINT flag is indicated
+		/* Check if 2MB huge pages are unavailable, that function
+		 * fails unless HINT flag is indicated
 		 */
 		if (!hugepage_2MB_avail) {
 			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
@@ -295,8 +297,9 @@ test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		/* Check if 1GB huge pages are unavailable, that function fails
-		 * unless HINT flag is indicated
+#ifdef RTE_ARCH_64
+		/* Check if 16GB huge pages are unavailable, that function
+		 * fails unless HINT flag is indicated
 		 */
 		if (!hugepage_16GB_avail) {
 			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
@@ -318,7 +321,9 @@ test_memzone_reserve_flags(void)
 				return -1;
 			}
 		}
+#endif
 	}
+#ifdef RTE_ARCH_64
 	/*As with 16MB tests above for 16GB huge page requests*/
 	if (hugepage_16GB_avail) {
 		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
@@ -343,8 +348,8 @@ test_memzone_reserve_flags(void)
 			return -1;
 		}
 
-		/* Check if 1GB huge pages are unavailable, that function fails
-		 * unless HINT flag is indicated
+		/* Check if 16MB huge pages are unavailable, that function
+		 * fails unless HINT flag is indicated
 		 */
 		if (!hugepage_16MB_avail) {
 			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
@@ -376,6 +381,7 @@ test_memzone_reserve_flags(void)
 			}
 		}
 	}
+#endif
 	return 0;
 }
 
diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
index b5a5d72..ee233ad 100644
--- a/lib/librte_eal/common/eal_common_memzone.c
+++ b/lib/librte_eal/common/eal_common_memzone.c
@@ -221,12 +221,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 		if ((flags & RTE_MEMZONE_1GB) &&
 				free_memseg[i].hugepage_sz == RTE_PGSIZE_2M)
 			continue;
+#ifdef RTE_ARCH_64
 		if ((flags & RTE_MEMZONE_16MB) &&
 				free_memseg[i].hugepage_sz == RTE_PGSIZE_16G)
 			continue;
 		if ((flags & RTE_MEMZONE_16GB) &&
 				free_memseg[i].hugepage_sz == RTE_PGSIZE_16M)
 			continue;
+#endif
 
 		/* this segment is the best until now */
 		if (memseg_idx == -1) {
diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 1990833..6bcb92b 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -53,12 +53,14 @@ extern "C" {
 #endif
 
 enum rte_page_sizes {
-	RTE_PGSIZE_4K = 1ULL << 12,
-	RTE_PGSIZE_2M = 1ULL << 21,
-	RTE_PGSIZE_1G = 1ULL << 30,
-	RTE_PGSIZE_64K = 1ULL << 16,
-	RTE_PGSIZE_16M = 1ULL << 24,
-	RTE_PGSIZE_16G = 1ULL << 34
+	RTE_PGSIZE_4K	= 1UL << 12,
+	RTE_PGSIZE_2M	= 1UL << 21,
+	RTE_PGSIZE_1G	= 1UL << 30,
+	RTE_PGSIZE_64K	= 1UL << 16,
+	RTE_PGSIZE_16M	= 1UL << 24,
+#ifdef RTE_ARCH_64
+	RTE_PGSIZE_16G	= 1UL << 34
+#endif
 };
 
 #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index e6cb919..833670c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -317,11 +317,10 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
 			hugepg_tbl[i].filepath[sizeof(hugepg_tbl[i].filepath) - 1] = '\0';
 		}
 #ifndef RTE_ARCH_64
-		/* for 32-bit systems, don't remap 1G and 16G pages, just reuse
-		 * original map address as final map address.
+		/* for 32-bit systems, don't remap 1G pages(16G not defined),
+		 * just reuse original map address as final map address.
 		 */
-		else if ((hugepage_sz == RTE_PGSIZE_1G)
-			|| (hugepage_sz == RTE_PGSIZE_16G)) {
+		else if (hugepage_sz == RTE_PGSIZE_1G) {
 			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
 			hugepg_tbl[i].orig_va = NULL;
 			continue;
@@ -422,11 +421,10 @@ remap_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
 	while (i < hpi->num_pages[0]) {
 
 #ifndef RTE_ARCH_64
-		/* for 32-bit systems, don't remap 1G pages and 16G pages,
+		/* for 32-bit systems, don't remap 1G pages(16G not defined,
 		 * just reuse original map address as final map address.
 		 */
-		if ((hugepage_sz == RTE_PGSIZE_1G)
-			|| (hugepage_sz == RTE_PGSIZE_16G)) {
+		if (hugepage_sz == RTE_PGSIZE_1G) {
 			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
 			hugepg_tbl[i].orig_va = NULL;
 			i++;
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2] Fix two compile issues with i686 platform

[Qiu, Michael]

On 12/4/2014 5:05 PM, Thomas Monjalon wrote:
> 2014-12-04 02:49, Qiu, Michael:
>> On 12/3/2014 11:40 PM, Richardson, Bruce wrote:
>>> On Wed, Dec 03, 2014 at 04:10:23PM +0800, Michael Qiu wrote:
>>>> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
>>>> is always false due to limited range of data type [-Werror=type-limits]
>>>>     || (hugepage_sz == RTE_PGSIZE_16G)) {
>>>>     ^
>>>> cc1: all warnings being treated as errors
>>>>
>>>> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
>>>> conversion from "long long" to "void *" may lose significant bits
>>>>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>>>>
>>>> This was introuduced by commit b77b5639:
>>>>         mem: add huge page sizes for IBM Power
>>>>
>>>> The root cause is that size_t and uintptr_t are 32-bit in i686
>>>> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>>>>
>>>> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
>>>> this issue.
>>>>
>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>> Minor comment below.
>>>
>>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>>>
>>>> ---
>>>>  app/test/test_memzone.c                    | 18 ++++++++++++------
>>>>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
>>>>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>>>>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>>>>  4 files changed, 27 insertions(+), 19 deletions(-)
>>>>
>>> ... snip ...
>>>> --- a/lib/librte_eal/common/include/rte_memory.h
>>>> +++ b/lib/librte_eal/common/include/rte_memory.h
>>>> @@ -53,12 +53,14 @@ extern "C" {
>>>>  #endif
>>>>  
>>>>  enum rte_page_sizes {
>>>> -	RTE_PGSIZE_4K = 1ULL << 12,
>>>> -	RTE_PGSIZE_2M = 1ULL << 21,
>>>> -	RTE_PGSIZE_1G = 1ULL << 30,
>>>> -	RTE_PGSIZE_64K = 1ULL << 16,
>>>> -	RTE_PGSIZE_16M = 1ULL << 24,
>>>> -	RTE_PGSIZE_16G = 1ULL << 34
>>>> +	RTE_PGSIZE_4K	= 1UL << 12,
>>>> +	RTE_PGSIZE_2M	= 1UL << 21,
>>>> +	RTE_PGSIZE_1G	= 1UL << 30,
>>>> +	RTE_PGSIZE_64K	= 1UL << 16,
>>>> +	RTE_PGSIZE_16M	= 1UL << 24,
>>>> +#ifdef RTE_ARCH_64
>>>> +	RTE_PGSIZE_16G	= 1ULL << 34
>>> you don't need the "LL" here as long type is 64-bits on 64-bit systems. Changing
>>> it to 1UL << 34 will keep all entries consistent.
>> Hi Thomas,
>>
>> Should I resend V3 patch to modify this or you can do it when you plan
>> to merge this patch?
> I could do the change by myself. But given that you had some problems to send
> the patch to the mailing list, please send the v3 to everyone.
> Then a review from Chao would be appreciated.

OK, I will send V3 to everyone.

Thanks,
Michael
>
> Thanks

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Qiu, Michael]

Hi Chao

Would you please take a look at this patch?

It's solved issue introduce by Power Arch support patch.

Your comments are very precious :)

Thanks,
Michael
On 12/5/2014 2:03 PM, Michael Qiu wrote:
> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> is always false due to limited range of data type [-Werror=type-limits]
>     || (hugepage_sz == RTE_PGSIZE_16G)) {
>     ^
> cc1: all warnings being treated as errors
>
> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> conversion from "long long" to "void *" may lose significant bits
>    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>
> This was introuduced by commit b77b5639:
>         mem: add huge page sizes for IBM Power
>
> The root cause is that size_t and uintptr_t are 32-bit in i686
> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>
> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> this issue.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  v3 ---> v2
> 	Change RTE_PGSIZE_16G from ULL to UL
> 	to keep all entries consistent
>
>  V2 ---> v1
> 	Change two type entries to one, and
> 	leave RTE_PGSIZE_16G only valid for
> 	64-bit platform
>
>  app/test/test_memzone.c                    | 18 ++++++++++++------
>  lib/librte_eal/common/eal_common_memzone.c |  2 ++
>  lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>  lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>  4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
> index 5da6903..7bab8b5 100644
> --- a/app/test/test_memzone.c
> +++ b/app/test/test_memzone.c
> @@ -145,8 +145,10 @@ test_memzone_reserve_flags(void)
>  			hugepage_1GB_avail = 1;
>  		if (ms[i].hugepage_sz == RTE_PGSIZE_16M)
>  			hugepage_16MB_avail = 1;
> +#ifdef RTE_ARCH_64
>  		if (ms[i].hugepage_sz == RTE_PGSIZE_16G)
>  			hugepage_16GB_avail = 1;
> +#endif
>  	}
>  	/* Display the availability of 2MB ,1GB, 16MB, 16GB pages */
>  	if (hugepage_2MB_avail)
> @@ -234,8 +236,8 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails unless
> -		 * HINT flag is indicated
> +		/* Check if 2MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_2MB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
> @@ -295,8 +297,9 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +#ifdef RTE_ARCH_64
> +		/* Check if 16GB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_16GB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
> @@ -318,7 +321,9 @@ test_memzone_reserve_flags(void)
>  				return -1;
>  			}
>  		}
> +#endif
>  	}
> +#ifdef RTE_ARCH_64
>  	/*As with 16MB tests above for 16GB huge page requests*/
>  	if (hugepage_16GB_avail) {
>  		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
> @@ -343,8 +348,8 @@ test_memzone_reserve_flags(void)
>  			return -1;
>  		}
>  
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +		/* Check if 16MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>  		 */
>  		if (!hugepage_16MB_avail) {
>  			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
> @@ -376,6 +381,7 @@ test_memzone_reserve_flags(void)
>  			}
>  		}
>  	}
> +#endif
>  	return 0;
>  }
>  
> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index b5a5d72..ee233ad 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -221,12 +221,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
>  		if ((flags & RTE_MEMZONE_1GB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_2M)
>  			continue;
> +#ifdef RTE_ARCH_64
>  		if ((flags & RTE_MEMZONE_16MB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_16G)
>  			continue;
>  		if ((flags & RTE_MEMZONE_16GB) &&
>  				free_memseg[i].hugepage_sz == RTE_PGSIZE_16M)
>  			continue;
> +#endif
>  
>  		/* this segment is the best until now */
>  		if (memseg_idx == -1) {
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 1990833..6bcb92b 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -53,12 +53,14 @@ extern "C" {
>  #endif
>  
>  enum rte_page_sizes {
> -	RTE_PGSIZE_4K = 1ULL << 12,
> -	RTE_PGSIZE_2M = 1ULL << 21,
> -	RTE_PGSIZE_1G = 1ULL << 30,
> -	RTE_PGSIZE_64K = 1ULL << 16,
> -	RTE_PGSIZE_16M = 1ULL << 24,
> -	RTE_PGSIZE_16G = 1ULL << 34
> +	RTE_PGSIZE_4K	= 1UL << 12,
> +	RTE_PGSIZE_2M	= 1UL << 21,
> +	RTE_PGSIZE_1G	= 1UL << 30,
> +	RTE_PGSIZE_64K	= 1UL << 16,
> +	RTE_PGSIZE_16M	= 1UL << 24,
> +#ifdef RTE_ARCH_64
> +	RTE_PGSIZE_16G	= 1UL << 34
> +#endif
>  };
>  
>  #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index e6cb919..833670c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -317,11 +317,10 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>  			hugepg_tbl[i].filepath[sizeof(hugepg_tbl[i].filepath) - 1] = '\0';
>  		}
>  #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G and 16G pages, just reuse
> -		 * original map address as final map address.
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined),
> +		 * just reuse original map address as final map address.
>  		 */
> -		else if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		else if (hugepage_sz == RTE_PGSIZE_1G) {
>  			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>  			hugepg_tbl[i].orig_va = NULL;
>  			continue;
> @@ -422,11 +421,10 @@ remap_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
>  	while (i < hpi->num_pages[0]) {
>  
>  #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G pages and 16G pages,
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined,
>  		 * just reuse original map address as final map address.
>  		 */
> -		if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		if (hugepage_sz == RTE_PGSIZE_1G) {
>  			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>  			hugepg_tbl[i].orig_va = NULL;
>  			i++;

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Chao Zhu]

Michael,

I'm looking at it. I'll give you feedback soon.

On 2014/12/5 14:56, Qiu, Michael wrote:
> Hi Chao
>
> Would you please take a look at this patch?
>
> It's solved issue introduce by Power Arch support patch.
>
> Your comments are very precious :)
>
> Thanks,
> Michael
> On 12/5/2014 2:03 PM, Michael Qiu wrote:
>> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
>> is always false due to limited range of data type [-Werror=type-limits]
>>      || (hugepage_sz == RTE_PGSIZE_16G)) {
>>      ^
>> cc1: all warnings being treated as errors
>>
>> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
>> conversion from "long long" to "void *" may lose significant bits
>>     RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>>
>> This was introuduced by commit b77b5639:
>>          mem: add huge page sizes for IBM Power
>>
>> The root cause is that size_t and uintptr_t are 32-bit in i686
>> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>>
>> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
>> this issue.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>> ---
>>   v3 ---> v2
>> 	Change RTE_PGSIZE_16G from ULL to UL
>> 	to keep all entries consistent
>>
>>   V2 ---> v1
>> 	Change two type entries to one, and
>> 	leave RTE_PGSIZE_16G only valid for
>> 	64-bit platform
>>
>>   app/test/test_memzone.c                    | 18 ++++++++++++------
>>   lib/librte_eal/common/eal_common_memzone.c |  2 ++
>>   lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>>   lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>>   4 files changed, 27 insertions(+), 19 deletions(-)
>>
>> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
>> index 5da6903..7bab8b5 100644
>> --- a/app/test/test_memzone.c
>> +++ b/app/test/test_memzone.c
>> @@ -145,8 +145,10 @@ test_memzone_reserve_flags(void)
>>   			hugepage_1GB_avail = 1;
>>   		if (ms[i].hugepage_sz == RTE_PGSIZE_16M)
>>   			hugepage_16MB_avail = 1;
>> +#ifdef RTE_ARCH_64
>>   		if (ms[i].hugepage_sz == RTE_PGSIZE_16G)
>>   			hugepage_16GB_avail = 1;
>> +#endif
>>   	}
>>   	/* Display the availability of 2MB ,1GB, 16MB, 16GB pages */
>>   	if (hugepage_2MB_avail)
>> @@ -234,8 +236,8 @@ test_memzone_reserve_flags(void)
>>   			return -1;
>>   		}
>>   
>> -		/* Check if 1GB huge pages are unavailable, that function fails unless
>> -		 * HINT flag is indicated
>> +		/* Check if 2MB huge pages are unavailable, that function
>> +		 * fails unless HINT flag is indicated
>>   		 */
>>   		if (!hugepage_2MB_avail) {
>>   			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
>> @@ -295,8 +297,9 @@ test_memzone_reserve_flags(void)
>>   			return -1;
>>   		}
>>   
>> -		/* Check if 1GB huge pages are unavailable, that function fails
>> -		 * unless HINT flag is indicated
>> +#ifdef RTE_ARCH_64
>> +		/* Check if 16GB huge pages are unavailable, that function
>> +		 * fails unless HINT flag is indicated
>>   		 */
>>   		if (!hugepage_16GB_avail) {
>>   			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
>> @@ -318,7 +321,9 @@ test_memzone_reserve_flags(void)
>>   				return -1;
>>   			}
>>   		}
>> +#endif
>>   	}
>> +#ifdef RTE_ARCH_64
>>   	/*As with 16MB tests above for 16GB huge page requests*/
>>   	if (hugepage_16GB_avail) {
>>   		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
>> @@ -343,8 +348,8 @@ test_memzone_reserve_flags(void)
>>   			return -1;
>>   		}
>>   
>> -		/* Check if 1GB huge pages are unavailable, that function fails
>> -		 * unless HINT flag is indicated
>> +		/* Check if 16MB huge pages are unavailable, that function
>> +		 * fails unless HINT flag is indicated
>>   		 */
>>   		if (!hugepage_16MB_avail) {
>>   			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
>> @@ -376,6 +381,7 @@ test_memzone_reserve_flags(void)
>>   			}
>>   		}
>>   	}
>> +#endif
>>   	return 0;
>>   }
>>   
>> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
>> index b5a5d72..ee233ad 100644
>> --- a/lib/librte_eal/common/eal_common_memzone.c
>> +++ b/lib/librte_eal/common/eal_common_memzone.c
>> @@ -221,12 +221,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
>>   		if ((flags & RTE_MEMZONE_1GB) &&
>>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_2M)
>>   			continue;
>> +#ifdef RTE_ARCH_64
>>   		if ((flags & RTE_MEMZONE_16MB) &&
>>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_16G)
>>   			continue;
>>   		if ((flags & RTE_MEMZONE_16GB) &&
>>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_16M)
>>   			continue;
>> +#endif
>>   
>>   		/* this segment is the best until now */
>>   		if (memseg_idx == -1) {
>> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
>> index 1990833..6bcb92b 100644
>> --- a/lib/librte_eal/common/include/rte_memory.h
>> +++ b/lib/librte_eal/common/include/rte_memory.h
>> @@ -53,12 +53,14 @@ extern "C" {
>>   #endif
>>   
>>   enum rte_page_sizes {
>> -	RTE_PGSIZE_4K = 1ULL << 12,
>> -	RTE_PGSIZE_2M = 1ULL << 21,
>> -	RTE_PGSIZE_1G = 1ULL << 30,
>> -	RTE_PGSIZE_64K = 1ULL << 16,
>> -	RTE_PGSIZE_16M = 1ULL << 24,
>> -	RTE_PGSIZE_16G = 1ULL << 34
>> +	RTE_PGSIZE_4K	= 1UL << 12,
>> +	RTE_PGSIZE_2M	= 1UL << 21,
>> +	RTE_PGSIZE_1G	= 1UL << 30,
>> +	RTE_PGSIZE_64K	= 1UL << 16,
>> +	RTE_PGSIZE_16M	= 1UL << 24,
>> +#ifdef RTE_ARCH_64
>> +	RTE_PGSIZE_16G	= 1UL << 34
>> +#endif
>>   };
>>   
>>   #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index e6cb919..833670c 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -317,11 +317,10 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>   			hugepg_tbl[i].filepath[sizeof(hugepg_tbl[i].filepath) - 1] = '\0';
>>   		}
>>   #ifndef RTE_ARCH_64
>> -		/* for 32-bit systems, don't remap 1G and 16G pages, just reuse
>> -		 * original map address as final map address.
>> +		/* for 32-bit systems, don't remap 1G pages(16G not defined),
>> +		 * just reuse original map address as final map address.
>>   		 */
>> -		else if ((hugepage_sz == RTE_PGSIZE_1G)
>> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
>> +		else if (hugepage_sz == RTE_PGSIZE_1G) {
>>   			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>>   			hugepg_tbl[i].orig_va = NULL;
>>   			continue;
>> @@ -422,11 +421,10 @@ remap_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
>>   	while (i < hpi->num_pages[0]) {
>>   
>>   #ifndef RTE_ARCH_64
>> -		/* for 32-bit systems, don't remap 1G pages and 16G pages,
>> +		/* for 32-bit systems, don't remap 1G pages(16G not defined,
>>   		 * just reuse original map address as final map address.
>>   		 */
>> -		if ((hugepage_sz == RTE_PGSIZE_1G)
>> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
>> +		if (hugepage_sz == RTE_PGSIZE_1G) {
>>   			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>>   			hugepg_tbl[i].orig_va = NULL;
>>   			i++;
>

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Chao Zhu]

On 2014/12/4 17:12, Michael Qiu wrote:
> lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> is always false due to limited range of data type [-Werror=type-limits]
>      || (hugepage_sz == RTE_PGSIZE_16G)) {
>      ^
> cc1: all warnings being treated as errors
>
> lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> conversion from "long long" to "void *" may lose significant bits
>     RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
>
> This was introuduced by commit b77b5639:
>          mem: add huge page sizes for IBM Power
>
> The root cause is that size_t and uintptr_t are 32-bit in i686
> platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
>
> Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> this issue.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>   v3 ---> v2
> 	Change RTE_PGSIZE_16G from ULL to UL
> 	to keep all entries consistent
>
>   V2 ---> v1
> 	Change two type entries to one, and
> 	leave RTE_PGSIZE_16G only valid for
> 	64-bit platform
>
>   app/test/test_memzone.c                    | 18 ++++++++++++------
>   lib/librte_eal/common/eal_common_memzone.c |  2 ++
>   lib/librte_eal/common/include/rte_memory.h | 14 ++++++++------
>   lib/librte_eal/linuxapp/eal/eal_memory.c   | 12 +++++-------
>   4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
> index 5da6903..7bab8b5 100644
> --- a/app/test/test_memzone.c
> +++ b/app/test/test_memzone.c
> @@ -145,8 +145,10 @@ test_memzone_reserve_flags(void)
>   			hugepage_1GB_avail = 1;
>   		if (ms[i].hugepage_sz == RTE_PGSIZE_16M)
>   			hugepage_16MB_avail = 1;
> +#ifdef RTE_ARCH_64
>   		if (ms[i].hugepage_sz == RTE_PGSIZE_16G)
>   			hugepage_16GB_avail = 1;
> +#endif
>   	}
>   	/* Display the availability of 2MB ,1GB, 16MB, 16GB pages */
>   	if (hugepage_2MB_avail)
> @@ -234,8 +236,8 @@ test_memzone_reserve_flags(void)
>   			return -1;
>   		}
>
> -		/* Check if 1GB huge pages are unavailable, that function fails unless
> -		 * HINT flag is indicated
> +		/* Check if 2MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>   		 */
>   		if (!hugepage_2MB_avail) {
>   			mz = rte_memzone_reserve("flag_zone_2M_HINT", size, SOCKET_ID_ANY,
> @@ -295,8 +297,9 @@ test_memzone_reserve_flags(void)
>   			return -1;
>   		}
>
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +#ifdef RTE_ARCH_64
> +		/* Check if 16GB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>   		 */
>   		if (!hugepage_16GB_avail) {
>   			mz = rte_memzone_reserve("flag_zone_16G_HINT", size,
> @@ -318,7 +321,9 @@ test_memzone_reserve_flags(void)
>   				return -1;
>   			}
>   		}
> +#endif
>   	}
> +#ifdef RTE_ARCH_64
>   	/*As with 16MB tests above for 16GB huge page requests*/
>   	if (hugepage_16GB_avail) {
>   		mz = rte_memzone_reserve("flag_zone_16G", size, SOCKET_ID_ANY,
> @@ -343,8 +348,8 @@ test_memzone_reserve_flags(void)
>   			return -1;
>   		}
>
> -		/* Check if 1GB huge pages are unavailable, that function fails
> -		 * unless HINT flag is indicated
> +		/* Check if 16MB huge pages are unavailable, that function
> +		 * fails unless HINT flag is indicated
>   		 */
>   		if (!hugepage_16MB_avail) {
>   			mz = rte_memzone_reserve("flag_zone_16M_HINT", size,
> @@ -376,6 +381,7 @@ test_memzone_reserve_flags(void)
>   			}
>   		}
>   	}
> +#endif
>   	return 0;
>   }
>
> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index b5a5d72..ee233ad 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -221,12 +221,14 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
>   		if ((flags & RTE_MEMZONE_1GB) &&
>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_2M)
>   			continue;
> +#ifdef RTE_ARCH_64
>   		if ((flags & RTE_MEMZONE_16MB) &&
>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_16G)
>   			continue;
>   		if ((flags & RTE_MEMZONE_16GB) &&
>   				free_memseg[i].hugepage_sz == RTE_PGSIZE_16M)
>   			continue;
> +#endif
>
>   		/* this segment is the best until now */
>   		if (memseg_idx == -1) {
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 1990833..6bcb92b 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -53,12 +53,14 @@ extern "C" {
>   #endif
>
>   enum rte_page_sizes {
> -	RTE_PGSIZE_4K = 1ULL << 12,
> -	RTE_PGSIZE_2M = 1ULL << 21,
> -	RTE_PGSIZE_1G = 1ULL << 30,
> -	RTE_PGSIZE_64K = 1ULL << 16,
> -	RTE_PGSIZE_16M = 1ULL << 24,
> -	RTE_PGSIZE_16G = 1ULL << 34
> +	RTE_PGSIZE_4K	= 1UL << 12,
> +	RTE_PGSIZE_2M	= 1UL << 21,
> +	RTE_PGSIZE_1G	= 1UL << 30,
> +	RTE_PGSIZE_64K	= 1UL << 16,
> +	RTE_PGSIZE_16M	= 1UL << 24,
> +#ifdef RTE_ARCH_64
> +	RTE_PGSIZE_16G	= 1UL << 34
> +#endif
>   };
>
>   #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index e6cb919..833670c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -317,11 +317,10 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>   			hugepg_tbl[i].filepath[sizeof(hugepg_tbl[i].filepath) - 1] = '\0';
>   		}
>   #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G and 16G pages, just reuse
> -		 * original map address as final map address.
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined),
> +		 * just reuse original map address as final map address.
>   		 */
> -		else if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		else if (hugepage_sz == RTE_PGSIZE_1G) {
>   			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>   			hugepg_tbl[i].orig_va = NULL;
>   			continue;
> @@ -422,11 +421,10 @@ remap_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)
>   	while (i < hpi->num_pages[0]) {
>
>   #ifndef RTE_ARCH_64
> -		/* for 32-bit systems, don't remap 1G pages and 16G pages,
> +		/* for 32-bit systems, don't remap 1G pages(16G not defined,
>   		 * just reuse original map address as final map address.
>   		 */
> -		if ((hugepage_sz == RTE_PGSIZE_1G)
> -			|| (hugepage_sz == RTE_PGSIZE_16G)) {
> +		if (hugepage_sz == RTE_PGSIZE_1G) {
>   			hugepg_tbl[i].final_va = hugepg_tbl[i].orig_va;
>   			hugepg_tbl[i].orig_va = NULL;
>   			i++;
This patch works on IBM PPC64.

Acked-by: Chao Zhu<chaozhu@linux.vnet.ibm.com>

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Neil Horman]

On Fri, Dec 05, 2014 at 04:31:47PM +0800, Chao Zhu wrote:
> 
> On 2014/12/4 17:12, Michael Qiu wrote:
> >lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> >is always false due to limited range of data type [-Werror=type-limits]
> >     || (hugepage_sz == RTE_PGSIZE_16G)) {
> >     ^
> >cc1: all warnings being treated as errors
> >
> >lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> >conversion from "long long" to "void *" may lose significant bits
> >    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
> >
> >This was introuduced by commit b77b5639:
> >         mem: add huge page sizes for IBM Power
> >
> >The root cause is that size_t and uintptr_t are 32-bit in i686
> >platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
> >
> >Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> >this issue.
> >
> >Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> >---
> >  v3 ---> v2
> >	Change RTE_PGSIZE_16G from ULL to UL
> >	to keep all entries consistent
> >
> >  V2 ---> v1
> >	Change two type entries to one, and
> >	leave RTE_PGSIZE_16G only valid for
> >	64-bit platform
> >
NACK, this is the wrong way to fix this problem.  Pagesizes are independent of
architecutre.  While a system can't have a hugepage size that exceeds its
virtual address limit, theres no need to do per-architecture special casing of
page sizes here.  Instead of littering the code with ifdef RTE_ARCH_64
everytime you want to check a page size, just convert the size_t to a uint64_t
and you can allow all of the enumerated page types on all architecutres, and
save yourself some ifdeffing in the process.

Neil

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Bruce Richardson]

On Fri, Dec 05, 2014 at 09:22:05AM -0500, Neil Horman wrote:
> On Fri, Dec 05, 2014 at 04:31:47PM +0800, Chao Zhu wrote:
> > 
> > On 2014/12/4 17:12, Michael Qiu wrote:
> > >lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> > >is always false due to limited range of data type [-Werror=type-limits]
> > >     || (hugepage_sz == RTE_PGSIZE_16G)) {
> > >     ^
> > >cc1: all warnings being treated as errors
> > >
> > >lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> > >conversion from "long long" to "void *" may lose significant bits
> > >    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
> > >
> > >This was introuduced by commit b77b5639:
> > >         mem: add huge page sizes for IBM Power
> > >
> > >The root cause is that size_t and uintptr_t are 32-bit in i686
> > >platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
> > >
> > >Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> > >this issue.
> > >
> > >Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > >---
> > >  v3 ---> v2
> > >	Change RTE_PGSIZE_16G from ULL to UL
> > >	to keep all entries consistent
> > >
> > >  V2 ---> v1
> > >	Change two type entries to one, and
> > >	leave RTE_PGSIZE_16G only valid for
> > >	64-bit platform
> > >
> NACK, this is the wrong way to fix this problem.  Pagesizes are independent of
> architecutre.  While a system can't have a hugepage size that exceeds its
> virtual address limit, theres no need to do per-architecture special casing of
> page sizes here.  Instead of littering the code with ifdef RTE_ARCH_64
> everytime you want to check a page size, just convert the size_t to a uint64_t
> and you can allow all of the enumerated page types on all architecutres, and
> save yourself some ifdeffing in the process.
> 
> Neil

While I get your point, I find it distasteful to use a uint64_t for memory sizes,
when there is the size_t type defined for that particular purpose.
However, I suppose that reducing the number of #ifdefs compared to using the
"correct" datatypes for objects is a more practical optino - however distastful
I find it.

/Bruce

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Neil Horman]

On Fri, Dec 05, 2014 at 03:02:33PM +0000, Bruce Richardson wrote:
> On Fri, Dec 05, 2014 at 09:22:05AM -0500, Neil Horman wrote:
> > On Fri, Dec 05, 2014 at 04:31:47PM +0800, Chao Zhu wrote:
> > > 
> > > On 2014/12/4 17:12, Michael Qiu wrote:
> > > >lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
> > > >is always false due to limited range of data type [-Werror=type-limits]
> > > >     || (hugepage_sz == RTE_PGSIZE_16G)) {
> > > >     ^
> > > >cc1: all warnings being treated as errors
> > > >
> > > >lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
> > > >conversion from "long long" to "void *" may lose significant bits
> > > >    RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
> > > >
> > > >This was introuduced by commit b77b5639:
> > > >         mem: add huge page sizes for IBM Power
> > > >
> > > >The root cause is that size_t and uintptr_t are 32-bit in i686
> > > >platform, but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.
> > > >
> > > >Define RTE_PGSIZE_16G only in 64 bit platform to avoid
> > > >this issue.
> > > >
> > > >Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > > >---
> > > >  v3 ---> v2
> > > >	Change RTE_PGSIZE_16G from ULL to UL
> > > >	to keep all entries consistent
> > > >
> > > >  V2 ---> v1
> > > >	Change two type entries to one, and
> > > >	leave RTE_PGSIZE_16G only valid for
> > > >	64-bit platform
> > > >
> > NACK, this is the wrong way to fix this problem.  Pagesizes are independent of
> > architecutre.  While a system can't have a hugepage size that exceeds its
> > virtual address limit, theres no need to do per-architecture special casing of
> > page sizes here.  Instead of littering the code with ifdef RTE_ARCH_64
> > everytime you want to check a page size, just convert the size_t to a uint64_t
> > and you can allow all of the enumerated page types on all architecutres, and
> > save yourself some ifdeffing in the process.
> > 
> > Neil
> 
> While I get your point, I find it distasteful to use a uint64_t for memory sizes,
> when there is the size_t type defined for that particular purpose.
> However, I suppose that reducing the number of #ifdefs compared to using the
> "correct" datatypes for objects is a more practical optino - however distastful
> I find it.

size_t isn't defined for memory sizes in the sense that we're using them here.
size_t is meant to address the need for a type to describe object sizes on a
particular system, and it itself is sized accordingly (32 bits on a 32 bit arch,
64 on 64), so that you can safely store a size that the system in question might
maximally allocate/return.  In this situation we are describing memory sizes
that might occur no a plurality of arches, and so size_t is inappropriate
because it as a type is not sized for anything other than the arch it is being
built for.  The pragmatic benefits of ennumerating page sizes in a single
canonical location far outweigh the desire to use a misappropriated type to
describe them.

Neil

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Qiu, Michael]

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

Neil,

This patch fix two compile issues, and we need to do *dpdk testing
affairs*,  if it is blocked in build stage, we can do *nothing* for testing.

I've get you mind and your concern. But we should take care of changing
the type of "hugepage_sz", because lots of places using it.

Would you mind if we consider this as hot fix, and we can post a better
fix later(like in dpdk 2.0)? Otherwise all test cycle are blocked.

Thanks,
Michael
> Neil
>
>

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Neil Horman]

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

Regards
Neil



diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 412b432..31a391c 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -96,7 +96,7 @@ rte_dump_physmem_layout(FILE *f)
 
 		fprintf(f, "Segment %u: phys:0x%"PRIx64", len:%zu, "
 		       "virt:%p, socket_id:%"PRId32", "
-		       "hugepage_sz:%zu, nchannel:%"PRIx32", "
+		       "hugepage_sz:%llu, nchannel:%"PRIx32", "
 		       "nrank:%"PRIx32"\n", i,
 		       mcfg->memseg[i].phys_addr,
 		       mcfg->memseg[i].len,
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index aac6abf..e2ecb0d 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -49,7 +49,7 @@
  * mount points of hugepages
  */
 struct hugepage_info {
-	size_t hugepage_sz;   /**< size of a huge page */
+	uint64_t hugepage_sz;   /**< size of a huge page */
 	const char *hugedir;    /**< dir where hugetlbfs is mounted */
 	uint32_t num_pages[RTE_MAX_NUMA_NODES];
 				/**< number of hugepages of that size on each socket */
diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 1990833..7f8103f 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -92,7 +92,7 @@ struct rte_memseg {
 	phys_addr_t ioremap_addr; /**< Real physical address inside the VM */
 #endif
 	size_t len;               /**< Length of the segment. */
-	size_t hugepage_sz;       /**< The pagesize of underlying memory */
+	uint64_t hugepage_sz;       /**< The pagesize of underlying memory */
 	int32_t socket_id;          /**< NUMA socket ID. */
 	uint32_t nchannel;          /**< Number of channels. */
 	uint32_t nrank;             /**< Number of ranks. */
diff --git a/lib/librte_eal/common/include/rte_memzone.h b/lib/librte_eal/common/include/rte_memzone.h
index 7d47bff..3006e81 100644
--- a/lib/librte_eal/common/include/rte_memzone.h
+++ b/lib/librte_eal/common/include/rte_memzone.h
@@ -83,7 +83,7 @@ struct rte_memzone {
 #endif
 	size_t len;                       /**< Length of the memzone. */
 
-	size_t hugepage_sz;               /**< The page size of underlying memory */
+	uint64_t hugepage_sz;               /**< The page size of underlying memory */
 
 	int32_t socket_id;                /**< NUMA socket ID. */
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index e6cb919..bae2507 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -300,7 +300,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
 #endif
 
 	for (i = 0; i < hpi->num_pages[0]; i++) {
-		size_t hugepage_sz = hpi->hugepage_sz;
+		uint64_t hugepage_sz = hpi->hugepage_sz;
 
 		if (orig) {
 			hugepg_tbl[i].file_id = i;

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Qiu, Michael]

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

We test dpdk upstream now as 1,8 rc2 and rc3 released :)

I know that what you mean. but lots of please using "hugepage_sz" do you
confirm it will not affect other issue?

On other hand, we use 32 bit address in 32 bit platform for better
performance(some of places also use uintptr_t for address check and
alignment).

And it should not acceptable in 32 bit platform to use 64-bit platform
specification affairs(like RTE_PGSIZE_16G).

Thanks,
Michael

>
>
> diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
> index 412b432..31a391c 100644
> --- a/lib/librte_eal/common/eal_common_memory.c
> +++ b/lib/librte_eal/common/eal_common_memory.c
> @@ -96,7 +96,7 @@ rte_dump_physmem_layout(FILE *f)
>  
>  		fprintf(f, "Segment %u: phys:0x%"PRIx64", len:%zu, "
>  		       "virt:%p, socket_id:%"PRId32", "
> -		       "hugepage_sz:%zu, nchannel:%"PRIx32", "
> +		       "hugepage_sz:%llu, nchannel:%"PRIx32", "
>  		       "nrank:%"PRIx32"\n", i,
>  		       mcfg->memseg[i].phys_addr,
>  		       mcfg->memseg[i].len,
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index aac6abf..e2ecb0d 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -49,7 +49,7 @@
>   * mount points of hugepages
>   */
>  struct hugepage_info {
> -	size_t hugepage_sz;   /**< size of a huge page */
> +	uint64_t hugepage_sz;   /**< size of a huge page */
>  	const char *hugedir;    /**< dir where hugetlbfs is mounted */
>  	uint32_t num_pages[RTE_MAX_NUMA_NODES];
>  				/**< number of hugepages of that size on each socket */
> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
> index 1990833..7f8103f 100644
> --- a/lib/librte_eal/common/include/rte_memory.h
> +++ b/lib/librte_eal/common/include/rte_memory.h
> @@ -92,7 +92,7 @@ struct rte_memseg {
>  	phys_addr_t ioremap_addr; /**< Real physical address inside the VM */
>  #endif
>  	size_t len;               /**< Length of the segment. */
> -	size_t hugepage_sz;       /**< The pagesize of underlying memory */
> +	uint64_t hugepage_sz;       /**< The pagesize of underlying memory */
>  	int32_t socket_id;          /**< NUMA socket ID. */
>  	uint32_t nchannel;          /**< Number of channels. */
>  	uint32_t nrank;             /**< Number of ranks. */
> diff --git a/lib/librte_eal/common/include/rte_memzone.h b/lib/librte_eal/common/include/rte_memzone.h
> index 7d47bff..3006e81 100644
> --- a/lib/librte_eal/common/include/rte_memzone.h
> +++ b/lib/librte_eal/common/include/rte_memzone.h
> @@ -83,7 +83,7 @@ struct rte_memzone {
>  #endif
>  	size_t len;                       /**< Length of the memzone. */
>  
> -	size_t hugepage_sz;               /**< The page size of underlying memory */
> +	uint64_t hugepage_sz;               /**< The page size of underlying memory */
>  
>  	int32_t socket_id;                /**< NUMA socket ID. */
>  
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index e6cb919..bae2507 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -300,7 +300,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>  #endif
>  
>  	for (i = 0; i < hpi->num_pages[0]; i++) {
> -		size_t hugepage_sz = hpi->hugepage_sz;
> +		uint64_t hugepage_sz = hpi->hugepage_sz;
>  
>  		if (orig) {
>  			hugepg_tbl[i].file_id = i;
>

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Qiu, Michael]

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

Sorry, please/places
> confirm it will not affect other issue?
>
> On other hand, we use 32 bit address in 32 bit platform for better
> performance(some of places also use uintptr_t for address check and
> alignment).
>
> And it should not acceptable in 32 bit platform to use 64-bit platform
> specification affairs(like RTE_PGSIZE_16G).
>
> Thanks,
> Michael
>
>>
>> diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
>> index 412b432..31a391c 100644
>> --- a/lib/librte_eal/common/eal_common_memory.c
>> +++ b/lib/librte_eal/common/eal_common_memory.c
>> @@ -96,7 +96,7 @@ rte_dump_physmem_layout(FILE *f)
>>  
>>  		fprintf(f, "Segment %u: phys:0x%"PRIx64", len:%zu, "
>>  		       "virt:%p, socket_id:%"PRId32", "
>> -		       "hugepage_sz:%zu, nchannel:%"PRIx32", "
>> +		       "hugepage_sz:%llu, nchannel:%"PRIx32", "
>>  		       "nrank:%"PRIx32"\n", i,
>>  		       mcfg->memseg[i].phys_addr,
>>  		       mcfg->memseg[i].len,
>> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
>> index aac6abf..e2ecb0d 100644
>> --- a/lib/librte_eal/common/eal_internal_cfg.h
>> +++ b/lib/librte_eal/common/eal_internal_cfg.h
>> @@ -49,7 +49,7 @@
>>   * mount points of hugepages
>>   */
>>  struct hugepage_info {
>> -	size_t hugepage_sz;   /**< size of a huge page */
>> +	uint64_t hugepage_sz;   /**< size of a huge page */
>>  	const char *hugedir;    /**< dir where hugetlbfs is mounted */
>>  	uint32_t num_pages[RTE_MAX_NUMA_NODES];
>>  				/**< number of hugepages of that size on each socket */
>> diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
>> index 1990833..7f8103f 100644
>> --- a/lib/librte_eal/common/include/rte_memory.h
>> +++ b/lib/librte_eal/common/include/rte_memory.h
>> @@ -92,7 +92,7 @@ struct rte_memseg {
>>  	phys_addr_t ioremap_addr; /**< Real physical address inside the VM */
>>  #endif
>>  	size_t len;               /**< Length of the segment. */
>> -	size_t hugepage_sz;       /**< The pagesize of underlying memory */
>> +	uint64_t hugepage_sz;       /**< The pagesize of underlying memory */
>>  	int32_t socket_id;          /**< NUMA socket ID. */
>>  	uint32_t nchannel;          /**< Number of channels. */
>>  	uint32_t nrank;             /**< Number of ranks. */
>> diff --git a/lib/librte_eal/common/include/rte_memzone.h b/lib/librte_eal/common/include/rte_memzone.h
>> index 7d47bff..3006e81 100644
>> --- a/lib/librte_eal/common/include/rte_memzone.h
>> +++ b/lib/librte_eal/common/include/rte_memzone.h
>> @@ -83,7 +83,7 @@ struct rte_memzone {
>>  #endif
>>  	size_t len;                       /**< Length of the memzone. */
>>  
>> -	size_t hugepage_sz;               /**< The page size of underlying memory */
>> +	uint64_t hugepage_sz;               /**< The page size of underlying memory */
>>  
>>  	int32_t socket_id;                /**< NUMA socket ID. */
>>  
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index e6cb919..bae2507 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -300,7 +300,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>  #endif
>>  
>>  	for (i = 0; i < hpi->num_pages[0]; i++) {
>> -		size_t hugepage_sz = hpi->hugepage_sz;
>> +		uint64_t hugepage_sz = hpi->hugepage_sz;
>>  
>>  		if (orig) {
>>  			hugepg_tbl[i].file_id = i;
>>
>

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Neil Horman]

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

> I know that what you mean. but lots of please using "hugepage_sz" do you
> confirm it will not affect other issue?
> 
5.  There are 5 placees that use hugepage_sz, as the patch below indicates.
Thats no alot.

Also, I take issue with the assertion that this patch creates no additional
problems.  I'm sure it creates no additional problems that your patch wouldn't
also create, arguably less.  If we were really being pragmatic here, I would
point out that this problem was caused by the fact that support for an entire
new architecture was integrated during the -rc phase of a release, which seems
extreemely risky to me, and as such, the most appropriate thing to do would be
to back support for ppc out until after the 1.8 release when it could be
properly tested.  Instead we are slamming in hacked up fixes that throw arch
specific ifdefs througout the code.

> On other hand, we use 32 bit address in 32 bit platform for better
> performance(some of places also use uintptr_t for address check and
> alignment).
> 
This has nothing to do with address bus size.

> And it should not acceptable in 32 bit platform to use 64-bit platform
> specification affairs(like RTE_PGSIZE_16G).
> 
Ok, so add a single arch specific runtime check during hugepage mapping to exit
on the 16G size use on 32 bit systems.  Thats a fair and reasonable thing to do,
though I think the hugepage remap is already ifdeffed for 54 bit arches only.

Neil

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Thomas Monjalon]

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

I agree. Neil's patch seems a lot better.

> > I know that what you mean. but lots of please using "hugepage_sz" do you
> > confirm it will not affect other issue?
> > 
> 5.  There are 5 placees that use hugepage_sz, as the patch below indicates.
> Thats no alot.
> 
> Also, I take issue with the assertion that this patch creates no additional
> problems.  I'm sure it creates no additional problems that your patch wouldn't
> also create, arguably less.  If we were really being pragmatic here, I would
> point out that this problem was caused by the fact that support for an entire
> new architecture was integrated during the -rc phase of a release, which seems
> extreemely risky to me,

No, it was integrated between -rc1 and -rc2. But -rc1 was not really a release
candidate. It was a first step after mbuf rework. This tag was requested for
validation but it was a bad idea. We won't create such wrong tag anymore.
</digress>
PPC was integrated before the real RC phase.

> and as such, the most appropriate thing to do would be
> to back support for ppc out until after the 1.8 release when it could be
> properly tested.  Instead we are slamming in hacked up fixes that throw arch
> specific ifdefs througout the code.

I think we can fix it (without ugly ifdefs) and avoid going back.
Thanks for your help.
-- 
Thomas

> > On other hand, we use 32 bit address in 32 bit platform for better
> > performance(some of places also use uintptr_t for address check and
> > alignment).
> > 
> This has nothing to do with address bus size.
> 
> > And it should not acceptable in 32 bit platform to use 64-bit platform
> > specification affairs(like RTE_PGSIZE_16G).
> > 
> Ok, so add a single arch specific runtime check during hugepage mapping to exit
> on the 16G size use on 32 bit systems.  Thats a fair and reasonable thing to do,
> though I think the hugepage remap is already ifdeffed for 54 bit arches only.
> 
> Neil

Re: [dpdk-dev] [PATCH v3] Fix two compile issues with i686 platform

[Qiu, Michael]

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

Agree :)

>> I know that what you mean. but lots of please using "hugepage_sz" do you
>> confirm it will not affect other issue?
>>
> 5.  There are 5 placees that use hugepage_sz, as the patch below indicates.
> Thats no alot.
>
> Also, I take issue with the assertion that this patch creates no additional
> problems.  I'm sure it creates no additional problems that your patch wouldn't
> also create, arguably less.  If we were really being pragmatic here, I would
> point out that this problem was caused by the fact that support for an entire
> new architecture was integrated during the -rc phase of a release, which seems
> extreemely risky to me, and as such, the most appropriate thing to do would be
> to back support for ppc out until after the 1.8 release when it could be
> properly tested.  Instead we are slamming in hacked up fixes that throw arch
> specific ifdefs througout the code.
>
>> On other hand, we use 32 bit address in 32 bit platform for better
>> performance(some of places also use uintptr_t for address check and
>> alignment).
>>
> This has nothing to do with address bus size.

Actually, it does, this is one of what I'm fixed. But it also introduced
by support Power Arch.

Other places I have not check yet.

Anyway, I will verify your solution, and to see any potential issues.

Thanks
Michael
>> And it should not acceptable in 32 bit platform to use 64-bit platform
>> specification affairs(like RTE_PGSIZE_16G).
>>
> Ok, so add a single arch specific runtime check during hugepage mapping to exit
> on the 16G size use on 32 bit systems.  Thats a fair and reasonable thing to do,
> though I think the hugepage remap is already ifdeffed for 54 bit arches only.
>
> Neil
>
>

[dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Michael Qiu]

These two issues are both introuduced by commit b77b5639:
        mem: add huge page sizes for IBM Power

Michael Qiu (2):
  Fix compile issue with hugepage_sz in 32-bit system
  Fix compile issue of eal with icc compile

 lib/librte_eal/common/eal_common_memory.c   | 2 +-
 lib/librte_eal/common/eal_internal_cfg.h    | 2 +-
 lib/librte_eal/common/include/rte_memory.h  | 2 +-
 lib/librte_eal/common/include/rte_memzone.h | 2 +-
 lib/librte_eal/linuxapp/eal/eal.c           | 2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c    | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2 v4] Fix compile issue with hugepage_sz in 32-bit system

[Michael Qiu]

lib/librte_eal/linuxapp/eal/eal_memory.c:324:4: error: comparison
is always false due to limited range of data type [-Werror=type-limits]
    || (hugepage_sz == RTE_PGSIZE_16G)) {
    ^
cc1: all warnings being treated as errors

This was introuduced by commit b77b5639:
        mem: add huge page sizes for IBM Power

The root cause is that size_t is 32-bit in i686 platform,
but RTE_PGSIZE_16M and RTE_PGSIZE_16G are always 64-bit.

Force hugepage_sz to always 64-bit to avoid this issue.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 v4 ---> v3
	Change hugepage_sz from size_t to uint64_t
	split second bugfix to another patch

 v3 ---> v2
	Change RTE_PGSIZE_16G from ULL to UL
	to keep all entries consistent

 V2 ---> v1
	Change two type entries to one, and
	leave RTE_PGSIZE_16G only valid for
	64-bit platform

 lib/librte_eal/common/eal_common_memory.c   | 2 +-
 lib/librte_eal/common/eal_internal_cfg.h    | 2 +-
 lib/librte_eal/common/include/rte_memory.h  | 2 +-
 lib/librte_eal/common/include/rte_memzone.h | 2 +-
 lib/librte_eal/linuxapp/eal/eal_memory.c    | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
index 412b432..77830f8 100644
--- a/lib/librte_eal/common/eal_common_memory.c
+++ b/lib/librte_eal/common/eal_common_memory.c
@@ -96,7 +96,7 @@ rte_dump_physmem_layout(FILE *f)
 
 		fprintf(f, "Segment %u: phys:0x%"PRIx64", len:%zu, "
 		       "virt:%p, socket_id:%"PRId32", "
-		       "hugepage_sz:%zu, nchannel:%"PRIx32", "
+		       "hugepage_sz:%"PRIu64", nchannel:%"PRIx32", "
 		       "nrank:%"PRIx32"\n", i,
 		       mcfg->memseg[i].phys_addr,
 		       mcfg->memseg[i].len,
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index aac6abf..e2ecb0d 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -49,7 +49,7 @@
  * mount points of hugepages
  */
 struct hugepage_info {
-	size_t hugepage_sz;   /**< size of a huge page */
+	uint64_t hugepage_sz;   /**< size of a huge page */
 	const char *hugedir;    /**< dir where hugetlbfs is mounted */
 	uint32_t num_pages[RTE_MAX_NUMA_NODES];
 				/**< number of hugepages of that size on each socket */
diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 1990833..7f8103f 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -92,7 +92,7 @@ struct rte_memseg {
 	phys_addr_t ioremap_addr; /**< Real physical address inside the VM */
 #endif
 	size_t len;               /**< Length of the segment. */
-	size_t hugepage_sz;       /**< The pagesize of underlying memory */
+	uint64_t hugepage_sz;       /**< The pagesize of underlying memory */
 	int32_t socket_id;          /**< NUMA socket ID. */
 	uint32_t nchannel;          /**< Number of channels. */
 	uint32_t nrank;             /**< Number of ranks. */
diff --git a/lib/librte_eal/common/include/rte_memzone.h b/lib/librte_eal/common/include/rte_memzone.h
index 7d47bff..3006e81 100644
--- a/lib/librte_eal/common/include/rte_memzone.h
+++ b/lib/librte_eal/common/include/rte_memzone.h
@@ -83,7 +83,7 @@ struct rte_memzone {
 #endif
 	size_t len;                       /**< Length of the memzone. */
 
-	size_t hugepage_sz;               /**< The page size of underlying memory */
+	uint64_t hugepage_sz;               /**< The page size of underlying memory */
 
 	int32_t socket_id;                /**< NUMA socket ID. */
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 700aba2..566a052 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -300,7 +300,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
 #endif
 
 	for (i = 0; i < hpi->num_pages[0]; i++) {
-		size_t hugepage_sz = hpi->hugepage_sz;
+		uint64_t hugepage_sz = hpi->hugepage_sz;
 
 		if (orig) {
 			hugepg_tbl[i].file_id = i;
-- 
1.9.3

[dpdk-dev] [PATCH 2/2] Fix compile issue of eal with icc compile

[Michael Qiu]

lib/librte_eal/linuxapp/eal/eal.c(461): error #2259: non-pointer
conversion from "long long" to "void *" may lose significant bits
   RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);

The root cause is that "RTE_PGSIZE_16M" is defined as unsigned long long.
But in i686 platform "void *" is 32-bit.
It is safe to cast to size_t and make it works in both 32 & 64-bit
platform.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 40b462e..37e4419 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -458,7 +458,7 @@ eal_parse_base_virtaddr(const char *arg)
 	 * it can align to 2MB for x86. So this alignment can also be used
 	 * on x86 */
 	internal_config.base_virtaddr =
-		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, RTE_PGSIZE_16M);
+		RTE_PTR_ALIGN_CEIL((uintptr_t)addr, (size_t)RTE_PGSIZE_16M);
 
 	return 0;
 }
-- 
1.9.3

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Thomas Monjalon]

> These two issues are both introuduced by commit b77b5639:
>         mem: add huge page sizes for IBM Power
> 
> Michael Qiu (2):
>   Fix compile issue with hugepage_sz in 32-bit system
>   Fix compile issue of eal with icc compile

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Neil Horman]

On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
> > These two issues are both introuduced by commit b77b5639:
> >         mem: add huge page sizes for IBM Power
> > 
> > Michael Qiu (2):
> >   Fix compile issue with hugepage_sz in 32-bit system
> >   Fix compile issue of eal with icc compile
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Applied
> 
> Thanks
> -- 
> Thomas
> 

Wait, why did you apply this patch?  We had outstanding debate on it, and
Michael indicated he was testing a new version of the patch.

Neil

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Qiu, Michael]

On 2014/12/11 21:26, Neil Horman wrote:
> On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
>>> These two issues are both introuduced by commit b77b5639:
>>>         mem: add huge page sizes for IBM Power
>>>
>>> Michael Qiu (2):
>>>   Fix compile issue with hugepage_sz in 32-bit system
>>>   Fix compile issue of eal with icc compile
>> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>>
>> Applied
>>
>> Thanks
>> -- 
>> Thomas
>>
> Wait, why did you apply this patch?  We had outstanding debate on it, and
> Michael indicated he was testing a new version of the patch.

Yes, I test the solution you suggest :) and it mostly works, but with a
little issue.
I have re-post not the old version.

Do you take a look at?

Thanks,
Michael
>
> Neil
>
>

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Thomas Monjalon]

2014-12-11 15:28, Qiu, Michael:
> On 2014/12/11 21:26, Neil Horman wrote:
> > On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
> >>> These two issues are both introuduced by commit b77b5639:
> >>>         mem: add huge page sizes for IBM Power
> >>>
> >>> Michael Qiu (2):
> >>>   Fix compile issue with hugepage_sz in 32-bit system
> >>>   Fix compile issue of eal with icc compile
> >> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> >>
> >> Applied
> >>
> >> Thanks
> >>
> > Wait, why did you apply this patch?  We had outstanding debate on it, and
> > Michael indicated he was testing a new version of the patch.
> 
> Yes, I test the solution you suggest :) and it mostly works, but with a
> little issue.
> I have re-post not the old version.

Neil, v4 is a new version implementing what you suggested.
There was no comment and it looks good so I applied it.
 
> Do you take a look at?

I think Neil missed the v4. Sorry to not have pinged you, I wanted rc4 for
validation at this time.
Neil do you agree this version is OK or do you see some issue to fix?

-- 
Thomas

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Neil Horman]

On Thu, Dec 11, 2014 at 10:21:44PM +0100, Thomas Monjalon wrote:
> 2014-12-11 15:28, Qiu, Michael:
> > On 2014/12/11 21:26, Neil Horman wrote:
> > > On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
> > >>> These two issues are both introuduced by commit b77b5639:
> > >>>         mem: add huge page sizes for IBM Power
> > >>>
> > >>> Michael Qiu (2):
> > >>>   Fix compile issue with hugepage_sz in 32-bit system
> > >>>   Fix compile issue of eal with icc compile
> > >> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > >>
> > >> Applied
> > >>
> > >> Thanks
> > >>
> > > Wait, why did you apply this patch?  We had outstanding debate on it, and
> > > Michael indicated he was testing a new version of the patch.
> > 
> > Yes, I test the solution you suggest :) and it mostly works, but with a
> > little issue.
> > I have re-post not the old version.
> 
> Neil, v4 is a new version implementing what you suggested.
> There was no comment and it looks good so I applied it.
>  
> > Do you take a look at?
> 
I didn't.  Apologies, I see the v4 now.  That said, something is off.  If you
look at the list archives, I see patch 0/2 v4 in the list, but not 1/2 or 2/2,
theres no actual patch that got posted.  Was it sent to you privately?

> I think Neil missed the v4. Sorry to not have pinged you, I wanted rc4 for
> validation at this time.
> Neil do you agree this version is OK or do you see some issue to fix?
> 
Again, I think Michales send went sideways.  0/4 went to the list but the actual
patches only went to you Thomas.  Please post them to the list

Neil

> -- 
> Thomas
> 

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Thomas Monjalon]

2014-12-12 06:38, Neil Horman:
> On Thu, Dec 11, 2014 at 10:21:44PM +0100, Thomas Monjalon wrote:
> > 2014-12-11 15:28, Qiu, Michael:
> > > On 2014/12/11 21:26, Neil Horman wrote:
> > > > On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
> > > >>> These two issues are both introuduced by commit b77b5639:
> > > >>>         mem: add huge page sizes for IBM Power
> > > >>>
> > > >>> Michael Qiu (2):
> > > >>>   Fix compile issue with hugepage_sz in 32-bit system
> > > >>>   Fix compile issue of eal with icc compile
> > > >> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > >>
> > > >> Applied
> > > >>
> > > >> Thanks
> > > >>
> > > > Wait, why did you apply this patch?  We had outstanding debate on it, and
> > > > Michael indicated he was testing a new version of the patch.
> > > 
> > > Yes, I test the solution you suggest :) and it mostly works, but with a
> > > little issue.
> > > I have re-post not the old version.
> > 
> > Neil, v4 is a new version implementing what you suggested.
> > There was no comment and it looks good so I applied it.
> >  
> > > Do you take a look at?
> > 
> I didn't.  Apologies, I see the v4 now.  That said, something is off.  If you
> look at the list archives, I see patch 0/2 v4 in the list, but not 1/2 or 2/2,
> theres no actual patch that got posted.  Was it sent to you privately?

No there are public and you are Cc.

> > I think Neil missed the v4. Sorry to not have pinged you, I wanted rc4 for
> > validation at this time.
> > Neil do you agree this version is OK or do you see some issue to fix?
> > 
> Again, I think Michales send went sideways.  0/4 went to the list but the actual
> patches only went to you Thomas.  Please post them to the list

They were correctly posted:
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/9282/focus=9754

-- 
Thomas

Re: [dpdk-dev] [PATCH 0/2 v4] Fix two compile issues with i686 platform

[Neil Horman]

On Fri, Dec 12, 2014 at 04:09:46PM +0100, Thomas Monjalon wrote:
> 2014-12-12 06:38, Neil Horman:
> > On Thu, Dec 11, 2014 at 10:21:44PM +0100, Thomas Monjalon wrote:
> > > 2014-12-11 15:28, Qiu, Michael:
> > > > On 2014/12/11 21:26, Neil Horman wrote:
> > > > > On Thu, Dec 11, 2014 at 01:56:06AM +0100, Thomas Monjalon wrote:
> > > > >>> These two issues are both introuduced by commit b77b5639:
> > > > >>>         mem: add huge page sizes for IBM Power
> > > > >>>
> > > > >>> Michael Qiu (2):
> > > > >>>   Fix compile issue with hugepage_sz in 32-bit system
> > > > >>>   Fix compile issue of eal with icc compile
> > > > >> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > > >>
> > > > >> Applied
> > > > >>
> > > > >> Thanks
> > > > >>
> > > > > Wait, why did you apply this patch?  We had outstanding debate on it, and
> > > > > Michael indicated he was testing a new version of the patch.
> > > > 
> > > > Yes, I test the solution you suggest :) and it mostly works, but with a
> > > > little issue.
> > > > I have re-post not the old version.
> > > 
> > > Neil, v4 is a new version implementing what you suggested.
> > > There was no comment and it looks good so I applied it.
> > >  
> > > > Do you take a look at?
> > > 
> > I didn't.  Apologies, I see the v4 now.  That said, something is off.  If you
> > look at the list archives, I see patch 0/2 v4 in the list, but not 1/2 or 2/2,
> > theres no actual patch that got posted.  Was it sent to you privately?
> 
> No there are public and you are Cc.
> 
> > > I think Neil missed the v4. Sorry to not have pinged you, I wanted rc4 for
> > > validation at this time.
> > > Neil do you agree this version is OK or do you see some issue to fix?
> > > 
> > Again, I think Michales send went sideways.  0/4 went to the list but the actual
> > patches only went to you Thomas.  Please post them to the list
> 
> They were correctly posted:
> http://thread.gmane.org/gmane.comp.networking.dpdk.devel/9282/focus=9754
> 
Hmm, I apologize.  somehow these haven't show up in my reader.  I must have a
bogus filter somewhere.

Looking at the patch, it looks good.  Thank you, and sorry for the noise.

For the record.
Acked-by: Neil Horman <nhorman@tuxdriver.com>

> -- 
> Thomas
>