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

Message ID 1417684369-21330-1-git-send-email-michael.qiu@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Michael Qiu Dec. 4, 2014, 9:12 a.m. UTC
  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(-)
  

Comments

Michael Qiu Dec. 5, 2014, 6:56 a.m. UTC | #1
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++;
  
Chao Zhu Dec. 5, 2014, 7:04 a.m. UTC | #2
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++;
>
  
Chao Zhu Dec. 5, 2014, 8:31 a.m. UTC | #3
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>
  
Neil Horman Dec. 5, 2014, 2:22 p.m. UTC | #4
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
  
Bruce Richardson Dec. 5, 2014, 3:02 p.m. UTC | #5
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
  
Neil Horman Dec. 5, 2014, 3:24 p.m. UTC | #6
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
  
Michael Qiu Dec. 8, 2014, 2:46 a.m. UTC | #7
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
>
>
  
Michael Qiu Dec. 10, 2014, 10:46 a.m. UTC | #8
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(-)
  
Thomas Monjalon Dec. 11, 2014, 12:56 a.m. UTC | #9
> 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
  
Neil Horman Dec. 11, 2014, 1:25 p.m. UTC | #10
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
  
Michael Qiu Dec. 11, 2014, 3:28 p.m. UTC | #11
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
>
>
  
Thomas Monjalon Dec. 11, 2014, 9:21 p.m. UTC | #12
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?
  
Neil Horman Dec. 12, 2014, 11:38 a.m. UTC | #13
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
>
  
Thomas Monjalon Dec. 12, 2014, 3:09 p.m. UTC | #14
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
  
Neil Horman Dec. 12, 2014, 3:29 p.m. UTC | #15
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
>
  

Patch

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++;