[PATCH] x86: Fix section mismatches in apic.

[PATCH] x86: Fix section mismatches in apic.

[Rakib Mullick]

 Impact: Fix section mismatch

find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() are non
init functions, but these functions calls some init functions. But we
need these functions as non-init functions.

WARNING: vmlinux.o(.text+0x16250): Section mismatch in reference from
the function find_unisys_acpi_oem_table() to the function
.init.text:early_acpi_os_unmap_memory()
The function find_unisys_acpi_oem_table() references
the function __init early_acpi_os_unmap_memory().
This is often because find_unisys_acpi_oem_table lacks a __init
annotation or the annotation of early_acpi_os_unmap_memory is wrong.

WARNING: vmlinux.o(.text+0x16269): Section mismatch in reference from
the function find_unisys_acpi_oem_table() to the function
.init.text:early_acpi_os_unmap_memory()
The function find_unisys_acpi_oem_table() references
the function __init early_acpi_os_unmap_memory().
This is often because find_unisys_acpi_oem_table lacks a __init
annotation or the annotation of early_acpi_os_unmap_memory is wrong.

WARNING: vmlinux.o(.text+0x16279): Section mismatch in reference from
the function find_unisys_acpi_oem_table() to the function
.init.text:__acpi_map_table()
The function find_unisys_acpi_oem_table() references
the function __init __acpi_map_table().
This is often because find_unisys_acpi_oem_table lacks a __init
annotation or the annotation of __acpi_map_table is wrong.

WARNING: vmlinux.o(.text+0x16297): Section mismatch in reference from
the function unmap_unisys_acpi_oem_table() to the function
.init.text:__acpi_unmap_table()
The function unmap_unisys_acpi_oem_table() references
the function __init __acpi_unmap_table().
This is often because unmap_unisys_acpi_oem_table lacks a __init
annotation or the annotation of __acpi_unmap_table is wrong.

----
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linus/arch/x86/kernel/apic/es7000_32.c	2009-04-10 22:28:47.000000000 +0600
+++ rakib/arch/x86/kernel/apic/es7000_32.c	2009-04-14 10:19:21.000000000 +0600
@@ -254,7 +254,7 @@ static int parse_unisys_oem(char *oemptr
 }

 #ifdef CONFIG_ACPI
