[RFC PATCH 0/3] Mark literal strings in __init / __exit code

* [RFC PATCH 0/3] Mark literal strings in __init / __exit code
@ 2014-06-22 22:46 Mathias Krause
  2014-06-22 22:46 ` [RFC PATCH 1/3] init.h: Add __init_str / __exit_str macros Mathias Krause
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Mathias Krause @ 2014-06-22 22:46 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Greg Kroah-Hartman, Steven Rostedt, Joe Perches,
	Rafael J. Wysocki, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Jason Baron, Mathias Krause

This RFC series tries to address the problem of dangling strings of
__init functions after initialization, as well as __exit strings for
code not even included in the final kernel image. The code might get
freed, but the format strings are not.

One solution to the problem might be to declare variables in the code
and mark those variables as __initconst. That, though, makes the code
ugly, as can be seen, e.g., in drivers/hwmon/w83627ehf.c -- a pile of
'static const char[] __initconst' lines just for the pr_info() call.

To be able to mark strings easily patch 1 adds macros to init.h to do so
without the need to explicitly define variables in the code. Internally
it'll declare ones nonetheless, as this seem to be the only way to
attach an __attribute__() to a verbatim string. That's already enough to
solve the problem -- mark the corresponding stings by using these
macros. But patch 2 adds some syntactical sugar for the most popular use
case, by providing pr_<level> alike macros, namely pi_<level> for __init
code and pe_<level> for __exit code. This hides the use of the marker
macros behind the commonly known printing functions -- with just a
single character changed.

Patch 3 exemplarily changes all strings and format strings in
arch/x86/kernel/acpi/boot.c to use the new macros. It also addresses a
few styling issues, though. But this already leads to ~1.7 kB of r/o
data moved to the .init.rodata section, marking it for release after
init.

Open issues with this approach:

1/ When CONFIG_DYNAMIC_DEBUG is enabled, pi_debug() and pe_debug()
fall-back to pr_debug() as there is currently no way of removing the
dynamic entries from the dynamic debug code after init.

2/ The variables used in the macros of patch 1 will pollute the symtab
with unneeded entries. That'll be a problem in the KALLSYMS_ALL case
only, though. But the symtab will be huge then, anyway. However,
filtering those even in this case might be desirable.

3/ It only seamlessly integrates for the pr_<level>() kind of use cases.
For other literal strings it gets slightly less readable, e.g. this:

   strncmp(str, "s4_nohwsig", 10)

becomes this:

   strncmp(str, __init_str("s4_nohwsig"), 10)

That might be okay, though, as it marks the string clearly as an init
string, so might actually increase the understanding of the life time of
the string literal.

So, is there interest in having such macros and markings? Patch 3 shows,
that there's actual value in it. A hacked up script, dully changing
pr_<level> to pi_<level> for __init functions under arch/x86/ already is
able to move ~8kB of r/o data into the .init section. The script,
though, is dump. It does not handle any of the printk() calls, nor does
it handle panic() calls or other strings used only in initialization
code. So there's more to squeeze out. I just want to get some feedback
first.

Also documentation of the new macros is missing, maybe even a
checkpatch.pl change to propose using the new macros instead of pr_*()
or plain printk() in __init / __exit functions.

What do you think?

Regards,
Mathias

Mathias Krause (3):
  init.h: Add __init_str / __exit_str macros
  printk: Provide pi_<level> / pe_<level> macros for __init / __exit
    code
  x86, acpi: Mark __init strings as such

 arch/x86/kernel/acpi/boot.c |  162 ++++++++++++++++++++-----------------------
 include/linux/init.h        |   18 +++++
 include/linux/printk.h      |   52 ++++++++++++++
 3 files changed, 144 insertions(+), 88 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 26+ messages in thread