From: Jeff Dike <jdike@ka...> - 2002-03-19 04:33:01
OK, I put in the stuff that seemed to make sense. I have questions about
some of it.
-static inline int ubd_test_bit(int bit, unsigned long *data)
+static inline int ubd_test_bit(__u64 bit, unsigned char *data)
Are we really expecting bit numbers > 4G? That would imply a > 2^41 byte ubd
device with 512 byte sectors. That's really quite large. I included this
because with the new block layer in 2.5, files that large are possible.
Also, fixing the endianness bug was done with the data becoming
unsigned char * and it's not obvious that this different calculation
- bits = sizeof(data) * 8;
- n = bit / bits;
- off = bit % bits;
+ n = bit >> 3; /* get byte index */
+ off = bit & 0x07; /* get bit within that byte */
return((data[n] & (1 << off)) != 0);
is better, either in terms of code or style. Feel free to argue, but I'm
just putting in the redeclaration of data for now.
- int bits, n, off;
+ __u64 n, off;
Declaring off as __u64 is silly.
Also, putting __u64 in the interface is a little dangerous because it's
included into both user and kernel code. If the userspace headers and kernel
headers ever disagreed on what __u64 is, than that could be somewhat
> Here is a shuffle to io_thread_req items first pointers then 64, then
> 32 then enum
I'm not a fan of arranging things in arbitrary ways such as sorting them
by size. If anything, they should be arranged functionally. Plus having
the enum at the top makes sense to me because that decides what the request
as a whole is.
> eventually it would be nice to drop the bitmaps and only pass a run of
I thought that, too, once. However, the bitmap is in virtual memory, and it's
surprisingly hard to make sure that the IO thread has up to date mappings of
the kernel's virtual memory. I gave up and decided just to copy the affected
bitmap data across to it.
Also, those patches were nicely split up. That's much more pleasant than
the 1000-line glob of diffs.
Get latest updates about Open Source Projects, Conferences and News.