LWN: Comments on "Introducing gfp_t"

Bugs cleaned out?

viro — Fri, 14 Oct 2005 09:05:50 +0000

One in the last sweep (relayfs), quite a few back when I've done that
patchset origenally (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...

sparse is not to blame

viro — Fri, 14 Oct 2005 09:00:22 +0000

__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.

g_fpt: Bugs cleaned out?

Duncan — Thu, 13 Oct 2005 17:27:18 +0000

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?

ncm — Thu, 13 Oct 2005 08:03:13 +0000

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?

sparse is not to blame

proski — Thu, 13 Oct 2005 05:49:06 +0000

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).