mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] powerpc/mm: fix always true/false warning in slice.c
@ 2018-06-22 13:49 Christophe Leroy
  2018-06-22 14:55 ` Mathieu Malaterre
  2018-06-29  2:55 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Christophe Leroy @ 2018-06-22 13:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, malat,
	aneesh.kumar
  Cc: linux-kernel, linuxppc-dev

This patch fixes the following warnings (obtained with make W=1).

arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (start < SLICE_LOW_TOP) {
            ^
arch/powerpc/mm/slice.c:81:20: error: comparison is always false due to limited range of data type [-Werror=type-limits]
  if ((start + len) > SLICE_LOW_TOP) {
                    ^
arch/powerpc/mm/slice.c: In function 'slice_mask_for_free':
arch/powerpc/mm/slice.c:136:17: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (high_limit <= SLICE_LOW_TOP)
                 ^
arch/powerpc/mm/slice.c: In function 'slice_check_range_fits':
arch/powerpc/mm/slice.c:185:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (start < SLICE_LOW_TOP) {
            ^
arch/powerpc/mm/slice.c:195:39: error: comparison is always false due to limited range of data type [-Werror=type-limits]
  if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
                                       ^
arch/powerpc/mm/slice.c: In function 'slice_scan_available':
arch/powerpc/mm/slice.c:306:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (addr < SLICE_LOW_TOP) {
           ^
arch/powerpc/mm/slice.c: In function 'get_slice_psize':
arch/powerpc/mm/slice.c:709:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (addr < SLICE_LOW_TOP) {
           ^

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/slice.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 9530c6db406a..17c57760e06c 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -62,6 +62,13 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
 
 #endif
 
+static inline bool slice_addr_is_low(unsigned long addr)
+{
+	u64 tmp = (u64)addr;
+
+	return tmp < SLICE_LOW_TOP;
+}
+
 static void slice_range_to_mask(unsigned long start, unsigned long len,
 				struct slice_mask *ret)
 {
@@ -71,7 +78,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
 	if (SLICE_NUM_HIGH)
 		bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
 
-	if (start < SLICE_LOW_TOP) {
+	if (slice_addr_is_low(start)) {
 		unsigned long mend = min(end,
 					 (unsigned long)(SLICE_LOW_TOP - 1));
 
@@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
 			- (1u << GET_LOW_SLICE_INDEX(start));
 	}
 
-	if ((start + len) > SLICE_LOW_TOP) {
+	if (!slice_addr_is_low(end)) {
 		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
 		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
 		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
@@ -134,7 +141,7 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
 		if (!slice_low_has_vma(mm, i))
 			ret->low_slices |= 1u << i;
 
-	if (high_limit <= SLICE_LOW_TOP)
+	if (slice_addr_is_low(high_limit - 1))
 		return;
 
 	for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
@@ -183,7 +190,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
 	unsigned long end = start + len - 1;
 	u64 low_slices = 0;
 
-	if (start < SLICE_LOW_TOP) {
+	if (slice_addr_is_low(start)) {
 		unsigned long mend = min(end,
 					 (unsigned long)(SLICE_LOW_TOP - 1));
 
@@ -193,7 +200,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
 	if ((low_slices & available->low_slices) != low_slices)
 		return false;
 
-	if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
+	if (SLICE_NUM_HIGH && !slice_addr_is_low(end)) {
 		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
 		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
 		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
@@ -304,7 +311,7 @@ static bool slice_scan_available(unsigned long addr,
 				 int end, unsigned long *boundary_addr)
 {
 	unsigned long slice;
-	if (addr < SLICE_LOW_TOP) {
+	if (slice_addr_is_low(addr)) {
 		slice = GET_LOW_SLICE_INDEX(addr);
 		*boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
 		return !!(available->low_slices & (1u << slice));
@@ -707,7 +714,7 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
 
 	VM_BUG_ON(radix_enabled());
 
-	if (addr < SLICE_LOW_TOP) {
+	if (slice_addr_is_low(addr)) {
 		psizes = mm->context.low_slices_psize;
 		index = GET_LOW_SLICE_INDEX(addr);
 	} else {
-- 
2.13.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/mm: fix always true/false warning in slice.c
  2018-06-22 13:49 [PATCH] powerpc/mm: fix always true/false warning in slice.c Christophe Leroy
@ 2018-06-22 14:55 ` Mathieu Malaterre
  2018-06-22 16:17   ` Christophe LEROY
  2018-06-29  2:55 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Mathieu Malaterre @ 2018-06-22 14:55 UTC (permalink / raw)
  To: Christophe LEROY
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Aneesh Kumar K.V, LKML, linuxppc-dev

On Fri, Jun 22, 2018 at 3:49 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
> This patch fixes the following warnings (obtained with make W=1).
>
> arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
> arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (start < SLICE_LOW_TOP) {
>             ^
> arch/powerpc/mm/slice.c:81:20: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>   if ((start + len) > SLICE_LOW_TOP) {
>                     ^
> arch/powerpc/mm/slice.c: In function 'slice_mask_for_free':
> arch/powerpc/mm/slice.c:136:17: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (high_limit <= SLICE_LOW_TOP)
>                  ^
> arch/powerpc/mm/slice.c: In function 'slice_check_range_fits':
> arch/powerpc/mm/slice.c:185:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (start < SLICE_LOW_TOP) {
>             ^
> arch/powerpc/mm/slice.c:195:39: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>   if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
>                                        ^
> arch/powerpc/mm/slice.c: In function 'slice_scan_available':
> arch/powerpc/mm/slice.c:306:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (addr < SLICE_LOW_TOP) {
>            ^
> arch/powerpc/mm/slice.c: In function 'get_slice_psize':
> arch/powerpc/mm/slice.c:709:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (addr < SLICE_LOW_TOP) {
>            ^
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/mm/slice.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 9530c6db406a..17c57760e06c 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -62,6 +62,13 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
>
>  #endif
>
> +static inline bool slice_addr_is_low(unsigned long addr)
> +{
> +       u64 tmp = (u64)addr;
> +
> +       return tmp < SLICE_LOW_TOP;
> +}
> +
>  static void slice_range_to_mask(unsigned long start, unsigned long len,
>                                 struct slice_mask *ret)
>  {
> @@ -71,7 +78,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>         if (SLICE_NUM_HIGH)
>                 bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
>
> -       if (start < SLICE_LOW_TOP) {
> +       if (slice_addr_is_low(start)) {
>                 unsigned long mend = min(end,
>                                          (unsigned long)(SLICE_LOW_TOP - 1));
>
> @@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>                         - (1u << GET_LOW_SLICE_INDEX(start));
>         }
>
> -       if ((start + len) > SLICE_LOW_TOP) {
> +       if (!slice_addr_is_low(end)) {
>                 unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>                 unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>                 unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> @@ -134,7 +141,7 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
>                 if (!slice_low_has_vma(mm, i))
>                         ret->low_slices |= 1u << i;
>
> -       if (high_limit <= SLICE_LOW_TOP)
> +       if (slice_addr_is_low(high_limit - 1))

Is high_limit ever going to be 0 ?

>                 return;
>
>         for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
> @@ -183,7 +190,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
>         unsigned long end = start + len - 1;
>         u64 low_slices = 0;
>
> -       if (start < SLICE_LOW_TOP) {
> +       if (slice_addr_is_low(start)) {
>                 unsigned long mend = min(end,
>                                          (unsigned long)(SLICE_LOW_TOP - 1));
>
> @@ -193,7 +200,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
>         if ((low_slices & available->low_slices) != low_slices)
>                 return false;
>
> -       if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
> +       if (SLICE_NUM_HIGH && !slice_addr_is_low(end)) {
>                 unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>                 unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>                 unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> @@ -304,7 +311,7 @@ static bool slice_scan_available(unsigned long addr,
>                                  int end, unsigned long *boundary_addr)
>  {
>         unsigned long slice;
> -       if (addr < SLICE_LOW_TOP) {
> +       if (slice_addr_is_low(addr)) {
>                 slice = GET_LOW_SLICE_INDEX(addr);
>                 *boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
>                 return !!(available->low_slices & (1u << slice));
> @@ -707,7 +714,7 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
>
>         VM_BUG_ON(radix_enabled());
>
> -       if (addr < SLICE_LOW_TOP) {
> +       if (slice_addr_is_low(addr)) {
>                 psizes = mm->context.low_slices_psize;
>                 index = GET_LOW_SLICE_INDEX(addr);
>         } else {
> --
> 2.13.3
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/mm: fix always true/false warning in slice.c
  2018-06-22 14:55 ` Mathieu Malaterre
