[PATCH] ia64: fix a few section mismatch warnings

[PATCH] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

Fix the following section mismatch warnings:

WARNING: vmlinux.o(.text+0x41902): Section mismatch: reference to .init.text:__alloc_bootmem (between 'ia64_mca_cpu_init' and 'ia64_do_tlb_purge')
WARNING: vmlinux.o(.text+0x49222): Section mismatch: reference to .init.text:__alloc_bootmem (between 'register_intr' and 'iosapic_register_intr')
WARNING: vmlinux.o(.text+0x62beb2): Section mismatch: reference to .init.text:__alloc_bootmem_node (between 'hubdev_init_node' and 'cnodeid_get_geoid')

For two of the warnings a helper function marked __init_refok was used.
For the last warning a missing __init annotation was added.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
The two remaning warnings in the defconfig build I could not fix:
WARNING: vmlinux.o(.text+0x5b0e2): Section mismatch: reference to .init.text:memmap_init_zone (between 'virtual_memmap_init' and 'memmap_init')
WARNING: vmlinux.o(.text+0x5b182): Section mismatch: reference to .init.text:memmap_init_zone (between 'memmap_init' and 'find_max_min_low_pfn')

I was not sure about the right way to do it since I did not understand
the code in mm/init.c deeply enough.

	Sam

diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 91e6dc1..92ac2b0 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -560,6 +560,11 @@ iosapic_reassign_vector (int irq)
 	}
 }
 
