2008-02-04 15:49:44

by Johannes Weiner

[permalink] [raw]
Subject: [RFC] Sectionized printk data

Hi,

current approaches to have printk format strings in the corresponding
data section to the function they appear in look like the following (at
least what I have seen so far):

int __init some_function(void)
{
static char errmsg[] __initdata = "failure %s in %s\n";

[...]
printk(errmsg);
[...]
}

The attached patch allows something along the lines:

int __init some_function(void)
{
[...]
pr_init(KERN_WARNING "failure %s in %s\n", ...);
[...]
}

Another idea I had was to make printk a macro that figures out the
section of the surrounding function and then moves the data
automatically when it is a literal, but I couldn't find mechanisms that
allow this. Anyone of you got an idea?

What do you think in general?

Hannes

---
From: Johannes Weiner <[email protected]>

Sectionized printk wrappers

Introduce printk wrappers that place format string literals in init/exit
data sections.

Signed-off-by: Johannes Weiner <[email protected]>
---
The place for these wrappers is probably very wrong. Suggestions
welcome.

<Imagine diff-stat here>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ff356b2..6a1355d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -192,7 +192,22 @@ static inline int __cold printk(const char *s, ...) { return 0; }
static inline int log_buf_get_len(void) { return 0; }
static inline int log_buf_read(int idx) { return 0; }
static inline int log_buf_copy(char *dest, int idx, int len) { return 0; }
-#endif
+
+/* Sectionized printk constant data */
+#include <linux/init.h>
+#define pr_section(sec, fmt, args...) ({ \
+ static const char __fmt[] sec = fmt; \
+ printk(__fmt, ## args); \
+})
+#define pr_init(fmt, args) pr_section(__initdata, fmt, ## args)
+#define pr_exit(fmt, args) pr_section(__exitdata, fmt, ## args)
+#define pr_devinit(fmt, args) pr_section(__devinitdata, fmt, ## args)
+#define pr_devexit(fmt, args) pr_section(__devexitdata, fmt, ## args)
+#define pr_cpuinit(fmt, args) pr_section(__cpuinitdata, fmt, ## args)
+#define pr_cpuexit(fmt, args) pr_section(__cpuexitdata, fmt, ## args)
+#define pr_meminit(fmt, args) pr_section(__meminitdata, fmt, ## args)
+#define pr_memexit(fmt, args) pr_section(__memexitdata, fmt, ## args)
+#endif /* CONFIG_PRINTK */

extern void __attribute__((format(printf, 1, 2)))
early_printk(const char *fmt, ...);


2008-02-04 18:07:56

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data

On Mon, Feb 04, 2008 at 04:48:34PM +0100, Johannes Weiner wrote:
> Hi,
>
> current approaches to have printk format strings in the corresponding
> data section to the function they appear in look like the following (at
> least what I have seen so far):
>
> int __init some_function(void)
> {
> static char errmsg[] __initdata = "failure %s in %s\n";
>
> [...]
> printk(errmsg);
> [...]
> }
>
> The attached patch allows something along the lines:
>
> int __init some_function(void)
> {
> [...]
> pr_init(KERN_WARNING "failure %s in %s\n", ...);
> [...]
> }
>
> Another idea I had was to make printk a macro that figures out the
> section of the surrounding function and then moves the data
> automatically when it is a literal, but I couldn't find mechanisms that
> allow this. Anyone of you got an idea?
>
> What do you think in general?

What is the rationale behind this?
In other words why should we investigate time looking into the matter?

If you say "saving memory" then please let us know with specific examples
in what area these savings will really pay off.

Thanks,
Sam

2008-02-04 20:24:48

by Johannes Weiner

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data

Hi Sam,

Sam Ravnborg <[email protected]> writes:

> On Mon, Feb 04, 2008 at 04:48:34PM +0100, Johannes Weiner wrote:
>> Hi,
>>
>> current approaches to have printk format strings in the corresponding
>> data section to the function they appear in look like the following (at
>> least what I have seen so far):
>>
>> int __init some_function(void)
>> {
>> static char errmsg[] __initdata = "failure %s in %s\n";
>>
>> [...]
>> printk(errmsg);
>> [...]
>> }
>>
>> The attached patch allows something along the lines:
>>
>> int __init some_function(void)
>> {
>> [...]
>> pr_init(KERN_WARNING "failure %s in %s\n", ...);
>> [...]
>> }
>>
>> Another idea I had was to make printk a macro that figures out the
>> section of the surrounding function and then moves the data
>> automatically when it is a literal, but I couldn't find mechanisms that
>> allow this. Anyone of you got an idea?
>>
>> What do you think in general?
>
> What is the rationale behind this?
> In other words why should we investigate time looking into the matter?
>
> If you say "saving memory" then please let us know with specific examples
> in what area these savings will really pay off.

I was not claiming anything. The thing is that I have seen people using
code as schematized in the first code example. And the point of this
patch is to make the placing of string literals in disposable sections -
which is already done - more convenient.

Using code as in the first example is just ugly. My approach is to make
it less ugly. But after all, it was not my idea to move the string
literals.

drivers/net/3c505.c does it, grep for notfound_msg for an in-tree
example.