-static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
+static int __ref find_unisys_acpi_oem_table(unsigned long *oem_addr)
 {
 	struct acpi_table_header *header = NULL;
 	struct es7000_oem_table *table;
@@ -285,7 +285,7 @@ static int find_unisys_acpi_oem_table(un
 	return 0;
 }

-static void unmap_unisys_acpi_oem_table(unsigned long oem_addr)
+static void __ref unmap_unisys_acpi_oem_table(unsigned long oem_addr)
 {
 	if (!oem_addr)
 		return;

[tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[tip-bot for Rakib Mullick]

Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
Author:     Rakib Mullick <rakib.mullick@gmail.com>
AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 19 Apr 2009 11:28:18 +0200

x86: Fix false positive section mismatch warnings in the apic code

find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() are non
init functions, but these functions calls some init functions. But we
need these functions as non-init functions.

So annotate them via __ref.

The following false positive warnings are fixed via this:

  WARNING: vmlinux.o(.text+0x16250): Section mismatch in reference from
  the function find_unisys_acpi_oem_table() to the function
  .init.text:early_acpi_os_unmap_memory()
  The function find_unisys_acpi_oem_table() references
  the function __init early_acpi_os_unmap_memory().
  This is often because find_unisys_acpi_oem_table lacks a __init
  annotation or the annotation of early_acpi_os_unmap_memory is wrong.

  WARNING: vmlinux.o(.text+0x16269): Section mismatch in reference from
  the function find_unisys_acpi_oem_table() to the function
  .init.text:early_acpi_os_unmap_memory()
  The function find_unisys_acpi_oem_table() references
  the function __init early_acpi_os_unmap_memory().
  This is often because find_unisys_acpi_oem_table lacks a __init
  annotation or the annotation of early_acpi_os_unmap_memory is wrong.

  WARNING: vmlinux.o(.text+0x16279): Section mismatch in reference from
  the function find_unisys_acpi_oem_table() to the function
  .init.text:__acpi_map_table()
  The function find_unisys_acpi_oem_table() references
  the function __init __acpi_map_table().
  This is often because find_unisys_acpi_oem_table lacks a __init
  annotation or the annotation of __acpi_map_table is wrong.

  WARNING: vmlinux.o(.text+0x16297): Section mismatch in reference from
  the function unmap_unisys_acpi_oem_table() to the function
  .init.text:__acpi_unmap_table()
  The function unmap_unisys_acpi_oem_table() references
  the function __init __acpi_unmap_table().
  This is often because unmap_unisys_acpi_oem_table lacks a __init
  annotation or the annotation of __acpi_unmap_table is wrong.

[ Impact: annotate away section mismatch warnings ]

Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
LKML-Reference: <b9df5fa10904181941u4a73fc97i80dfa90a50b8daf5@mail.gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/apic/es7000_32.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 1c11b81..96c150c 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -254,7 +254,7 @@ static int parse_unisys_oem(char *oemptr)
 }
 
 #ifdef CONFIG_ACPI
-static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
+static int __ref find_unisys_acpi_oem_table(unsigned long *oem_addr)
 {
 	struct acpi_table_header *header = NULL;
 	struct es7000_oem_table *table;
@@ -285,7 +285,7 @@ static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
 	return 0;
 }
 
-static void unmap_unisys_acpi_oem_table(unsigned long oem_addr)
+static void __ref unmap_unisys_acpi_oem_table(unsigned long oem_addr)
 {
 	if (!oem_addr)
 		return;

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Marcin Slusarz]

tip-bot for Rakib Mullick pisze:
> Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> Author:     Rakib Mullick <rakib.mullick@gmail.com>
> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Sun, 19 Apr 2009 11:28:18 +0200
> 
> x86: Fix false positive section mismatch warnings in the apic code
> 
> find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() are non
> init functions, but these functions calls some init functions. But we
> need these functions as non-init functions.

Why? This warning seems to be valid.

> 
> So annotate them via __ref.
> 
> The following false positive warnings are fixed via this:
> 
>   WARNING: vmlinux.o(.text+0x16250): Section mismatch in reference from
>   the function find_unisys_acpi_oem_table() to the function
>   .init.text:early_acpi_os_unmap_memory()
>   The function find_unisys_acpi_oem_table() references
>   the function __init early_acpi_os_unmap_memory().
>   This is often because find_unisys_acpi_oem_table lacks a __init
>   annotation or the annotation of early_acpi_os_unmap_memory is wrong.
> 
>   WARNING: vmlinux.o(.text+0x16269): Section mismatch in reference from
>   the function find_unisys_acpi_oem_table() to the function
>   .init.text:early_acpi_os_unmap_memory()
>   The function find_unisys_acpi_oem_table() references
>   the function __init early_acpi_os_unmap_memory().
>   This is often because find_unisys_acpi_oem_table lacks a __init
>   annotation or the annotation of early_acpi_os_unmap_memory is wrong.
> 
>   WARNING: vmlinux.o(.text+0x16279): Section mismatch in reference from
>   the function find_unisys_acpi_oem_table() to the function
>   .init.text:__acpi_map_table()
>   The function find_unisys_acpi_oem_table() references
>   the function __init __acpi_map_table().
>   This is often because find_unisys_acpi_oem_table lacks a __init
>   annotation or the annotation of __acpi_map_table is wrong.
> 
>   WARNING: vmlinux.o(.text+0x16297): Section mismatch in reference from
>   the function unmap_unisys_acpi_oem_table() to the function
>   .init.text:__acpi_unmap_table()
>   The function unmap_unisys_acpi_oem_table() references
>   the function __init __acpi_unmap_table().
>   This is often because unmap_unisys_acpi_oem_table lacks a __init
>   annotation or the annotation of __acpi_unmap_table is wrong.
> 
> [ Impact: annotate away section mismatch warnings ]
> 
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> LKML-Reference: <b9df5fa10904181941u4a73fc97i80dfa90a50b8daf5@mail.gmail.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  arch/x86/kernel/apic/es7000_32.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
> index 1c11b81..96c150c 100644
> --- a/arch/x86/kernel/apic/es7000_32.c
> +++ b/arch/x86/kernel/apic/es7000_32.c
> @@ -254,7 +254,7 @@ static int parse_unisys_oem(char *oemptr)
>  }
>  
>  #ifdef CONFIG_ACPI
> -static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
> +static int __ref find_unisys_acpi_oem_table(unsigned long *oem_addr)
>  {
>  	struct acpi_table_header *header = NULL;
>  	struct es7000_oem_table *table;
> @@ -285,7 +285,7 @@ static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
>  	return 0;
>  }
>  
> -static void unmap_unisys_acpi_oem_table(unsigned long oem_addr)
> +static void __ref unmap_unisys_acpi_oem_table(unsigned long oem_addr)
>  {
>  	if (!oem_addr)
>  		return;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Ingo Molnar]

* Marcin Slusarz <marcin.slusarz@gmail.com> wrote:

> tip-bot for Rakib Mullick pisze:
> > Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > Author:     Rakib Mullick <rakib.mullick@gmail.com>
> > AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Sun, 19 Apr 2009 11:28:18 +0200
> > 
> > x86: Fix false positive section mismatch warnings in the apic code
> > 
> > find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() 
> > are non init functions, but these functions calls some init 
> > functions. But we need these functions as non-init functions.
> 
> Why? This warning seems to be valid.

It's put into struct apic::acpi_madt_oem_check - which is a non-init 
structure. That particular field is only used from init context - 
but other fields are used all the time.

Sam, what's the preferred way to handle these? We could mark the 
function pointer prototype there as __initdata, but i suspect we'll 
still get the warning in that case.

	Ingo

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Marcin Slusarz]

