[Re: PATCH] numactl: fix libnuma on big-endian 64-bit systems

[Re: PATCH] numactl: fix libnuma on big-endian 64-bit systems

[Cliff Wickman]

linux-numa@vger.kernel.org

Arnd Bergmann and Mijo Safradin called my attention to problems with
libnuma's read_mask() on a big-endian 64-bit machine.

In testing that function I found it to be buggy on little-endian and 32-bit
systems as well.

I propose a function that looks like the below.  The patch is farther below.
(Arnd, I did not switch to longs, as that proved problematic.  The ascii hex
 strings being scanned are 32-bit values.)

I tested on a 64-bit MIPS system.  Could you test on your platform?

/*
 * Read a mask consisting of a sequence of hexadecimal longs separated by
 * commas. Order them correctly and return the number of the last bit
 * set.
 */
static int
read_mask(char *s, struct bitmask *bmp)
{
	char *end = s;
	unsigned int *start = (unsigned int *)bmp->maskp;
	unsigned int *p = start;
	unsigned int *q;
	unsigned int i;
	unsigned int n = 0;

	i = strtoul(s, &end, 16);

	/* Skip leading zeros */
	while (!i && *end++ == ',') {
		i = strtoul(end, &end, 16);
	}
	end++; /* past the , */

	if (!i)
		/* End of string. No mask */
		return -1;

	start[n++] = i;
	/* Read sequence of ints */
	do {
		i = strtoul(end, &end, 16);
		if (i)
			start[n++] = i;
	} while (*end++ == ',');
	n--;

	/*
	 * Invert sequence of ints if necessary since the first int
	 * is the highest and we put it first because we read it first.
	 */
	for (q = start + n, p = start; p < q; q--, p++) {
		unsigned int x = *q;

		*q = *p;
		*p = x;
	}

	/* Poor mans fls() */
	for(i = 31; i >= 0; i--)
		if (test_bit(i, start + n))
			break;

	/*
	 * Return the last bit set
	 */
	return ((sizeof(unsigned int)*8) * n) + i;
}



Signed-off-by: Cliff Wickman <cpw@sgi.com>
---
 libnuma.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Index: numactl-dev/libnuma.c
===================================================================
--- numactl-dev.orig/libnuma.c
+++ numactl-dev/libnuma.c
@@ -371,7 +371,6 @@ static int
 read_mask(char *s, struct bitmask *bmp)
 {
 	char *end = s;
-	char *prevend;
 	unsigned int *start = (unsigned int *)bmp->maskp;
 	unsigned int *p = start;
 	unsigned int *q;
@@ -380,22 +379,22 @@ read_mask(char *s, struct bitmask *bmp)
 
 	i = strtoul(s, &end, 16);
 
-	prevend = end;
 	/* Skip leading zeros */
 	while (!i && *end++ == ',') {
-		prevend = end;
 		i = strtoul(end, &end, 16);
 	}
-	end = prevend;
+	end++; /* past the , */
 
 	if (!i)
 		/* End of string. No mask */
 		return -1;
 
+	start[n++] = i;
 	/* Read sequence of ints */
 	do {
-		start[n++] = i;
 		i = strtoul(end, &end, 16);
+		if (i)
+			start[n++] = i;
 	} while (*end++ == ',');
 	n--;
 

Re: [Re: PATCH] numactl: fix libnuma on big-endian 64-bit systems

[Mijo Safradin]

Cliff Wickman wrote:
> linux-numa@vger.kernel.org
> 
> Arnd Bergmann and Mijo Safradin called my attention to problems with
> libnuma's read_mask() on a big-endian 64-bit machine.
> 
> In testing that function I found it to be buggy on little-endian and 32-bit
> systems as well.
> 
> I propose a function that looks like the below.  The patch is farther below.
> (Arnd, I did not switch to longs, as that proved problematic.  The ascii hex
>  strings being scanned are 32-bit values.)
> 
> I tested on a 64-bit MIPS system.  Could you test on your platform?
> 

Hi Cliff,

using the current numactl-2.0.3-rc1.tar.gz, which contains your patch.
numademo breaks with "mbind: Invalid argument". 

Mijo