diff mbox

numactl: fix libnuma on big-endian 64-bit systems

Message ID 200812041834.45931.arnd@arndb.de (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Arnd Bergmann Dec. 4, 2008, 5:34 p.m. UTC
The read-mask function assumes that it is running in 32-bit mode,
by addressing the bitmask as a series of int values, instead of
longs. This is broken as can easily be reproduced by running numademo
on a bit-endian 64-bit system.

Changing the addressing to use 'long' values fixes the problem.

Reported-by: Mijo Safradin <safradin@de.ibm.com>

---

Note: the set_nodemask_size() function is broken as well, it seems
to always set the nodemask size to "17" with the s2nbits implementation.
The fallback path in there looks correct.

Comments

Lee Schermerhorn Dec. 4, 2008, 5:46 p.m. UTC | #1
On Thu, 2008-12-04 at 18:34 +0100, Arnd Bergmann wrote:
> The read-mask function assumes that it is running in 32-bit mode,
> by addressing the bitmask as a series of int values, instead of
> longs. This is broken as can easily be reproduced by running numademo
> on a bit-endian 64-bit system.
> 
> Changing the addressing to use 'long' values fixes the problem.

Hi, Arnd:

Not sure what you mean here.  If the patch below is a proposed fix [I
don't see a 'Signed-off-by:", but maybe not needed for libnuma
patches?], the description above doesn't match the code.  Looks like
you're changing the addressing FROM 'long' values to use 'int' values so
that the size is compatible between 32- and 64-bits.  Or is that a
reverse patch/diff below?

Lee


> Reported-by: Mijo Safradin <safradin@de.ibm.com>
> 
> ---
> 
> Note: the set_nodemask_size() function is broken as well, it seems
> to always set the nodemask size to "17" with the s2nbits implementation.
> The fallback path in there looks correct.
> 
> --- a/libnuma.c	2008-12-04 14:25:30.000000000 +0100
> +++ b/libnuma.c	2008-11-20 13:40:29.000000000 +0100
> @@ -392,9 +372,9 @@ read_mask(char *s, struct bitmask *bmp)
>  {
>  	char *end = s;
>  	char *prevend;
> -	unsigned long *start = bmp->maskp;
> -	unsigned long *p = start;
> -	unsigned long *q;
> +	unsigned int *start = (unsigned int *)bmp->maskp;
> +	unsigned int *p = start;
> +	unsigned int *q;
>  	unsigned int i;
>  	unsigned int n = 0;
>  
> @@ -431,14 +411,14 @@
>  	}
>  
>  	/* Poor mans fls() */
> -	for(i = sizeof(long) * 8 - 1; i >= 0; i--)
> +	for(i = 31; i >= 0; i--)
>  		if (test_bit(i, start + n))
>  			break;
>  
>  	/*
>  	 * Return the last bit set
>  	 */
> -	return ((sizeof(unsigned long)*8) * n) + i;
> +	return ((sizeof(unsigned int)*8) * n) + i;
>  }
>  
>  /*
Arnd Bergmann Dec. 4, 2008, 8:49 p.m. UTC | #2
On Thursday 04 December 2008, Lee Schermerhorn wrote:
> On Thu, 2008-12-04 at 18:34 +0100, Arnd Bergmann wrote:
> > The read-mask function assumes that it is running in 32-bit mode,
> > by addressing the bitmask as a series of int values, instead of
> > longs. This is broken as can easily be reproduced by running numademo
> > on a bit-endian 64-bit system.
> > 
> > Changing the addressing to use 'long' values fixes the problem.
> 
> Hi, Arnd:
> 
> Not sure what you mean here.  If the patch below is a proposed fix [I
> don't see a 'Signed-off-by:", but maybe not needed for libnuma
> patches?], the description above doesn't match the code.  Looks like
> you're changing the addressing FROM 'long' values to use 'int' values so
> that the size is compatible between 32- and 64-bits.  Or is that a
> reverse patch/diff below?

Sorry about that, I was in a hurry when sending this one out and attached
the wrong file, the reverse patch is needed indeed. I'll resend with
a proper Signed-off-by.

	Arnd <><
diff mbox

Patch

--- a/libnuma.c	2008-12-04 14:25:30.000000000 +0100
+++ b/libnuma.c	2008-11-20 13:40:29.000000000 +0100
@@ -392,9 +372,9 @@  read_mask(char *s, struct bitmask *bmp)
 {
 	char *end = s;
 	char *prevend;
-	unsigned long *start = bmp->maskp;
-	unsigned long *p = start;
-	unsigned long *q;
+	unsigned int *start = (unsigned int *)bmp->maskp;
+	unsigned int *p = start;
+	unsigned int *q;
 	unsigned int i;
 	unsigned int n = 0;
 
@@ -431,14 +411,14 @@ 
 	}
 
 	/* Poor mans fls() */
-	for(i = sizeof(long) * 8 - 1; i >= 0; i--)
+	for(i = 31; i >= 0; i--)
 		if (test_bit(i, start + n))
 			break;
 
 	/*
 	 * Return the last bit set
 	 */
-	return ((sizeof(unsigned long)*8) * n) + i;
+	return ((sizeof(unsigned int)*8) * n) + i;
 }
 
 /*