Ingo Molnar wrote:
> * Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
> 
>> tip-bot for Rakib Mullick pisze:
>>> Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
>>> Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
>>> Author:     Rakib Mullick <rakib.mullick@gmail.com>
>>> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
>>> Committer:  Ingo Molnar <mingo@elte.hu>
>>> CommitDate: Sun, 19 Apr 2009 11:28:18 +0200
>>>
>>> x86: Fix false positive section mismatch warnings in the apic code
>>>
>>> find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() 
>>> are non init functions, but these functions calls some init 
>>> functions. But we need these functions as non-init functions.
>> Why? This warning seems to be valid.
> 
> It's put into struct apic::acpi_madt_oem_check - which is a non-init 
> structure. That particular field is only used from init context - 
> but other fields are used all the time.

Can we have a rule that "every __ref usage should have a comment explaining
why __ref is safe in this place"? 

Marcin

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Ingo Molnar]

* Marcin Slusarz <marcin.slusarz@gmail.com> wrote:

> Ingo Molnar wrote:
> > * Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
> > 
> >> tip-bot for Rakib Mullick pisze:
> >>> Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> >>> Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> >>> Author:     Rakib Mullick <rakib.mullick@gmail.com>
> >>> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> >>> Committer:  Ingo Molnar <mingo@elte.hu>
> >>> CommitDate: Sun, 19 Apr 2009 11:28:18 +0200
> >>>
> >>> x86: Fix false positive section mismatch warnings in the apic code
> >>>
> >>> find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() 
> >>> are non init functions, but these functions calls some init 
> >>> functions. But we need these functions as non-init functions.
> >> Why? This warning seems to be valid.
> > 
> > It's put into struct apic::acpi_madt_oem_check - which is a 
> > non-init structure. That particular field is only used from init 
> > context - but other fields are used all the time.
> 
> Can we have a rule that "every __ref usage should have a comment 
> explaining why __ref is safe in this place"?

