Weekly Edition
Return to the Kernel page
| |||||||||||||
Introducing gfp_t
Most kernel functions which deal with memory allocation take a set of "GFP
flags" as an argument. These flags describe the allocation and how it
should be satisfied; among other things, they control whether it is
possible to sleep while waiting for memory, whether high memory can be
used, and whether it is possible to call into the filesystem code. The
flags are a simple integer value, and that leads to a potential problem:
coding errors could result in functions being called with incorrect
arguments. An occasional error has turned up where function arguments
have gotten confused (usually through ordering mistakes). The resulting
bugs can be strange and hard to track down.
A while back, the __nocast attribute was added to catch these mistakes. This attribute simply says that automatic type coercion should not be applied; it is used by the sparse utility. A more complete solution is on the way, now, in the form of a new gfp_t type. The patch defining this type, and changing several kernel interfaces, was posted by Al Viro and merged just before 2.6.14-rc4 came out. There are several more patches in the series, but they have evidently been put on hold for now. The patches are surprisingly large and intrusive; it turns out that quite a few kernel functions accept GFP flags as arguments. For all that, the actual code generated does not change, and the code, as seen by gcc, changes very little. Once the patch set is complete, however, it will allow comprehensive type checking of GFP flag arguments, catching a whole class of potential bugs before they bite anybody. (Log in to post comments)
sparse is not to blame Posted Oct 13, 2005 5:49 UTC (Thu) by proski (subscriber, #104) [Link] It's important to note that the patch is not dictated by any shortcomings of sparse. No good code checker would like a function that takes enum as argument but passes it as int to other functions. Also, the patch mostly replaces "unsigned int __nocast", which is sparse specific, with gfp_t, which is not (except that its definition uses __nocast in one header file).
sparse is not to blame Posted Oct 14, 2005 9:00 UTC (Fri) by viro (subscriber, #7872) [Link] __nocast is actually not the best way to do that in sparse (thusthe switch to __bitwise later in the series); it's far too easy to lose without sparse noticing. E.g. a function with __nocast foo argument can pass it to function taking foo and nobody will notice; it warns on implicit conversions, which catches quite a few mismatches, but not all of them. __bitwise is a type qualifier and it's a lot stronger. OTOH, it simply had not been there when Linus first added annotations on (some) allocation functions.
What happens now is (a) switch to typedef (merged); (b) switch to
Bugs cleaned out? Posted Oct 13, 2005 8:03 UTC (Thu) by ncm (subscriber, #165) [Link] If these values are used that much, and if this change helps much, then there must be a few bugs that the tightening up brought to light. How many and where?
g_fpt: Bugs cleaned out? Posted Oct 13, 2005 17:27 UTC (Thu) by Duncan (guest, #6647) [Link] The values are used a lot, sure, but the patch wasn't introduced due toknown current errors, of which there (apparently) aren't any known currently, but to prevent future errors of a type they've seen too many of in the past. ... By my reading as a non-coder, simply reading Jon's description, anyway... Duncan
Bugs cleaned out? Posted Oct 14, 2005 9:05 UTC (Fri) by viro (subscriber, #7872) [Link] One in the last sweep (relayfs), quite a few back when I've done thatpatchset originally (back in June, IIRC). Fixes got merged, while annotations themselves sat around and suffered from bitrot for a while, so I ended up redoing annotations from scratch...
|
|||||||||||||