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 <[email protected]>
--- 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;
Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
Author: Rakib Mullick <[email protected]>
AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Cc: Andrew Morton <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
tip-bot for Rakib Mullick pisze:
> Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> Author: Rakib Mullick <[email protected]>
> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> Committer: Ingo Molnar <[email protected]>
> 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 <[email protected]>
> Cc: Andrew Morton <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
* Marcin Slusarz <[email protected]> wrote:
> tip-bot for Rakib Mullick pisze:
> > Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > Author: Rakib Mullick <[email protected]>
> > AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> > Committer: Ingo Molnar <[email protected]>
> > 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
Ingo Molnar wrote:
> * Marcin Slusarz <[email protected]> wrote:
>
>> tip-bot for Rakib Mullick pisze:
>>> Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
>>> Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
>>> Author: Rakib Mullick <[email protected]>
>>> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
>>> Committer: Ingo Molnar <[email protected]>
>>> 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
On Sun, Apr 19, 2009 at 11:57:59AM +0200, Ingo Molnar wrote:
>
> * Marcin Slusarz <[email protected]> wrote:
>
> > tip-bot for Rakib Mullick pisze:
> > > Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > > Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> > > Author: Rakib Mullick <[email protected]>
> > > AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> > > Committer: Ingo Molnar <[email protected]>
> > > 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 <[email protected]>
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,
* Marcin Slusarz <[email protected]> wrote:
> Ingo Molnar wrote:
> > * Marcin Slusarz <[email protected]> wrote:
> >
> >> tip-bot for Rakib Mullick pisze:
> >>> Commit-ID: aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> >>> Gitweb: http://git.kernel.org/tip/aa57a15ad17d284e62fbd24cf7e0eb628b2cb3f7
> >>> Author: Rakib Mullick <[email protected]>
> >>> AuthorDate: Sun, 19 Apr 2009 08:41:17 +0600
> >>> Committer: Ingo Molnar <[email protected]>
> >>> 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
On 4/19/09, Sam Ravnborg <[email protected]> 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 <[email protected]>
> 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.