Re: [Patch 17/23] mask v2 = [6/7] nodemask_t_ia64_changes

Paul Jackson
2004-04-07T06:52:39Z

Denis asked:
> why such a simple thing require 700 bytes of code in the first place?

Well ... it doesn't "require" 700 bytes of code. But it is currently
consuming that much in this patch set, each time a for node loop is
invoked.

This is because "for_each_online_node" boils down to two copies of
"find_next_bit" (to get the first bit and then to get the next bit), and
in the file include/asm-ia64/bitops.h, find_next_bit() is the following
hefty chunk of inline code:

/*
* Find next bit in a bitmap reasonably efficiently..
*/
static inline int
find_next_bit(const void *addr, unsigned long size, unsigned long offset)
{
unsigned long *p = ((unsigned long *) addr) + (offset >> 6);
unsigned long result = offset & ~63UL;
unsigned long tmp;

if (offset >= size)
return size;
size -= result;
offset &= 63UL;
if (offset) {
tmp = *(p++);
tmp &= ~0UL << offset;
if (size < 64)
goto found_first;
if (tmp)
goto found_middle;
size -= 64;
result += 64;
}
while (size & ~63UL) {
if ((tmp = *(p++)))
goto found_middle;
result += 64;
size -= 64;
}
if (!size)
return result;
tmp = *p;
found_first:
tmp &= ~0UL >> (64-size);
if (tmp == 0UL) /* Are any bits set? */
return result + size; /* Nope. */
found_middle:
return result + __ffs(tmp);
}

===

Some things that Matthew might want to try:
1) Don't inline ia64 find_next_bit
2) Hunt down and minimize uses find_next_bit (benefits more than just numamask)
3) Instead of having the loop macro evaluate to:

for (i = first_node(mask); i < MAX_NUMNODES; i = next_node(i, mask))

rather have it evaluate something like this (node_set is more efficient):

for (
({ i = 0; while(!node_set(i, mask) && i < MAX_NUMNODES) i++; i; });
i < MAX_NUMNODES;
({ i++; while(!node_set(i, mask) && i < MAX_NUMNODES) i++; i; })
)

Hmmm ... (3) looks rather nice (in an ugly sort of way ...). It might be
worth moving lower, perhaps into bitmap, for use by both cpumask and
nodemask.