[RFC] __initstr & __exitstr

[RFC] __initstr & __exitstr

[Arnaldo Carvalho de Melo]

Hi,

	Please comment on this approach to move strings in __init functions
from .rodata to .data.init so that it get discarded after initialization,
like the variables marked as __initdata and the functions marked as __init,
as well as move strings in __exit marked functions to .data.exit, that will
be discarded and not even get into the generated kernel image.

	Please note that if possible the best approach was for gcc to move
those strings automatically if the function was marked with modified 
__init/__exit macros, but we have to keep in mind that some of the strings
in those functions can not be discarded because they keep being referenced
by say register_chrdev and others, unlike, for example, proc functions and
others that copy the string passed to some malloc'ed data structure, so we
have to be selective marking exactly the ones that can indeed be discarded.

	I've also implemented helper functions for printk thats the most
common case, and leaved the other common case, panic, using
__initstr/__exitstr explicitely, so that people can comment on what is
better.

	Here is the basic implementation in include/linux/init.h:

#define __initstr(s)    ({ static char __tmp_init_str[] __initdata=s;
__tmp_init_str;})
#define __exitstr(s)    ({ static char __tmp_exit_str[] __exitdata=s;
__tmp_exit_str;})
#define init_printk(fmt,arg...) printk(__initstr(fmt) , ##arg)
#define exit_printk(fmt,arg...) printk(__exitstr(fmt) , ##arg)

	For modules its a no op, as modules doesn't get rid of code/data
marked as __init{data}, please correct me if I'm wrong as I didn't checked
that in detail, but from first quick analysis it doesn't move it to some
different .data/.text section, so I assume it doesn't discards it after
initialization.
	
	For my config, compiling everything statically, with a pristine
2.4.6-ac1 kernel I get 172 KB freed after init, with this patch we save 16
KB more.

	I've put the patch at:
http://bazar.conectiva.com.br/~acme/patches/wip/__initstr.patch.4

	And yes, its intrusive, but it serves, IMHO, as an experiment to
see how much can be saved with this.

	Please advise/comment.

- Arnaldo

Re: [RFC] __initstr & __exitstr

[Philipp Rumpf]

On Fri, Jul 06, 2001 at 11:17:44PM -0300, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> 	Please comment on this approach to move strings in __init functions
> from .rodata to .data.init so that it get discarded after initialization,
> like the variables marked as __initdata and the functions marked as __init,
> as well as move strings in __exit marked functions to .data.exit, that will
> be discarded and not even get into the generated kernel image.
> 
> 	Please note that if possible the best approach was for gcc to move
> those strings automatically if the function was marked with modified 
> __init/__exit macros, but we have to keep in mind that some of the strings
> in those functions can not be discarded because they keep being referenced
> by say register_chrdev and others, unlike, for example, proc functions and
> others that copy the string passed to some malloc'ed data structure, so we
> have to be selective marking exactly the ones that can indeed be discarded.

.. or fix all registration functions to use a private copy of the string,
which would avoid some common oopses.

> 	I've also implemented helper functions for printk thats the most
> common case, and leaved the other common case, panic, using
> __initstr/__exitstr explicitely, so that people can comment on what is
> better.

> #define init_printk(fmt,arg...) printk(__initstr(fmt) , ##arg)

I dislike init_printk;  it combines variadic functions/macros with
assumptions about how the first argument is specified (i.e. as a string
constant), so it's potentially very confusing.

Also, printk is used for debugging, and accidentally using init_printk
instead of printk would result in no messages being printed at all if
the driver is compiled into the kernel (while everything would work
fine if the driver is compiled as a module, where init_printk and printk
are identical).  I think this would be very annoying to track down for
less experienced kernel hackers.

> #define __initstr(s)    ({ static char __tmp_init_str[] __initdata=s;
> __tmp_init_str;})

I think this would fail if used in structure initialisations ?

	Philipp Rumpf

Re: [RFC] __initstr & __exitstr

[Arnaldo Carvalho de Melo]

Em Fri, Jul 06, 2001 at 10:05:14PM -0500, Philipp Rumpf escreveu:
> On Fri, Jul 06, 2001 at 11:17:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > Hi,
> > 
> > 	Please comment on this approach to move strings in __init functions
> > from .rodata to .data.init so that it get discarded after initialization,
> > like the variables marked as __initdata and the functions marked as __init,
> > as well as move strings in __exit marked functions to .data.exit, that will
> > be discarded and not even get into the generated kernel image.
> > 
> > 	Please note that if possible the best approach was for gcc to move
> > those strings automatically if the function was marked with modified 
> > __init/__exit macros, but we have to keep in mind that some of the strings
> > in those functions can not be discarded because they keep being referenced
> > by say register_chrdev and others, unlike, for example, proc functions and
> > others that copy the string passed to some malloc'ed data structure, so we
> > have to be selective marking exactly the ones that can indeed be discarded.
> 
> .. or fix all registration functions to use a private copy of the string,
> which would avoid some common oopses.

yes, that would be nice, if allowed I can put my janitor hat and do that :)
 
> > 	I've also implemented helper functions for printk thats the most
> > common case, and leaved the other common case, panic, using
> > __initstr/__exitstr explicitely, so that people can comment on what is
> > better.
> 
> > #define init_printk(fmt,arg...) printk(__initstr(fmt) , ##arg)
> 
> I dislike init_printk;  it combines variadic functions/macros with
> assumptions about how the first argument is specified (i.e. as a string
> constant), so it's potentially very confusing.
> 
> Also, printk is used for debugging, and accidentally using init_printk
> instead of printk would result in no messages being printed at all if
> the driver is compiled into the kernel (while everything would work
> fine if the driver is compiled as a module, where init_printk and printk
> are identical).  I think this would be very annoying to track down for
> less experienced kernel hackers.

that's why I've didn't coded a init_panic :) ok, I can change this if the
rest of the patch is considered ok
 
> > #define __initstr(s)    ({ static char __tmp_init_str[] __initdata=s;
> > __tmp_init_str;})
> 
> I think this would fail if used in structure initialisations ?

yes, this fails in such cases, any ideas on another approach that works in
this case as well in the other cases covered by the patch?

Also please note that strings in __initdata marked structs with just 'char
*' instances are going to .rodata and not getting discarded, thats why I've
changed some to char[SOME_MAX_STRING_SIZE], so that it gets moved to
.data.init, crude, but works, I'm interested in another approach for this
case as well.

Thanks for the comments.

- Arnaldo