2004-10-10 15:29:02

by Oleg Nesterov

[permalink] [raw]
Subject: [RFC] __initdata strings

Hello.

This patch is not intended for inclusion, just for illustration.

__init functions leaves strings (mainly printk's arguments) in
.data section. It make sense to move them in .init.data.

Is there anyone else who would consider this useful?

Oleg.

Signed-off-by: Oleg Nesterov <[email protected]>

diff -rup 2.6.9-rc3-clean/include/linux/init.h 2.6.9-rc3-i_str/include/linux/init.h
--- 2.6.9-rc3-clean/include/linux/init.h Sun Oct 10 11:48:46 2004
+++ 2.6.9-rc3-i_str/include/linux/init.h Sun Oct 10 11:55:07 2004
@@ -61,6 +61,31 @@
/*
* Used for initialization calls..
*/
+
+#define I_STRING(str) \
+({ \
+ static char data[] __initdata = (str); \
+ data; \
+})
+
+#if defined(__GNUC__) && (__GNUC__ >= 3)
+#define CK_FMTSTR(expr) do if (0) { expr; } while (0)
+#else
+#define CK_FMTSTR(expr) do ; while (0)
+#endif
+
+#define i_printk(fmt, args...) \
+({ \
+ CK_FMTSTR(printk(fmt , ##args)); \
+ printk(I_STRING(fmt) , ##args); \
+})
+
+#define i_panic(fmt, args...) \
+({ \
+ CK_FMTSTR(panic(fmt , ##args)); \
+ panic(I_STRING(fmt) , ##args); \
+})
+
typedef int (*initcall_t)(void);
typedef void (*exitcall_t)(void);

diff -rup 2.6.9-rc3-clean/init/do_mounts.c 2.6.9-rc3-i_str/init/do_mounts.c
--- 2.6.9-rc3-clean/init/do_mounts.c Sun Oct 10 11:48:57 2004
+++ 2.6.9-rc3-i_str/init/do_mounts.c Sat Oct 9 18:18:30 2004
@@ -265,10 +265,10 @@ static int __init do_mount_root(char *na

sys_chdir("/root");
ROOT_DEV = current->fs->pwdmnt->mnt_sb->s_dev;
- printk("VFS: Mounted root (%s filesystem)%s.\n",
+ i_printk("VFS: Mounted root (%s filesystem)%s.\n",
current->fs->pwdmnt->mnt_sb->s_type->name,
current->fs->pwdmnt->mnt_sb->s_flags & MS_RDONLY ?
- " readonly" : "");
+ I_STRING(" readonly") : "");
return 0;
}

@@ -296,13 +296,13 @@ retry:
* and bad superblock on root device.
*/
__bdevname(ROOT_DEV, b);
- printk("VFS: Cannot open root device \"%s\" or %s\n",
+ i_printk("VFS: Cannot open root device \"%s\" or %s\n",
root_device_name, b);
- printk("Please append a correct \"root=\" boot option\n");
+ i_printk("Please append a correct \"root=\" boot option\n");

- panic("VFS: Unable to mount root fs on %s", b);
+ i_panic("VFS: Unable to mount root fs on %s", b);
}
- panic("VFS: Unable to mount root fs on %s", __bdevname(ROOT_DEV, b));
+ i_panic("VFS: Unable to mount root fs on %s", __bdevname(ROOT_DEV, b));
out:
putname(fs_names);
}
@@ -336,7 +336,7 @@ void __init change_floppy(char *fmt, ...
sys_ioctl(fd, FDEJECT, 0);
sys_close(fd);
}
- printk(KERN_NOTICE "VFS: Insert %s and press ENTER\n", buf);
+ i_printk(KERN_NOTICE "VFS: Insert %s and press ENTER\n", buf);
fd = sys_open("/dev/console", O_RDWR, 0);
if (fd >= 0) {
sys_ioctl(fd, TCGETS, (long)&termios);
@@ -357,7 +357,7 @@ void __init mount_root(void)
if (mount_nfs_root())
return;

- printk(KERN_ERR "VFS: Unable to mount root fs via NFS, trying floppy.\n");
+ i_printk(KERN_ERR "VFS: Unable to mount root fs via NFS, trying floppy.\n");
ROOT_DEV = Root_FD0;
}
#endif


2004-10-10 15:45:20

by Andi Kleen

[permalink] [raw]
Subject: Re: [RFC] __initdata strings

Oleg Nesterov <[email protected]> writes:

> Hello.
>
> This patch is not intended for inclusion, just for illustration.
>
> __init functions leaves strings (mainly printk's arguments) in
> .data section. It make sense to move them in .init.data.
>
> Is there anyone else who would consider this useful?

There is a more generic way to do this with gcc extensions. Something like
(uncompiled/untested)

#define __i(x) ({ static char __str[] __initdata = x; __str; })

But I'm not sure the few bytes saved are worth the code uglification.
Probably not. likely/unlikely is already bad enough.

-Andi

2004-10-10 16:12:53

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [RFC] __initdata strings

Andi Kleen wrote:
>
> There is a more generic way to do this with gcc extensions. Something like
>
> #define __i(x) ({ static char __str[] __initdata = x; __str; })

I can't see any difference with:

#define I_STRING(str) \
({ \
static char data[] __initdata = (str); \
data; \
})

> But I'm not sure the few bytes saved are worth the code uglification.

Probably you are right, but i think it would be few kilobytes,
there are so many __init functions.

Oleg.

2004-10-10 16:31:58

by Matt Mackall

[permalink] [raw]
Subject: Re: [RFC] __initdata strings

On Sun, Oct 10, 2004 at 07:28:29PM +0400, Oleg Nesterov wrote:
> Hello.
>
> This patch is not intended for inclusion, just for illustration.
>
> __init functions leaves strings (mainly printk's arguments) in
> .data section. It make sense to move them in .init.data.

Probably better to do this with something like objcopy?

--
Mathematics is the supreme nostalgia of our time.

2004-10-10 16:33:58

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC] __initdata strings



Oleg Nesterov wrote:
> Andi Kleen wrote:
>
>>There is a more generic way to do this with gcc extensions. Something like

Or with a pre-processor, using a sparse based tool such as this one I've
written a long time ago:

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

That uses what Andi described, but does this automatically if initstr is
made part of the building process.

>>
>>#define __i(x) ({ static char __str[] __initdata = x; __str; })

Regards,

- Arnaldo

2004-10-10 16:38:04

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC] __initdata strings



Matt Mackall wrote:
> On Sun, Oct 10, 2004 at 07:28:29PM +0400, Oleg Nesterov wrote:
>
>>Hello.
>>
>>This patch is not intended for inclusion, just for illustration.
>>
>>__init functions leaves strings (mainly printk's arguments) in
>>.data section. It make sense to move them in .init.data.
>
>
> Probably better to do this with something like objcopy?
>

Yes, this is another way of doing that, but the kernel has to be
prepared to get such treatment, think about register functions
that only save a pointer to strings passed from __init functions...

Ah, nothing related to this specific way of doing what is intended,
any scheme that moves strings in __init functions to .data.init has
to deal with this.

- Arnaldo