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 Fri, 9 Feb 2007 18:37:34 +0100 (CET)
Roman Zippel [off-list ref] wrote:
Hi,
On Fri, 9 Feb 2007, Heiko Carstens wrote:
quoted
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.