You were asking me my own question. I placed an `RFC' in the Subject:
because I am interested in your (and others) view on this topic.

I will convert some of the code to use the macros I introduced and look
if it gains some significant size improvements on a common desktop
configuration as I have it on my laptop.

Hannes

2008-02-09 22:08:55

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data


On Feb 4 2008 19:07, Sam Ravnborg wrote:
>> The attached patch allows something along the lines:
>>
>> int __init some_function(void)
>> {
>> [...]
>> pr_init(KERN_WARNING "failure %s in %s\n", ...);
>> [...]
>> }
>>
>> Another idea I had was to make printk a macro that figures out the
>> section of the surrounding function and then moves the data
>> automatically when it is a literal, but I couldn't find mechanisms that
>> allow this. Anyone of you got an idea?
>>
>> What do you think in general?
>
>What is the rationale behind this?

To drop strings that are only shown once anyway, such as:

static int __init ebtables_init(void)
{
int ret;

mutex_lock(&ebt_mutex);
list_add(&ebt_standard_target.list, &ebt_targets);
mutex_unlock(&ebt_mutex);
if ((ret = nf_register_sockopt(&ebt_sockopts)) < 0)
return ret;

-> printk(KERN_INFO "Ebtables v2.0 registered\n");
return 0;
}

>If you say "saving memory" then please let us know with specific examples
>in what area these savings will really pay off.

2008-02-09 23:54:52

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data

Em Sat, Feb 09, 2008 at 11:08:45PM +0100, Jan Engelhardt escreveu:
>
> On Feb 4 2008 19:07, Sam Ravnborg wrote:
> >> The attached patch allows something along the lines:
> >>
> >> int __init some_function(void)
> >> {
> >> [...]
> >> pr_init(KERN_WARNING "failure %s in %s\n", ...);
> >> [...]
> >> }
> >>
> >> Another idea I had was to make printk a macro that figures out the
> >> section of the surrounding function and then moves the data
> >> automatically when it is a literal, but I couldn't find mechanisms that
> >> allow this. Anyone of you got an idea?
> >>
> >> What do you think in general?
> >
> >What is the rationale behind this?
>
> To drop strings that are only shown once anyway, such as:
>
> static int __init ebtables_init(void)
> {
> int ret;
>
> mutex_lock(&ebt_mutex);
> list_add(&ebt_standard_target.list, &ebt_targets);
> mutex_unlock(&ebt_mutex);
> if ((ret = nf_register_sockopt(&ebt_sockopts)) < 0)
> return ret;
>
> -> printk(KERN_INFO "Ebtables v2.0 registered\n");
> return 0;
> }
>
> >If you say "saving memory" then please let us know with specific examples
> >in what area these savings will really pay off.

A long time ago I played with this, using a sparse based tool that was
inserted as the compiler and modified the code before passing to gcc,
i.e. a pre-pre-processor:

http://www.kernel.org/pub/linux/kernel/people/acme/sparse/initstr.c

I couldn't find in the archives, but IIRC some extra pages were freed
after boot, i.e. strings moved from .data to .init.data.

With a tool like this the advantage is that no source code has to be
changed, strings in __init functions are automagically moved to
.init.data, the disadvantage is that not all strings can be moved to
.init.data as there were (are?) subsystems that keep pointers to the
string passed and another tool would be involved in the build process.

- Arnaldo

2008-02-10 00:18:30

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data


On Feb 9 2008 21:54, Arnaldo Carvalho de Melo wrote:
>> To drop strings that are only shown once anyway, such as:
>>
>> static int __init ebtables_init(void)
>> {
>> int ret;
>>
>> mutex_lock(&ebt_mutex);
>> list_add(&ebt_standard_target.list, &ebt_targets);
>> mutex_unlock(&ebt_mutex);
>> if ((ret = nf_register_sockopt(&ebt_sockopts)) < 0)
>> return ret;
>>
>> -> printk(KERN_INFO "Ebtables v2.0 registered\n");
>> return 0;
>> }
>>
>> >If you say "saving memory" then please let us know with specific examples
>> >in what area these savings will really pay off.
>
>[...]
>With a tool like this the advantage is that no source code has to be
>changed, strings in __init functions are automagically moved to
>.init.data, the disadvantage is that not all strings can be moved to
>.init.data as there were (are?) subsystems that keep pointers to the
>string passed and another tool would be involved in the build process.

There is one corner case to consider:


static char abc[] = "foo";

int __init init_module(void)
{
printk(abc);
}

I am not sure if gcc/ld is smart enough to figure out that abc is
only ever used from within an __init function and that it could hence
be moved to __initdata.

2008-02-10 00:27:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC] Sectionized printk data

Em Sun, Feb 10, 2008 at 01:18:18AM +0100, Jan Engelhardt escreveu:
>
> On Feb 9 2008 21:54, Arnaldo Carvalho de Melo wrote:
> >> To drop strings that are only shown once anyway, such as:
> >>
> >> static int __init ebtables_init(void)
> >> {
> >> int ret;
> >>
> >> mutex_lock(&ebt_mutex);
> >> list_add(&ebt_standard_target.list, &ebt_targets);
> >> mutex_unlock(&ebt_mutex);
> >> if ((ret = nf_register_sockopt(&ebt_sockopts)) < 0)
> >> return ret;
> >>
> >> -> printk(KERN_INFO "Ebtables v2.0 registered\n");
> >> return 0;
> >> }
> >>
> >> >If you say "saving memory" then please let us know with specific examples
> >> >in what area these savings will really pay off.
> >
> >[...]
> >With a tool like this the advantage is that no source code has to be
> >changed, strings in __init functions are automagically moved to
> >.init.data, the disadvantage is that not all strings can be moved to
> >.init.data as there were (are?) subsystems that keep pointers to the
> >string passed and another tool would be involved in the build process.
>
> There is one corner case to consider:
>
>
> static char abc[] = "foo";
>
> int __init init_module(void)
> {
> printk(abc);
> }
>
> I am not sure if gcc/ld is smart enough to figure out that abc is
> only ever used from within an __init function and that it could hence
> be moved to __initdata.

The initstr tool mentioned doesn't touches this case, as it doesn't
searches specific functions such as printk, it looks for strings inside
__init marked functions. In the above example abc won't be marked as
__initdata.

So if there are two places where the same string is used, with one being
in a __init function one copy goes to .init.data and another to .data.

- Arnaldo