@ 2018-06-22 16:17   ` Christophe LEROY
  0 siblings, 0 replies; 5+ messages in thread
From: Christophe LEROY @ 2018-06-22 16:17 UTC (permalink / raw)
  To: Mathieu Malaterre
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Aneesh Kumar K.V, LKML, linuxppc-dev



Le 22/06/2018 à 16:55, Mathieu Malaterre a écrit :
> On Fri, Jun 22, 2018 at 3:49 PM Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
>>
>> This patch fixes the following warnings (obtained with make W=1).
>>
>> arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
>> arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (start < SLICE_LOW_TOP) {
>>              ^
>> arch/powerpc/mm/slice.c:81:20: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>>    if ((start + len) > SLICE_LOW_TOP) {
>>                      ^
>> arch/powerpc/mm/slice.c: In function 'slice_mask_for_free':
>> arch/powerpc/mm/slice.c:136:17: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (high_limit <= SLICE_LOW_TOP)
>>                   ^
>> arch/powerpc/mm/slice.c: In function 'slice_check_range_fits':
>> arch/powerpc/mm/slice.c:185:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (start < SLICE_LOW_TOP) {
>>              ^
>> arch/powerpc/mm/slice.c:195:39: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>>    if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
>>                                         ^
>> arch/powerpc/mm/slice.c: In function 'slice_scan_available':
>> arch/powerpc/mm/slice.c:306:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (addr < SLICE_LOW_TOP) {
>>             ^
>> arch/powerpc/mm/slice.c: In function 'get_slice_psize':
>> arch/powerpc/mm/slice.c:709:11: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (addr < SLICE_LOW_TOP) {
>>             ^
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   arch/powerpc/mm/slice.c | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
>> index 9530c6db406a..17c57760e06c 100644
>> --- a/arch/powerpc/mm/slice.c
>> +++ b/arch/powerpc/mm/slice.c
>> @@ -62,6 +62,13 @@ static void slice_print_mask(const char *label, const struct slice_mask *mask) {
>>
>>   #endif
>>
>> +static inline bool slice_addr_is_low(unsigned long addr)
>> +{
>> +       u64 tmp = (u64)addr;
>> +
>> +       return tmp < SLICE_LOW_TOP;
>> +}
>> +
>>   static void slice_range_to_mask(unsigned long start, unsigned long len,
>>                                  struct slice_mask *ret)
>>   {
>> @@ -71,7 +78,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>>          if (SLICE_NUM_HIGH)
>>                  bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
>>
>> -       if (start < SLICE_LOW_TOP) {
>> +       if (slice_addr_is_low(start)) {
>>                  unsigned long mend = min(end,
>>                                           (unsigned long)(SLICE_LOW_TOP - 1));
>>
>> @@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>>                          - (1u << GET_LOW_SLICE_INDEX(start));
>>          }
>>
>> -       if ((start + len) > SLICE_LOW_TOP) {
>> +       if (!slice_addr_is_low(end)) {
>>                  unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>>                  unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>>                  unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
>> @@ -134,7 +141,7 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
>>                  if (!slice_low_has_vma(mm, i))
>>                          ret->low_slices |= 1u << i;
>>
>> -       if (high_limit <= SLICE_LOW_TOP)
>> +       if (slice_addr_is_low(high_limit - 1))
> 
> Is high_limit ever going to be 0 ?

See slice_get_unmapped_area(), high_limit will never be 0.

Christophe

> 
>>                  return;
>>
>>          for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
>> @@ -183,7 +190,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
>>          unsigned long end = start + len - 1;
>>          u64 low_slices = 0;
>>
>> -       if (start < SLICE_LOW_TOP) {
>> +       if (slice_addr_is_low(start)) {
>>                  unsigned long mend = min(end,
>>                                           (unsigned long)(SLICE_LOW_TOP - 1));
>>
>> @@ -193,7 +200,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
>>          if ((low_slices & available->low_slices) != low_slices)
>>                  return false;
>>
>> -       if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
>> +       if (SLICE_NUM_HIGH && !slice_addr_is_low(end)) {
>>                  unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>>                  unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>>                  unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
>> @@ -304,7 +311,7 @@ static bool slice_scan_available(unsigned long addr,
>>                                   int end, unsigned long *boundary_addr)
>>   {
>>          unsigned long slice;
>> -       if (addr < SLICE_LOW_TOP) {
>> +       if (slice_addr_is_low(addr)) {
>>                  slice = GET_LOW_SLICE_INDEX(addr);
>>                  *boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
>>                  return !!(available->low_slices & (1u << slice));
>> @@ -707,7 +714,7 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
>>
>>          VM_BUG_ON(radix_enabled());
>>
>> -       if (addr < SLICE_LOW_TOP) {
>> +       if (slice_addr_is_low(addr)) {
>>                  psizes = mm->context.low_slices_psize;
>>                  index = GET_LOW_SLICE_INDEX(addr);
>>          } else {
>> --
>> 2.13.3
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/mm: fix always true/false warning in slice.c
  2018-06-22 13:49 [PATCH] powerpc/mm: fix always true/false warning in slice.c Christophe Leroy
  2018-06-22 14:55 ` Mathieu Malaterre
