lkml.org 
[lkml]   [2004]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: seperator error in __mask_snprintf_len
FWIW, I think a 32-bit chunk-size is preferable.  I personally don't 
think "dead,beef" is easier to read than "deadbeef", nor "0073,0f0f"
compared to "00730f0f". On the small CPU count, ie: <=32, either
version is pretty readable, but on larger CPU count boxen you're going
to overflow your brain counting groups of 4 versus groups of 8. I can't
believe I just called a 32 CPU box "small". My world perspective is a
bit skewed... ;)

Also, on the input side, a lot of apps will output a 32-bit CPU mask.
With commas separating every 32 bits, we can feed an "uncommafied" mask
to the kernel and it won't barf. If we go with 16-bit chunks, we'll
have to "commafy" these 32-bit bitmasks to feed them to the kernel.

As a NUMA guy who deals with largish CPU count machines daily, that's my
2 cents...

-Matt


Paul Jackson wrote:
>>This patch captures what I am looking for in bitmap display and input.
>
>
> Interesting. I appear to have provoked Joe into a burst of coding.
> Now, if I had any smarts, I would stand aside and let Joe own this,
> just as Bill Irwin did when I posted my initial lib/mask.c patch a
> couple months ago.
>
>
> Andrew:
>
> If you find Joe's coding more to your liking than my "Gad" style,
> I will bless this, and after tossing in a few parting shots, will
> stand aside. It meets my essential needs, which were:
> - chunked output (a comma every 16 or 32 bits),
> - symmetric input and output formats, and
> - display and parsing code generic to diverse bitmap sizes.
>
> My actual recommendation, if however you are still undecided, is:
> - my patch of last night (with the M32X() 64 bit big endian fix),
> - Joe's recommended format, zero-filling to chunksize each word,
> - Joe's renaming/refactoring from lib/mask.c to lib/bitmap.h, and
> - a chunksize of 16 rather than 32 (Joe likes 16, I don't care).
>
> The essential differences between Joe's and my proposals that I see are:
> - Joe's has more code, especially in the parsing routine,
> - Joe's bitmap size resolution is bits, not words, and
> - I use an implied alloca of 4 x sizeof(mask) bytes.
>
>
> Comments on Joe's patch:
>
>
>>ChangeLog:
>
>
> Good job of summarizing for the Changelog the changes.
>
>
>>o move into the bitmap.h family, rename and refactor interface to match
>
>
> I think I like this - good.
>
>
>> o bitmap size resolution changed from byte to bit
>
>
> Why? This adds a fair bit of complexity to the code, I suspect.
> I am not aware of a need for this, but if there is one, ok.
>
>
>>o chunking (digits between commas) changed from 8 to 4 digits
>
>
> Ok - either 8 or 4 works for me. This detail should be decided
> by those working on 16 to 32 cpu systems, who will notice this
> choice the most. Those of us on larger or smaller systems are
> going to see, or not see, separators in either case.
>
>
>> o display no longer affected by sizeof(unsigned long).
>
>
> A bit of a misstatement. The display was only affected by the
> chunksize, one of 32 (sizeof(u32)*8) or 16 (CHUNKSZ).
>
>
>> o no alloca usage
>
>
> True. Though on the other hand, you need to roll your own parsing
> code of comma-separated chunks, instead of getting by with using
> strsep(). So you trade alloca usage for code complexity. Either
> way works - coders choice. I agree that we disagree on this tradeoff.
>
>
>> o works correctly independent of the size of an unsigned long.
>
>
> And my version doesn't?
>
>
>> o works on big and little endian machine.
>
>
> With my M32X() eor-1 fix, so did mine.
>
>
>>+ * lib/bitmask.c - bitmask manipulation routines too big to go into bitmask.h
>
>
> Typo? Did you mean bitmap.c and bitmap.h, not bitmask?
>
>
>>+int bitmap_parse(const char __user *ubuf, unsigned int ubuflen,
>>+ unsigned long *maskp, unsigned int nmaskbits)
>
>
> This routine has quite a bit more code detail than my corresponding
> parsing routine. I hope that:
> (1) providing bit-level resolution, and
> (2) removing the implied alloca
> justifies this increase in code detail.
>
>
>>+ if (isspace(c))
>>+ continue;
>
>
> So a space embedded in a hex number is skipped? That is, your code
> parses "dead,beef" and "de a d, bee f" the same? This seems strange.
> Perhaps you would prefer to suppress only leading spaces in each chunk:
>
> if (!n && isspace(c))
> continue;
>
>
>>+ for (j = 0; j < 32; j++) {
>
>
> What's this "32", an unrepentant CHUNKSZ?
>


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:00    [W:0.219 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site