Linus Torvalds wrote:
On Tue, 4 Dec 2007, Andi Drebes wrote:
Perhaps I'm missing somehting, but I think for cramfs, unfortunately,
there has to be this statement. The bitfields in the cramfs_inode structure
cause some problems.
I agree that bitfields can be painful, but they should likely be just
rewritten to be accesses using actual masks and shifts. The thing is,
bitfields aren't actually endianness safe *anyway*, in that a compiler may
end up using a *different* bit order than the byte order. So you cannot
really use bitfields reliably on things like that (although Linux has a
notion of a "__[BIG|LITTLE]_ENDIAN_BITFIELD", if you really want to).
Bitfields also generate lower-quality assembly than masks&shifts
(typically more instructions using additional temporaries to accomplish
the same thing), based on my own informal gcc testing.
You would think gcc would be advanced enough to turn bitfield use into
masks and shifts under the hood, but for whatever reason that often is
not the case in kernel code.
Due to the way they're used, bitfields make more difficult the common
code pattern of setting several flags at once:
(assuming 'foo', 'bar' and 'baz' are bitfields in a struct)
pdev->foo = 1;
pdev->bar = 0;
pdev->baz = 1;
versus
flag_foo = (1 << 0);
flag_bar = (1 << 1);
flag_baz = (1 << 2);
...
pdev->flags = flag_foo | flag_bar;
</pet peeve>
And getting back on topic, I think "pdev->flags =
cpu_to_le32(flag1|flag2)" is nicer than dealing with bitfields, when
your data structures are fixed-endian.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