+static void *__init_refok alloc_rte(unsigned long size)
+{
+	return alloc_bootmem(size);
+}
+
 static struct iosapic_rte_info *iosapic_alloc_rte (void)
 {
 	int i;
@@ -567,7 +572,7 @@ static struct iosapic_rte_info *iosapic_alloc_rte (void)
 	int preallocated = 0;
 
 	if (!iosapic_kmalloc_ok && list_empty(&free_rte_list)) {
-		rte = alloc_bootmem(sizeof(struct iosapic_rte_info) *
+		rte = alloc_rte(sizeof(struct iosapic_rte_info) *
 				    NR_PREALLOCATE_RTE_ENTRIES);
 		if (!rte)
 			return NULL;
diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
index 4b5daa3..e17bf97 100644
--- a/arch/ia64/kernel/mca.c
+++ b/arch/ia64/kernel/mca.c
@@ -1751,6 +1751,10 @@ format_mca_init_stack(void *mca_data, unsigned long offset,
 }
 
 /* Do per-CPU MCA-related initialization.  */
+static void * __init_refok mca_bootmem(unsigned long size)
+{
+	return alloc_bootmem(size);
+}
 
 void __cpuinit
 ia64_mca_cpu_init(void *cpu_data)
@@ -1763,7 +1767,7 @@ ia64_mca_cpu_init(void *cpu_data)
 		int cpu;
 
 		first_time = 0;
-		mca_data = alloc_bootmem(sizeof(struct ia64_mca_cpu)
+		mca_data = mca_bootmem(sizeof(struct ia64_mca_cpu)
 					 * NR_CPUS + KERNEL_STACK_SIZE);
 		mca_data = (void *)(((unsigned long)mca_data +
 					KERNEL_STACK_SIZE - 1) &
diff --git a/arch/ia64/sn/kernel/io_common.c b/arch/ia64/sn/kernel/io_common.c
index 787ed64..4594770 100644
--- a/arch/ia64/sn/kernel/io_common.c
+++ b/arch/ia64/sn/kernel/io_common.c
@@ -391,7 +391,7 @@ void sn_bus_free_sysdata(void)
  * hubdev_init_node() - Creates the HUB data structure and link them to it's
  *			own NODE specific data area.
  */
-void hubdev_init_node(nodepda_t * npda, cnodeid_t node)
+void __init hubdev_init_node(nodepda_t * npda, cnodeid_t node)
 {
 	struct hubdev_info *hubdev_info;
 	int size;

Re: [PATCH] ia64: fix a few section mismatch warnings

[Al Viro]

On Thu, Jul 26, 2007 at 11:01:41PM +0200, Sam Ravnborg wrote:
  
> +static void *__init_refok alloc_rte(unsigned long size)
> +{
> +	return alloc_bootmem(size);
> +}

That makes no sense at all.  If we ever call that after freeing initmem,
we are screwed, period.  Sounds like __init fodder.

Re: [PATCH] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

On Fri, Jul 27, 2007 at 02:18:13AM +0100, Al Viro wrote:
> On Thu, Jul 26, 2007 at 11:01:41PM +0200, Sam Ravnborg wrote:
>   
> > +static void *__init_refok alloc_rte(unsigned long size)
> > +{
> > +	return alloc_bootmem(size);
> > +}
> 
> That makes no sense at all.  If we ever call that after freeing initmem,
> we are screwed, period.  Sounds like __init fodder.

The call site has logic to prevent this from being called after init.
And the call site cannot be made __init and to limit the scope of
the __init_refok a small function is used.

So unless I mis-understood something the above should be OK.

	Sam

Re: [PATCH] ia64: fix a few section mismatch warnings

[Al Viro]

On Fri, Jul 27, 2007 at 06:27:44AM +0200, Sam Ravnborg wrote:
> On Fri, Jul 27, 2007 at 02:18:13AM +0100, Al Viro wrote:
> > On Thu, Jul 26, 2007 at 11:01:41PM +0200, Sam Ravnborg wrote:
> >   
> > > +static void *__init_refok alloc_rte(unsigned long size)
> > > +{
> > > +	return alloc_bootmem(size);
> > > +}
> > 
> > That makes no sense at all.  If we ever call that after freeing initmem,
> > we are screwed, period.  Sounds like __init fodder.
> 
> The call site has logic to prevent this from being called after init.
> And the call site cannot be made __init and to limit the scope of
> the __init_refok a small function is used.
> 
> So unless I mis-understood something the above should be OK.

Then the call site must be __init_refok, AFAICS.  The point is,
the callers of alloc_rte() are where the analysis belongs; use of
__init_refok means that you assert that all calls of __init *from*
*it* are safe.  Otherwise it just becomes "make modpost STFU and
let's hope that code is really OK".

Re: [PATCH] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

On Fri, Jul 27, 2007 at 05:36:14AM +0100, Al Viro wrote:
> On Fri, Jul 27, 2007 at 06:27:44AM +0200, Sam Ravnborg wrote:
> > On Fri, Jul 27, 2007 at 02:18:13AM +0100, Al Viro wrote:
> > > On Thu, Jul 26, 2007 at 11:01:41PM +0200, Sam Ravnborg wrote:
> > >   
> > > > +static void *__init_refok alloc_rte(unsigned long size)
> > > > +{
> > > > +	return alloc_bootmem(size);
> > > > +}
> > > 
> > > That makes no sense at all.  If we ever call that after freeing initmem,
> > > we are screwed, period.  Sounds like __init fodder.
> > 
> > The call site has logic to prevent this from being called after init.
> > And the call site cannot be made __init and to limit the scope of
> > the __init_refok a small function is used.
> > 
> > So unless I mis-understood something the above should be OK.
> 
> Then the call site must be __init_refok, AFAICS.  The point is,
> the callers of alloc_rte() are where the analysis belongs; use of
> __init_refok means that you assert that all calls of __init *from*
> *it* are safe.  Otherwise it just becomes "make modpost STFU and
> let's hope that code is really OK".

The above function is suposed to be used solely from
iosapic_alloc_rte() and not for general use.
And instead of declaring all of iosapic_alloc_rte() as __init_refok
then the single line doing the call to an __init function
was factored out and marked __init.
alloc_rte() can be used from everywhere but the naming suggest
that this is for rte only.

To make it more clear what the function was used for I could have made
it like this:

/*
 * Call site shall make sure this isused only during early init.
 * Use __init_refok to avoid warnings from modpost.
 */ 
static void * __init_refok alloc_rte(unsigned long size)
{
	return alloc_bootmen(sizeof(struct iosapic_rte_info) * size);
}

Would that make the intention more clear?

As for marking all of iosapic_alloc_rte() __init_refok this was not done
because that would then not detect any new __init usage within the function.
That could be done too obviously - it is anyway only 30 loc.

	Sam

Re: [PATCH] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

Fix the following section mismatch warnings:

WARNING: vmlinux.o(.text+0x41902): Section mismatch: reference to .init.text:__alloc_bootmem (between 'ia64_mca_cpu_init' and 'ia64_do_tlb_purge')
WARNING: vmlinux.o(.text+0x49222): Section mismatch: reference to .init.text:__alloc_bootmem (between 'register_intr' and 'iosapic_register_intr')
WARNING: vmlinux.o(.text+0x62beb2): Section mismatch: reference to .init.text:__alloc_bootmem_node (between 'hubdev_init_node' and 'cnodeid_get_geoid')

For ithe first warning a helper function marked __init_refok was used.
For the last warning a missing __init annotation was added.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
Updated - inspired by comments from Al Viro.

diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 91e6dc1..f4bd285 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -560,7 +560,7 @@ iosapic_reassign_vector (int irq)
 	}
 }
 
-static struct iosapic_rte_info *iosapic_alloc_rte (void)
+static struct iosapic_rte_info * __init_refok iosapic_alloc_rte (void)
 {
 	int i;
 	struct iosapic_rte_info *rte;
diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
index 4b5daa3..8b7681b 100644
--- a/arch/ia64/kernel/mca.c
+++ b/arch/ia64/kernel/mca.c
@@ -1751,6 +1751,10 @@ format_mca_init_stack(void *mca_data, unsigned long offset,
 }
 
 /* Do per-CPU MCA-related initialization.  */
+static void * __init_refok mca_bootmem(unsigned long size)
+{
+	return alloc_bootmem(size * sizeof(struct ia64_mca_cpu));
+}
 
 void __cpuinit
 ia64_mca_cpu_init(void *cpu_data)
@@ -1763,8 +1767,7 @@ ia64_mca_cpu_init(void *cpu_data)
 		int cpu;
 
 		first_time = 0;
-		mca_data = alloc_bootmem(sizeof(struct ia64_mca_cpu)
-					 * NR_CPUS + KERNEL_STACK_SIZE);
+		mca_data = mca_bootmem(NR_CPUS + KERNEL_STACK_SIZE);
 		mca_data = (void *)(((unsigned long)mca_data +
 					KERNEL_STACK_SIZE - 1) &
 				(-KERNEL_STACK_SIZE));
diff --git a/arch/ia64/sn/kernel/io_common.c b/arch/ia64/sn/kernel/io_common.c
index 787ed64..4594770 100644
--- a/arch/ia64/sn/kernel/io_common.c
+++ b/arch/ia64/sn/kernel/io_common.c
@@ -391,7 +391,7 @@ void sn_bus_free_sysdata(void)
  * hubdev_init_node() - Creates the HUB data structure and link them to it's
  *			own NODE specific data area.
  */
-void hubdev_init_node(nodepda_t * npda, cnodeid_t node)
+void __init hubdev_init_node(nodepda_t * npda, cnodeid_t node)
 {
 	struct hubdev_info *hubdev_info;
 	int size;

RE: [PATCH] ia64: fix a few section mismatch warnings

[Luck, Tony]

-		mca_data = alloc_bootmem(sizeof(struct ia64_mca_cpu)
-					 * NR_CPUS + KERNEL_STACK_SIZE);
+		mca_data = mca_bootmem(NR_CPUS + KERNEL_STACK_SIZE);

Oops.  You moved the multiply by sizeof(struct ia64_mca_cpu) up into
the mca_bootmem() function to make it very specific to this use. But
mutiply has higher precedence than addition.

-Tony

Re: [PATCH] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

On Fri, Jul 27, 2007 at 03:32:13PM -0700, Luck, Tony wrote:
> -		mca_data = alloc_bootmem(sizeof(struct ia64_mca_cpu)
> -					 * NR_CPUS + KERNEL_STACK_SIZE);
> +		mca_data = mca_bootmem(NR_CPUS + KERNEL_STACK_SIZE);
> 
> Oops.  You moved the multiply by sizeof(struct ia64_mca_cpu) up into
> the mca_bootmem() function to make it very specific to this use. But
> mutiply has higher precedence than addition.

Oh crap - good catch.
Shall I resubmit a corrected patch?

	Sam

RE: [PATCH] ia64: fix a few section mismatch warnings

[Luck, Tony]

> > Oops.  You moved the multiply by sizeof(struct ia64_mca_cpu) up into
> > the mca_bootmem() function to make it very specific to this use. But
> > mutiply has higher precedence than addition.
>
> Oh crap - good catch.
> Shall I resubmit a corrected patch?

Are there any other ways that we might tag the callsite to let
modpost know that this instance is safe?  Adding a call to a wrapper
function in __init_refok space feels kludgy, and whatever comments
you stick on that function, it is sitting there waiting for someone
who shouldn't to make a call.

If not ... then just make the mca_bootmem() function take
no args.  It can calculate the amount of memory, it can do
the next bit too and return a result KERNEL_STACK_SIZE aligned
too.

-Tony

[PATCH v3] ia64: fix a few section mismatch warnings

[Sam Ravnborg]

Fix the following section mismatch warnings:

WARNING: vmlinux.o(.text+0x41902): Section mismatch: reference to .init.text:__alloc_bootmem (between 'ia64_mca_cpu_init' and 'ia64_do_tlb_purge')
WARNING: vmlinux.o(.text+0x49222): Section mismatch: reference to .init.text:__alloc_bootmem (between 'register_intr' and 'iosapic_register_intr')
WARNING: vmlinux.o(.text+0x62beb2): Section mismatch: reference to .init.text:__alloc_bootmem_node (between 'hubdev_init_node' and 'cnodeid_get_geoid')

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
This should be in line with your latest comments.
Notice I introduce use of ALIGN to make it more readable.
I actually made a smal .c program just to verify that the below
expression and ALIGN agreed on how to calculate the alignment.

	Sam

diff --git a/arch/ia64/kernel/iosapic.c b/arch/ia64/kernel/iosapic.c
index 91e6dc1..f4bd285 100644
--- a/arch/ia64/kernel/iosapic.c
+++ b/arch/ia64/kernel/iosapic.c
@@ -560,7 +560,7 @@ iosapic_reassign_vector (int irq)
 	}
 }
 
-static struct iosapic_rte_info *iosapic_alloc_rte (void)
+static struct iosapic_rte_info * __init_refok iosapic_alloc_rte (void)
 {
 	int i;
 	struct iosapic_rte_info *rte;
diff --git a/arch/ia64/kernel/mca.c b/arch/ia64/kernel/mca.c
index 4b5daa3..ff28620 100644
--- a/arch/ia64/kernel/mca.c
+++ b/arch/ia64/kernel/mca.c
@@ -1750,8 +1750,17 @@ format_mca_init_stack(void *mca_data, unsigned long offset,
 	strncpy(p->comm, type, sizeof(p->comm)-1);
 }
 
-/* Do per-CPU MCA-related initialization.  */
+/* Caller prevents this from being called after init */
+static void * __init_refok mca_bootmem(void)
+{
+	void *p;
 
+	p = alloc_bootmem(sizeof(struct ia64_mca_cpu) * NR_CPUS +
+	                  KERNEL_STACK_SIZE);
+	return (void *)ALIGN((unsigned long)p, KERNEL_STACK_SIZE);
+}
+
+/* Do per-CPU MCA-related initialization.  */
 void __cpuinit
 ia64_mca_cpu_init(void *cpu_data)
 {
@@ -1763,11 +1772,7 @@ ia64_mca_cpu_init(void *cpu_data)
 		int cpu;
 
 		first_time = 0;
-		mca_data = alloc_bootmem(sizeof(struct ia64_mca_cpu)
-					 * NR_CPUS + KERNEL_STACK_SIZE);
-		mca_data = (void *)(((unsigned long)mca_data +
-					KERNEL_STACK_SIZE - 1) &
-				(-KERNEL_STACK_SIZE));
+		mca_data = mca_bootmem();
 		for (cpu = 0; cpu < NR_CPUS; cpu++) {
 			format_mca_init_stack(mca_data,
 					offsetof(struct ia64_mca_cpu, mca_stack),
diff --git a/arch/ia64/sn/kernel/io_common.c b/arch/ia64/sn/kernel/io_common.c
index 787ed64..4594770 100644
--- a/arch/ia64/sn/kernel/io_common.c
+++ b/arch/ia64/sn/kernel/io_common.c
@@ -391,7 +391,7 @@ void sn_bus_free_sysdata(void)
  * hubdev_init_node() - Creates the HUB data structure and link them to it's
  *			own NODE specific data area.
  */
-void hubdev_init_node(nodepda_t * npda, cnodeid_t node)
+void __init hubdev_init_node(nodepda_t * npda, cnodeid_t node)
 {
 	struct hubdev_info *hubdev_info;
 	int size;