Re: [PATCH] mask ADT: new mask.h file [2/22]

Paul Jackson
2004-04-06T10:43:38Z

Rusty - thank-you very much for your constructive feedback so far.

Seems to me that we are in agreement that slimming down the
internals of cpumask_t is worth proceeding with, but not on possible
changes to the cpumask API seen by the rest of the kernel.

Following your suggestion, I have a new bitmap.c/bitmap.h working in my
workarea, that pulls in the changes I had done before in mask.h, such as
special casing bitmaps of a single word at compile time to use native C
ops.

A sample of the code in my include/linux/bitmap.h:

static inline void bitmap_and(unsigned long *d, const unsigned long *s1,
const unsigned long *s2, int nbits)
{
if (nbits <= BITS_PER_LONG)
d[0] = s1[0] & s2[0];
else
_bitmap_and(d, s1, s2, nbits);
}

And in lib/bitmap.c, the routine that was called bitmap_and() is now
called _bitmap_and(), and is (as before) not inlined.

I will continue with the coding to remove the mask ADT header file
include/linux/mask.h from my patch set, recoding cpumask.h (and
Matthew's nodemask.h) directly on top of bitmap/bitops, and defining
cpumask_t as:

typedef struct { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;

I'd like to table for the moment discussions of changing the cpumask API
seen by the rest of the kernel code. Some of us like what we have, and
we don't have anything resembling a consensus on something else. So
unless you want to agree that whatever I said is entirely right <grin>,
we lack the concensus needed to make pervasive cpumask API changes.

The above changes to the innards of cpumask will open the door to the
alternatives being considered (struct vs typedef, full vs partial vs no
extra 'cpumask' api, explicit vs implicit '&' pass by reference).

I acknowledge once again Bill Irwin's notice that this presents some
arch-specific risks. I've done some reasonable work resolving these for
sparc, i386 and ia64. But I still need to seek further feedback and
approval/disapproval from the arch maintainers. My current expectation
is that any remaining arch-specific issues will be narrow enough, and
that the gains from this infrastructure cleanup are broad enough, that
we will be able to resolve the arch-specific issues without compromising
the overall cleanup. Only time will tell for sure.

For now I expect to preserve the current implicit pass by reference that
(some of) the existing cpumask implementations used, I will preserve all
28 or so cpumask ops with their current signature, and I will likely use
a layer of static inlines to increase type safety.

That is, cpumasks will be bitmap arrays wrapped in a struct, and the ops
on them typically will be bitmap ops wrapped in inlines wrapped in macros.

I agree that there's a whole lot of wrapping going on there ... ;).

Continued feedback welcome.