Messages in this thread | | | Date | Fri, 16 Jan 2004 15:29:48 -0800 | From | Matthew Dobson <> | Subject | Re: 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/
| |