Introducing gfp_t [LWN.net]

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 (thus
the 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
use of __bitwise in declaration and propagation of gfp_t use to
the rest of places where it's needed (post-2.6.14). Full set of
patches is done, it's just waiting for 2.6.14 to be released.

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 to
known 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 that
patchset 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...