Andrew,
This patch replaces the call to bitmap_shift_right() in bitmap_parse()
with bitmap_shift_left().
This mental confusion between right and left did not show up in my
(userland) testing, as I foolishly wrote my own bitmap_shift routines
rather than drag over the kernel versions. And it did not show up in my
kernel testing because no shift routine is called when NR_CPUS <= 32.
I tested this in userland with the kernel's versions of bitmap_shift_*
and compiled a kernel and spot checked it on a 2-cpu system.
I also prepended comments to the bitmap_shift_* functions defining what
'left' and 'right' means. This is under the theory that if I and all the
reviewers were bamboozled, others in the future occasionally might be too.
Regards,
Joe
--- base/lib/bitmap.c 2004-03-10 21:55:43.000000000 -0500
+++ new/lib/bitmap.c 2004-03-22 14:41:03.000000000 -0500
@@ -71,6 +71,11 @@
}
EXPORT_SYMBOL(bitmap_complement);
+/*
+ * Shifting right (dividing) means moving bits in the MS -> LS bit
+ * direction. Zeros are fed into the vacated MS positions and the
+ * LS bits shifted off the bottom are lost.
+ */
void bitmap_shift_right(unsigned long *dst,
const unsigned long *src, int shift, int bits)
{
@@ -86,6 +91,11 @@
}
EXPORT_SYMBOL(bitmap_shift_right);
+/*
+ * Shifting left (multiplying) means moving bits in the LS -> MS
+ * direction. Zeros are fed into the vacated LS bit positions
+ * and those MS bits shifted off the top are lost.
+ */
void bitmap_shift_left(unsigned long *dst,
const unsigned long *src, int shift, int bits)
{
@@ -269,7 +279,7 @@
if (nchunks == 0 && chunk == 0)
continue;
- bitmap_shift_right(maskp, maskp, CHUNKSZ, nmaskbits);
+ bitmap_shift_left(maskp, maskp, CHUNKSZ, nmaskbits);
*maskp |= chunk;
nchunks++;
nbits += (nchunks == 1) ? nbits_to_hold_value(chunk) : CHUNKSZ;
> I also prepended comments to the bitmap_shift_* functions defining what
Nice work.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373
On Mon, Mar 22, 2004 at 03:21:18PM -0500, Joe Korty wrote:
> Andrew,
> This patch replaces the call to bitmap_shift_right() in bitmap_parse()
> with bitmap_shift_left().
> This mental confusion between right and left did not show up in my
> (userland) testing, as I foolishly wrote my own bitmap_shift routines
> rather than drag over the kernel versions. And it did not show up in my
> kernel testing because no shift routine is called when NR_CPUS <= 32.
> I tested this in userland with the kernel's versions of bitmap_shift_*
> and compiled a kernel and spot checked it on a 2-cpu system.
> I also prepended comments to the bitmap_shift_* functions defining what
> 'left' and 'right' means. This is under the theory that if I and all the
> reviewers were bamboozled, others in the future occasionally might be too.
Bugfixes are always good. Maybe the kerneldoc stuff would be a good idea
for these functions, and the rest of the non-static functions ppl might
be expected to call.
-- wli
> Bugfixes are always good. Maybe the kerneldoc stuff would be a good idea
> for these functions, and the rest of the non-static functions ppl might
> be expected to call.
IMO, one+ liners describing how a function is used is best put near
the function, where it is most likely to be seen. Stuff going into
Documentation/*.txt should be bulky stuff not suitable for inlining,
such as largish tutorials, annotated examples, theory papers, etc.
Joe
> Maybe the kerneldoc stuff would be a good idea
> for these functions, and the rest of the non-static functions ppl might
> be expected to call.
Not quite sure what Bill is converying here with the qualifier 'non-static'.
My inclinations lay more toward looking for improvements, explored in
other messages on a concurrent thread "[PATCH] Introduce nodemask_t
ADT", to the cpumask API, to be followed by a kerneldoc, rather than
trying very hard to document the current API much more.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373
At some point in the past, I wrote:
>> Bugfixes are always good. Maybe the kerneldoc stuff would be a good idea
>> for these functions, and the rest of the non-static functions ppl might
>> be expected to call.
On Mon, Mar 22, 2004 at 07:14:33PM -0500, Joe Korty wrote:
> IMO, one+ liners describing how a function is used is best put near
> the function, where it is most likely to be seen. Stuff going into
> Documentation/*.txt should be bulky stuff not suitable for inlining,
> such as largish tutorials, annotated examples, theory papers, etc.
Sorry about not being clear; I meant the : and @ stuff I've seen around
various comments that somehow gets yanked directly out of C comments in
the source and generated into a pdf.
-- wli
On Mon, Mar 22, 2004 at 04:19:31PM -0800, Paul Jackson wrote:
> Not quite sure what Bill is converying here with the qualifier 'non-static'.
> My inclinations lay more toward looking for improvements, explored in
> other messages on a concurrent thread "[PATCH] Introduce nodemask_t
> ADT", to the cpumask API, to be followed by a kerneldoc, rather than
> trying very hard to document the current API much more.
non-static == exported for people to use, declared in a header, and
without the "static" qualifier to the function. Basically, things added
to the kernel API. As this is the low-level bitmap stuff, I believe it
should be relatively unchanged across whatever API changes you may have
in store for higher-level API's e.g. cpumasks. This is the bitmap
library code we're talking about, isn't it? At least it's what I'm
talking about.
-- wli
On Mon, Mar 22, 2004 at 06:09:07PM -0800, William Lee Irwin III wrote:
> At some point in the past, I wrote:
>>> Bugfixes are always good. Maybe the kerneldoc stuff would be a good idea
>>> for these functions, and the rest of the non-static functions ppl might
>>> be expected to call.
>
> On Mon, Mar 22, 2004 at 07:14:33PM -0500, Joe Korty wrote:
>> IMO, one+ liners describing how a function is used is best put near
>> the function, where it is most likely to be seen. Stuff going into
>> Documentation/*.txt should be bulky stuff not suitable for inlining,
>> such as largish tutorials, annotated examples, theory papers, etc.
>
> Sorry about not being clear; I meant the : and @ stuff I've seen around
> various comments that somehow gets yanked directly out of C comments in
> the source and generated into a pdf.
Ah, I see. Here is the New-n-Improved patch:
--- base/lib/bitmap.c 2004-03-10 21:55:43.000000000 -0500
+++ new/lib/bitmap.c 2004-03-22 23:04:51.000000000 -0500
@@ -71,6 +71,17 @@
}
EXPORT_SYMBOL(bitmap_complement);
+/*
+ * bitmap_shift_write - logical right shift of the bits in a bitmap
+ * @dst - destination bitmap
+ * @src - source bitmap
+ * @nbits - shift by this many bits
+ * @bits - bitmap size, in bits
+ *
+ * Shifting right (dividing) means moving bits in the MS -> LS bit
+ * direction. Zeros are fed into the vacated MS positions and the
+ * LS bits shifted off the bottom are lost.
+ */
void bitmap_shift_right(unsigned long *dst,
const unsigned long *src, int shift, int bits)
{
@@ -86,6 +97,17 @@
}
EXPORT_SYMBOL(bitmap_shift_right);
+/*
+ * bitmap_shift_left - logical left shift of the bits in a bitmap
+ * @dst - destination bitmap
+ * @src - source bitmap
+ * @nbits - shift by this many bits
+ * @bits - bitmap size, in bits
+ *
+ * Shifting left (multiplying) means moving bits in the LS -> MS
+ * direction. Zeros are fed into the vacated LS bit positions
+ * and those MS bits shifted off the top are lost.
+ */
void bitmap_shift_left(unsigned long *dst,
const unsigned long *src, int shift, int bits)
{
@@ -269,7 +291,7 @@
if (nchunks == 0 && chunk == 0)
continue;
- bitmap_shift_right(maskp, maskp, CHUNKSZ, nmaskbits);
+ bitmap_shift_left(maskp, maskp, CHUNKSZ, nmaskbits);
*maskp |= chunk;
nchunks++;
nbits += (nchunks == 1) ? nbits_to_hold_value(chunk) : CHUNKSZ;
On Mon, Mar 22, 2004 at 06:09:07PM -0800, William Lee Irwin III wrote:
>> Sorry about not being clear; I meant the : and @ stuff I've seen around
>> various comments that somehow gets yanked directly out of C comments in
>> the source and generated into a pdf.
On Mon, Mar 22, 2004 at 11:10:13PM -0500, Joe Korty wrote:
> Ah, I see. Here is the New-n-Improved patch:
Cool! I wouldn't have said it was a requirement, but I certainly like
the updated patch.
-- wli