@ 2018-06-29  2:55 ` Michael Ellerman
  2018-08-02  9:30   ` Christophe Leroy
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2018-06-29  2:55 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, malat,
	aneesh.kumar
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> This patch fixes the following warnings (obtained with make W=1).
>
> arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
> arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (start < SLICE_LOW_TOP) {

Presumably only on 32-bit ?

> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 9530c6db406a..17c57760e06c 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>  			- (1u << GET_LOW_SLICE_INDEX(start));
>  	}
>  
> -	if ((start + len) > SLICE_LOW_TOP) {
> +	if (!slice_addr_is_low(end)) {
>  		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>  		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>  		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;

This worries me.

By casting before the comparison in the helper you squash the compiler
warning, but the code is still broken if (start + len) overflows.

Presumably that "never happens", but it just seems fishy.

The other similar check in that file does:

  if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {

Where SLICE_NUM_HIGH == 0 on 32-bit.


Could we fix the less than comparisons with SLICE_LOW_TOP with something
similar, eg:

	if (!SLICE_NUM_HIGH || start < SLICE_LOW_TOP) {

ie. limit them to the 64-bit code?

cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/mm: fix always true/false warning in slice.c
  2018-06-29  2:55 ` Michael Ellerman
@ 2018-08-02  9:30   ` Christophe Leroy
  0 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2018-08-02  9:30 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, malat,
	aneesh.kumar
  Cc: linux-kernel, linuxppc-dev



On 06/29/2018 02:55 AM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> This patch fixes the following warnings (obtained with make W=1).
>>
>> arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
>> arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>    if (start < SLICE_LOW_TOP) {
> 
> Presumably only on 32-bit ?

Sure

> 
>> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
>> index 9530c6db406a..17c57760e06c 100644
>> --- a/arch/powerpc/mm/slice.c
>> +++ b/arch/powerpc/mm/slice.c
>> @@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, unsigned long len,
>>   			- (1u << GET_LOW_SLICE_INDEX(start));
>>   	}
>>   
>> -	if ((start + len) > SLICE_LOW_TOP) {
>> +	if (!slice_addr_is_low(end)) {
>>   		unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>>   		unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>>   		unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
> 
> This worries me.

I'll change it in v2 to:
  	if (SLICE_NUM_HIGH && !slice_addr_is_low(end)) {


> 
> By casting before the comparison in the helper you squash the compiler
> warning, but the code is still broken if (start + len) overflows.
> 
> Presumably that "never happens", but it just seems fishy.
> 
> The other similar check in that file does:
> 
>    if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
> 
> Where SLICE_NUM_HIGH == 0 on 32-bit.
> 
> 
> Could we fix the less than comparisons with SLICE_LOW_TOP with something
> similar, eg:
> 
> 	if (!SLICE_NUM_HIGH || start < SLICE_LOW_TOP) {
> 
> ie. limit them to the 64-bit code?

That's not enough to make GCC happy:

arch/powerpc/mm/slice.c: In function ‘slice_range_to_mask’:
arch/powerpc/mm/slice.c:74:31: error: comparison is always true due to 
limited range of data type [-Werror=type-limits]
   if (!SLICE_NUM_HIGH || start < SLICE_LOW_TOP) {

Christophe


> 
> cheers
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-08-02  9:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-22 13:49 [PATCH] powerpc/mm: fix always true/false warning in slice.c Christophe Leroy
2018-06-22 14:55 ` Mathieu Malaterre
2018-06-22 16:17   ` Christophe LEROY
2018-06-29  2:55 ` Michael Ellerman
2018-08-02  9:30   ` Christophe Leroy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®