[PATCH 00/34] __initdata cleanup

[PATCH 00/34] __initdata cleanup

[Alon Bar-Lev]

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.

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
Signed-off-by: Bernhard Walle <bwalle@suse.de>

---

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 27/34] __initdata cleanup - mm

[Alon Bar-Lev]

Trivial.

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
Signed-off-by: Bernhard Walle <bwalle@suse.de>

---

diff -urNp linux-2.6.20-rc6-mm3.org/mm/page_alloc.c linux-2.6.20-rc6-mm3/mm/page_alloc.c
--- linux-2.6.20-rc6-mm3.org/mm/page_alloc.c	2007-01-31 22:15:42.000000000 +0200
+++ linux-2.6.20-rc6-mm3/mm/page_alloc.c	2007-01-31 22:19:30.000000000 +0200
@@ -101,9 +101,9 @@ static char * const zone_names[MAX_NR_ZO
 
 int min_free_kbytes = 1024;
 
-unsigned long __meminitdata nr_kernel_pages;
-unsigned long __meminitdata nr_all_pages;
-static unsigned long __initdata dma_reserve;
+unsigned long __meminitdata nr_kernel_pages = 0l;
+unsigned long __meminitdata nr_all_pages = 0l;
+static unsigned long __initdata dma_reserve = 0l;
 
 #ifdef CONFIG_ARCH_POPULATES_NODE_MAP
   /*
@@ -126,13 +126,13 @@ static unsigned long __initdata dma_rese
     #endif
   #endif
 
-  struct node_active_region __initdata early_node_map[MAX_ACTIVE_REGIONS];
-  int __initdata nr_nodemap_entries;
-  unsigned long __initdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES];
-  unsigned long __initdata arch_zone_highest_possible_pfn[MAX_NR_ZONES];
+  struct node_active_region __initdata early_node_map[MAX_ACTIVE_REGIONS] = {{0}};
+  int __initdata nr_nodemap_entries = 0;
+  unsigned long __initdata arch_zone_lowest_possible_pfn[MAX_NR_ZONES] = {0};
+  unsigned long __initdata arch_zone_highest_possible_pfn[MAX_NR_ZONES] = {0};
 #ifdef CONFIG_MEMORY_HOTPLUG_RESERVE
-  unsigned long __initdata node_boundary_start_pfn[MAX_NUMNODES];
-  unsigned long __initdata node_boundary_end_pfn[MAX_NUMNODES];
+  unsigned long __initdata node_boundary_start_pfn[MAX_NUMNODES] = {0};
+  unsigned long __initdata node_boundary_end_pfn[MAX_NUMNODES] = {0};
 #endif /* CONFIG_MEMORY_HOTPLUG_RESERVE */
 #endif /* CONFIG_ARCH_POPULATES_NODE_MAP */
 
@@ -1776,7 +1776,7 @@ static int __meminit build_zonelists_nod
 
 #ifdef CONFIG_NUMA
 #define MAX_NODE_LOAD (num_online_nodes())
-static int __meminitdata node_load[MAX_NUMNODES];
+static int __meminitdata node_load[MAX_NUMNODES] = {0};
 /**
  * find_next_best_node - find the next node that should appear in a given node's fallback list
  * @node: node whose fallback list we're appending
diff -urNp linux-2.6.20-rc6-mm3.org/mm/slab.c linux-2.6.20-rc6-mm3/mm/slab.c
--- linux-2.6.20-rc6-mm3.org/mm/slab.c	2007-01-31 22:15:42.000000000 +0200
+++ linux-2.6.20-rc6-mm3/mm/slab.c	2007-01-31 22:19:30.000000000 +0200
@@ -305,7 +305,7 @@ struct kmem_list3 {
  * Need this for bootstrapping a per node allocator.
  */
 #define NUM_INIT_LISTS (2 * MAX_NUMNODES + 1)
-struct kmem_list3 __initdata initkmem_list3[NUM_INIT_LISTS];
+struct kmem_list3 __initdata initkmem_list3[NUM_INIT_LISTS] = {{{0}}};
 #define	CACHE_CACHE 0
 #define	SIZE_AC 1
 #define	SIZE_L3 (1 + MAX_NUMNODES)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 00/34] __initdata cleanup

[Heiko Carstens]

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.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 00/34] __initdata cleanup

[Alon Bar-Lev]

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));

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 00/34] __initdata cleanup

[Roman Zippel]

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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 00/34] __initdata cleanup

[Andrew Morton]

On Fri, 9 Feb 2007 18:37:34 +0100 (CET)
Roman Zippel <zippel@linux-m68k.org> 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.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>