Re: seperator error in __mask_snprintf_len

[Paul Jackson]

Andrew asked:
> Just looking at this function, it seems to walking an array of longs using
> a u32*.  Could someone please convince me that this is correct on both
> little-endian and big-endian 64-bit hardware?  

Good question, Andrew.  Thanks.  I suspect I did indeed break something
here.

The key thing to recall first is that these masks, and the bitops of
longer standing on which they rely, are very little endian.  On all
architectures, masks are essentially as if an array of bytes, even
though they have a size that is a multiple of sizeof(long).

To quote from include/asm-ppc64/bitops.h:

 * Bitops are odd when viewed on big-endian systems.  They were designed
 * on little endian so the size of the bitset doesn't matter (low order bytes
 * come first) as long as the bit in question is valid.

>From one u32 word to the next, both my input and output routines in
lib/mask.c (mask_snprintf and mask_parse) respect this order, so that
part is correct, I believe.  For a mask beginning at address A, bytes
A[0..3] form the first u32 word, A[4..7] the second, and so forth. 
Good.

Whether the mask size is a multiple of u32 or u64 doesn't matter.

 ==> However, _within_ each word, I suspect that I have the bytes arse
backwards on a big endian machine.  The underlying snprintf("%x") and
strtoul() routines that I call will presume that the byte order of the
referenced u32 binary word is native (big on big endian).  Not good.

Anyone with a big-endian SMP machine should be able to see this, by
displaying /proc/irq/prof_cpu_mask or /proc/irq/<pid>/smp_affinity, and
determining if they see the expected low order bit(s) set, or instead
the output is byte reversed.

Given the good chance that I'm still confused, I will now broadcast
under a more enticing Subject a request for someone with a big endian
SMP to verify whether I've broken this as I suspect.

If this request proceeds as expected, I will follow up with a patch to
lib/mask.c that will likely make use of the cpu_to_le32() and
le32_to_cpu() macros in byteorder.h to swap bytes in the u32 words being
displayed and parsed.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@xxxxxxx> 1.650.933.1373
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/