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

[Sam Ravnborg <sam@ravnborg.org>]

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,