seperator error in __mask_snprintf_len

DISCUSSION Andi Kleen 3 participants 3 messages Jan 16, 2004 13 messages not in the archive

Share & tools

A message that is not in the archive (sent off-list or not yet received): [email protected]

Andrew Morton

2004-01-16 08:35 UTC #14 lore raw

Quoted 22 lines · Andi Kleen

Review
Quoted 8 lines · Andrew Morton
(quoted diff) +8 -0
+void bitmap_complement(unsigned long *bitmap, int bits)
+{
+	int k;
+
+	for (k = 0; k < BITS_TO_LONGS(bits); ++k)
+		bitmap[k] = ~bitmap[k];
+}
+EXPORT_SYMBOL(bitmap_complement);
I actually had to change that one in my NUMA API patchkit (which uses bitmap functions for its node maps), because gcc 3.2 miscompiled the loop.
Please add something like that (looks silly, but makes a big difference):
static inline void bitmap_complement(unsigned long *bitmap, int bits)
 {
        int k;

Review

Quoted 5 lines

(quoted diff) +2 -1

+       int max = BITS_TO_LONGS(bits);
 
-       for (k = 0; k < BITS_TO_LONGS(bits); ++k)
+       for (k = 0; k < max; ++k)
                bitmap[k] = ~bitmap[k];

OK. bitmap_and() and bitmap_or() were converted to this form a while back because they too were hitting the bug. On ia32. It might no longer happen now they're uninlined but whatever - you don't have to look at the code and think "gee, I hope the compiler moves that arith out of the loop".

Andi Kleen

2004-01-16 10:15 UTC #15 lore raw

Quoted 9 lines

Review
Quoted 8 lines
static inline void bitmap_complement(unsigned long *bitmap, int bits)
 {
        int k;
(quoted diff) +2 -1
+       int max = BITS_TO_LONGS(bits);
 
-       for (k = 0; k < BITS_TO_LONGS(bits); ++k)
+       for (k = 0; k < max; ++k)
                bitmap[k] = ~bitmap[k];

Quoted 5 lines

OK. bitmap_and() and bitmap_or() were converted to this form a while back because they too were hitting the bug. On ia32. It might no longer happen now they're uninlined but whatever - you don't have to look at the code and think "gee, I hope the compiler moves that arith out of the loop".

BTW I think the original point of the inlining was that when you have bits == BITS_PER_LONG the function could collapse to a single ~ operation using the compiler's optimizer. That would be the case with nodemasks on x86-64. As for the NUMA API i don't care particularly because there is no bitmap manipulation in any fast path. Avoiding miscompilations is definitely higher priority.

-Andi