[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080828183223.GA30781@localhost.localdomain>
Date: Thu, 28 Aug 2008 20:32:23 +0200
From: Vegard Nossum <vegard.nossum@...il.com>
To: David Miller <davem@...emloft.net>, Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: Pekka Enberg <penberg@...helsinki.fi>, linux-kernel@...r.kernel.org
Subject: [RFC][PATCH] bitfields API
Hi,
kmemcheck is the kernel patch that detects use of uninitialized memory.
kmemcheck reports eagerly, that is, any load of uninitialized memory
will be reported, even though the bits in question will be thrown away
later (before they are used in making a decision).
One problem that we have encountered (which generates a false positive
report) is that of bitfield accesses. In particular, whenever a bit of
a bitfield is set (or cleared), the whole byte may be loaded before the
specific bit is toggled. If the bitfield has not been accessed prior to
this, it will of course load the uninitialized byte and report this to
the user.
One solution to the problem is to tell the compiler that the memory
location in question is in fact uninitialized. This means that the
compiler can optimize out the initial load (since it will be unused in
either case), and no warning will be given by kmemcheck.
One way to tell this to the compiler is to set all the members of the
bitfield at once. We could do this unconditionally, although it adds
some overhead on certain architectures, or we can do it for certain
architectures only. My proposal is the attached patch -- a bitfields
"API". Please see the patch description for a thorough explanation.
(Please note that the patches do nothing unless the architecture is
explicit about wanting the functionality. On i386, the second patch in
the series would actually save 7 bytes, while on sparc it would take
about 48 bytes _more_.)
(The alternative to the patch is to explicitly initialize all the
fields [including any leftover "padding" fields that must also be
present] to zero at the same time, before copying in the new values.
This seems a little excessive to me, and I believe that a single
bitfield_begin_init() is neater and harder to break than an explicit
[and seemingly meaningless] list of bitfield member initializations).
So:
How do you feel about this patch? It's all about making kmemcheck more
useful... and not much else. Does it have any chance of entering the
kernel along with kmemcheck (when/if that happens)?
Thanks in advance.
Vegard
View attachment "0001-bitfields-add-bitfields-API.patch" of type "text/plain" (2267 bytes)
View attachment "0002-net-use-bitfields-API-in-skbuff.patch" of type "text/plain" (1708 bytes)
Powered by blists - more mailing lists