On Fri, Feb 09, 2007 at 05:11:32PM +0200, Alon Bar-Lev wrote:
>
> Follow-up Russell King comment at http://lkml.org/lkml/2007/1/22/267
>
> All __initdata variables should be initialized so they won't end up
> in BSS.
>
> There is no dependency between patches or even hunks.
>
> Some architecture patches are untested, this is documented as "UNTESTED"
>
> Against 2.6.20-rc6-mm3.
To quote parts of that:
Anyway, here's what the GCC manual has to say about use of
__attribute__((section)) on variables:
`section ("SECTION-NAME")'
Use the `section' attribute with an _initialized_ definition of a
_global_ variable, as shown in the example. GCC issues a warning
and otherwise ignores the `section' attribute in uninitialized
variable declarations.
You may only use the `section' attribute with a fully initialized
global definition because of the way linkers work. The linker
requires each object be defined once, with the exception that
uninitialized variables tentatively go in the `common' (or `bss')
section and can be multiply "defined". You can force a variable
to be initialized with the `-fno-common' flag or the `nocommon'
attribute.
And the top-level Makefile has:
CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common
Note the -fno-common.
And indeed all the __initdata annotated local and global variables on
s390 are in the init.data section. So I'm wondering what this patch
series is about. Or I must have missed something.
On Friday 09 February 2007, Heiko Carstens wrote:
> And the top-level Makefile has:
>
> CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> -fno-strict-aliasing -fno-common
>
> Note the -fno-common.
>
> And indeed all the __initdata annotated local and global variables on
> s390 are in the init.data section. So I'm wondering what this patch
> series is about. Or I must have missed something.
>
Hmmm... You have a valid point!
So it reduces the patch to the following.
>From the previous discussion I was afraid that I added some invalid variables.
Thanks!
Best Regards,
Alon Bar-Lev.
---
diff -urNp linux-2.6.20-rc6-mm3.org/arch/x86_64/kernel/e820.c linux-2.6.20-rc6-mm3/arch/x86_64/kernel/e820.c
--- linux-2.6.20-rc6-mm3.org/arch/x86_64/kernel/e820.c
+++ linux-2.6.20-rc6-mm3/arch/x86_64/kernel/e820.c
@@ -402,10 +402,10 @@ static int __init sanitize_e820_map(stru
struct e820entry *pbios; /* pointer to original bios entry */
unsigned long long addr; /* address for this change point */
};
- static struct change_member change_point_list[2*E820MAX] __initdata;
- static struct change_member *change_point[2*E820MAX] __initdata;
- static struct e820entry *overlap_list[E820MAX] __initdata;
- static struct e820entry new_bios[E820MAX] __initdata;
+ static struct change_member change_point_list[2*E820MAX] __initdata = {{0}};
+ static struct change_member *change_point[2*E820MAX] __initdata = {0};
+ static struct e820entry *overlap_list[E820MAX] __initdata = {0};
+ static struct e820entry new_bios[E820MAX] __initdata = {{0}};
struct change_member *change_tmp;
unsigned long current_type, last_type;
unsigned long long last_addr;
diff -urNp linux-2.6.20-rc6-mm3.org/fs/nfs/nfsroot.c linux-2.6.20-rc6-mm3/fs/nfs/nfsroot.c
--- linux-2.6.20-rc6-mm3.org/fs/nfs/nfsroot.c 2007-01-25 04:19:28.000000000 +0200
+++ linux-2.6.20-rc6-mm3/fs/nfs/nfsroot.c 2007-01-31 22:19:30.000000000 +0200
@@ -289,7 +289,7 @@ static int __init root_nfs_parse(char *n
*/
static int __init root_nfs_name(char *name)
{
- static char buf[NFS_MAXPATHLEN] __initdata;
+ static char buf[NFS_MAXPATHLEN] __initdata = { 0, };
char *cp;
/* Set some default values */
diff -urNp linux-2.6.20-rc6-mm3.org/init/main.c linux-2.6.20-rc6-mm3/init/main.c
--- linux-2.6.20-rc6-mm3.org/init/main.c 2007-01-31 22:15:41.000000000 +0200
+++ linux-2.6.20-rc6-mm3/init/main.c 2007-01-31 22:19:30.000000000 +0200
@@ -470,7 +470,7 @@ static int __init do_early_param(char *p
void __init parse_early_param(void)
{
static __initdata int done = 0;
- static __initdata char tmp_cmdline[COMMAND_LINE_SIZE];
+ static __initdata char tmp_cmdline[COMMAND_LINE_SIZE] = "";
if (done)
return;
diff -urNp linux-2.6.20-rc6-mm3.org/drivers/input/keyboard/amikbd.c linux-2.6.20-rc6-mm3/drivers/input/keyboard/amikbd.c
--- linux-2.6.20-rc6-mm3.org/drivers/input/keyboard/amikbd.c 2007-01-25 04:19:28.000000000 +0200
+++ linux-2.6.20-rc6-mm3/drivers/input/keyboard/amikbd.c 2007-01-31 22:19:30.000000000 +0200
@@ -215,7 +215,7 @@ static int __init amikbd_init(void)
set_bit(i, amikbd_dev->keybit);
for (i = 0; i < MAX_NR_KEYMAPS; i++) {
- static u_short temp_map[NR_KEYS] __initdata;
+ static u_short temp_map[NR_KEYS] __initdata = {0};
if (!key_maps[i])
continue;
memset(temp_map, 0, sizeof(temp_map));
Hi,
On Fri, 9 Feb 2007, Heiko Carstens wrote:
> And indeed all the __initdata annotated local and global variables on
> s390 are in the init.data section. So I'm wondering what this patch
> series is about. Or I must have missed something.
I think it reaches back to times when gcc 2.7.* was still supported, which
does behave as described in the documentation. gcc 2.95 and newer don't
require explicit initialization anymore, so this has become a non-issue.
bye, Roman
On 2/9/07, Andrew Morton <[email protected]> wrote:
> If we really do have a problem here it'd be better to fix it in some
> central and global fashion: either by ensuring that each architecture's
> startup code will zero this memory or by some compiler/linker option such
> as -fno-common.
Great,
But what about the variables that are not in global scope?
As I understand from init.h description:
"Don't forget to initialize data not at file scope, i.e. within a function,
as gcc otherwise puts the data into the bss section and not into the init
section."
Best Regards,
Alon Bar-Lev.
On Fri, 9 Feb 2007 18:37:34 +0100 (CET)
Roman Zippel <[email protected]> wrote:
> Hi,
>
> On Fri, 9 Feb 2007, Heiko Carstens wrote:
>
> > And indeed all the __initdata annotated local and global variables on
> > s390 are in the init.data section. So I'm wondering what this patch
> > series is about. Or I must have missed something.
>
> I think it reaches back to times when gcc 2.7.* was still supported, which
> does behave as described in the documentation. gcc 2.95 and newer don't
> require explicit initialization anymore, so this has become a non-issue.
>
Yes, nobody's been observing any problems arising from this, and if this
memory was really uninitialised, people would be hitting problems.
I don't want to have to require that all __attribute__((section)) storage
be initialised - people will surely forget to do it and things will slip
through.
If we really do have a problem here it'd be better to fix it in some
central and global fashion: either by ensuring that each architecture's
startup code will zero this memory or by some compiler/linker option such
as -fno-common.
On Fri, 9 Feb 2007 23:48:36 +0200
"Alon Bar-Lev" <[email protected]> wrote:
> On 2/9/07, Andrew Morton <[email protected]> wrote:
> > If we really do have a problem here it'd be better to fix it in some
> > central and global fashion: either by ensuring that each architecture's
> > startup code will zero this memory or by some compiler/linker option such
> > as -fno-common.
>
> Great,
> But what about the variables that are not in global scope?
> As I understand from init.h description:
> "Don't forget to initialize data not at file scope, i.e. within a function,
> as gcc otherwise puts the data into the bss section and not into the init
> section."
>
It could be that this is referring to a toolchain which we don't use any
more. That comment has been there for at least seven years.
This:
--- a/fs/open.c~a
+++ a/fs/open.c
@@ -223,6 +223,8 @@ static long do_sys_truncate(const char _
struct inode * inode;
int error;
+ static char blobwozzle[100] __initdata;
+
error = -EINVAL;
if (length < 0) /* sorry, but loff_t says... */
goto out;
_
puts the array in .init.data on my fairly old toolchain.