Yes, that's prudent.

	Ingo

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Sam Ravnborg]

On Sun, Apr 19, 2009 at 11:57:59AM +0200, Ingo Molnar wrote:
> 
> * Marcin Slusarz <marcin.slusarz@gmail.com> wrote:
> 
> > tip-bot for Rakib Mullick pisze:
> > > Commit-ID:  aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > > Gitweb:     http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > > Author:     Rakib Mullick <rakib.mullick@gmail.com>
> > > AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > CommitDate: Sun, 19 Apr 2009 11:28:18 +0200
> > > 
> > > x86: Fix false positive section mismatch warnings in the apic code
> > > 
> > > find_unisys_acpi_oem_table() and unmap_unisys_acpi_oem_table() 
> > > are non init functions, but these functions calls some init 
> > > functions. But we need these functions as non-init functions.
> > 
> > Why? This warning seems to be valid.
> 
> It's put into struct apic::acpi_madt_oem_check - which is a non-init 
> structure. That particular field is only used from init context - 
> but other fields are used all the time.
> 
> Sam, what's the preferred way to handle these? We could mark the 
> function pointer prototype there as __initdata, but i suspect we'll 
> still get the warning in that case.

If I understand it correct we have the following:

(data) struct apic apic_es7000.acpi_madt_oem_check =>
(function) es7000_acpi_madt_oem_check =>
(function) find_unisys_acpi_oem_table
(__init) early_acpi_os_unmap_memory

So the real fix is to:
1) annotate find_unisys_acpi_oem_table __init
2) annotate es7000_acpi_madt_oem_check __init
3) teach modpost that struct apic apic_es7000 may reference __init

Step 3 is done using __refdata

Based on the above analysis I would assume the best fix
would look like this the following.
[I only looked at the first warning]

Rabik/Marcin - if you agree in the analysis could you produce
a proper patch and send to Ingo - thanks.

You can add my:
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
if the patch is ok.

	Sam


diff --git a/arch/x86/kernel/apic/es7000_32.c b/arch/x86/kernel/apic/es7000_32.c
index 1c11b81..810d5ce 100644
--- a/arch/x86/kernel/apic/es7000_32.c
+++ b/arch/x86/kernel/apic/es7000_32.c
@@ -254,7 +254,7 @@ static int parse_unisys_oem(char *oemptr)
 }
 
 #ifdef CONFIG_ACPI
-static int find_unisys_acpi_oem_table(unsigned long *oem_addr)
+static int __init find_unisys_acpi_oem_table(unsigned long *oem_addr)
 {
 	struct acpi_table_header *header = NULL;
 	struct es7000_oem_table *table;
@@ -306,7 +306,7 @@ static int es7000_check_dsdt(void)
 static int es7000_acpi_ret;
 
 /* Hook from generic ACPI tables.c */
-static int es7000_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int __init es7000_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	unsigned long oem_addr = 0;
 	int check_dsdt;
@@ -717,7 +717,11 @@ struct apic apic_es7000_cluster = {
 	.safe_wait_icr_idle		= native_safe_apic_wait_icr_idle,
 };
 
-struct apic apic_es7000 = {
+/*
+ * .acpi_madt_oem_check references an init function which is ok.
+ * Annotate with __refdata to silence section mismatch warning
+ */
+struct apic __refdata apic_es7000 = {
 
 	.name				= "es7000",
 	.probe				= probe_es7000,

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Rakib Mullick]

On 4/19/09, Sam Ravnborg <sam@ravnborg.org> wrote:
> If I understand it correct we have the following:

>  (data) struct apic apic_es7000.acpi_madt_oem_check =>
>  (function) es7000_acpi_madt_oem_check =>
>  (function) find_unisys_acpi_oem_table
>  (__init) early_acpi_os_unmap_memory
>
>  So the real fix is to:
>  1) annotate find_unisys_acpi_oem_table __init
>  2) annotate es7000_acpi_madt_oem_check __init
>  3) teach modpost that struct apic apic_es7000 may reference __init
>
>  Step 3 is done using __refdata
>
>  Based on the above analysis I would assume the best fix
>  would look like this the following.
>  [I only looked at the first warning]
>
>  Rabik/Marcin - if you agree in the analysis could you produce
>  a proper patch and send to Ingo - thanks.
>
>  You can add my:
>  Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
>  if the patch is ok.
>
>         Sam
Hi Sam, after applying your suggested three steps, we still have the
following warning:

WARNING: arch/x86/kernel/built-in.o(.text+0x36174): Section mismatch
in reference from the function unmap_unisys_acpi_oem_table() to the
function .init.text:__acpi_unmap_table()
The function unmap_unisys_acpi_oem_table() references
the function __init __acpi_unmap_table().
This is often because unmap_unisys_acpi_oem_table lacks a __init
annotation or the annotation of __acpi_unmap_table is wrong.

So, I think we've to annote unmap_unisys_acpi_oem_table with __init (
If we apply Sam's suggested steps). If anything else please notice.

Rakib.

Re: [tip:x86/urgent] x86: Fix false positive section mismatch warnings in the apic code

[Sam Ravnborg]

On Mon, Apr 20, 2009 at 10:00:24AM +0600, Rakib Mullick wrote:
> On 4/19/09, Sam Ravnborg <sam@ravnborg.org> wrote:
> > If I understand it correct we have the following:
> 
> >  (data) struct apic apic_es7000.acpi_madt_oem_check =>
> >  (function) es7000_acpi_madt_oem_check =>
> >  (function) find_unisys_acpi_oem_table
> >  (__init) early_acpi_os_unmap_memory
> >
> >  So the real fix is to:
> >  1) annotate find_unisys_acpi_oem_table __init
> >  2) annotate es7000_acpi_madt_oem_check __init
> >  3) teach modpost that struct apic apic_es7000 may reference __init
> >
> >  Step 3 is done using __refdata
> >
> >  Based on the above analysis I would assume the best fix
> >  would look like this the following.
> >  [I only looked at the first warning]
> >
> >  Rabik/Marcin - if you agree in the analysis could you produce
> >  a proper patch and send to Ingo - thanks.
> >
> >  You can add my:
> >  Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> >  if the patch is ok.
> >
> >         Sam
> Hi Sam, after applying your suggested three steps, we still have the
> following warning:
> 
> WARNING: arch/x86/kernel/built-in.o(.text+0x36174): Section mismatch
> in reference from the function unmap_unisys_acpi_oem_table() to the
> function .init.text:__acpi_unmap_table()
> The function unmap_unisys_acpi_oem_table() references
> the function __init __acpi_unmap_table().
> This is often because unmap_unisys_acpi_oem_table lacks a __init
> annotation or the annotation of __acpi_unmap_table is wrong.
> 
> So, I think we've to annote unmap_unisys_acpi_oem_table with __init (
> If we apply Sam's suggested steps). If anything else please notice.

Hi guys - I lost track of this so here come a late reply.

What we have is apic_es7000.acpi_madt_oem_check that references es7000_acpi_madt_oem_check

And es7000_acpi_madt_oem_check =>
	unmap_unisys_acpi_oem_table =>
		__acpi_unmap_table

So we may only annotate unmap_unisys_acpi_oem_table if
we know that apic_es7000.acpi_madt_oem_check is only used from __init.

Looking at probe_32.c this seems to be true - but I did not look too closely
on this.

If you can make sure apic_es7000.acpi_madt_oem_check is only used from __init
then the right fix is to annotate the relevant functions with __init.
That is es7000_acpi_madt_oem_check() and unmap_unisys_acpi_oem_table().

	Sam