[PATCH] Modpost section mismatch fix

[PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

[Sorry if duplicate, one earlier was corrupt]

Hi,
     I got section mismatches reported by modpost in latest build. It got
     reported for xen_register_pirq and xen_unplug_emulated_devices
     functions. xen_register_pirq makes reference to
     acpi_sci_override_gsi in init.data section; marking
     xen_register_pirq with __init is not feasible since calls are made
     to it from acpi_register_gsi in non-init contexts. So marking it
     __refdata based on assumption that when acpi_sci_override_gsi is
     referenced, it is in  early stages where it is alive.


--------------------------
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #2: 0001-xen-pci-Fix-modpost-warnings-regarding-section-misma.patch --]
[-- Type: text/plain, Size: 1730 bytes --]

From 7dfd47575fd695fe8ddba951515cfac01a2ab8bc Mon Sep 17 00:00:00 2001
Message-Id: <7dfd47575fd695fe8ddba951515cfac01a2ab8bc.1309641255.git.rprabhu@wnohang.net>
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Sun, 3 Jul 2011 01:48:50 +0530
Subject: [PATCH] xen/pci: Fix modpost warnings regarding section mismatch

Marking xen_register_pirq with __refdata since it is referencing
acpi_sci_override_gsi, an __initdata variable.  Removing __init from
check_platform_magic since it is called by xen_unplug_emulated_devices in
non-init contexts (It probably gets inlined because of
-finline-functions-called-once, removing __init is more to avoid mismatch being
reported).

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 arch/x86/pci/xen.c                 |    2 +-
 arch/x86/xen/platform-pci-unplug.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index fe00830..fb5eeb0 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -327,7 +327,7 @@ int __init pci_xen_hvm_init(void)
 }
 
 #ifdef CONFIG_XEN_DOM0
-static int xen_register_pirq(u32 gsi, int triggering)
+static  __refdata  int xen_register_pirq(u32 gsi, int triggering)
 {
 	int rc, pirq, irq = -1;
 	struct physdev_map_pirq map_irq;
diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
index 25c52f9..ffcf261 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
 #ifdef CONFIG_XEN_PVHVM
 static int xen_emul_unplug;
 
-static int __init check_platform_magic(void)
+static int check_platform_magic(void)
 {
 	short magic;
 	char protocol;
-- 
1.7.6


[-- Attachment #3: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

[-- Attachment #1: Type: text/plain, Size: 6959 bytes --]

* On Tue, Jul 05, 2011 at 05:32:43PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote:
>> > >>xen_register_gsi and hence, xen_register_pirq are called from
>> > >>init (with xen_setup_acpi_sci) and non-init (with
>> > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
>> > >>acpi_sci_override_gsi and is marked __init, the best way would be to
>> > >>call xen_register_gsi and xen_register_pirq with a boolean argument like
>> > >>sci_override to obviate the need to use acpi_sci_override_gsi in
>> > >>register_pirq. I will send the patch with this change if it looks good.

>> > >Hold on, let me rebase #stable/pci.cleanups and see if the issue
>> > >here disappears.
>> > Thanks, will wait until the rebase and test after that.

>> Hm, it actually looks like it wont do the trick. Why don't you send
>> a patch against 3.0-rc6 with the outlined mechanism mentioned above.
>
>Or this patch (against 3.0-rc6) might do the trick:
>
>diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>index fe00830..f567965 100644
>--- a/arch/x86/pci/xen.c
>+++ b/arch/x86/pci/xen.c
>@@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void)
> }
>
> #ifdef CONFIG_XEN_DOM0
>-static int xen_register_pirq(u32 gsi, int triggering)
>+static int xen_register_pirq(u32 gsi, int gsi_override, int triggering)
> {
> 	int rc, pirq, irq = -1;
> 	struct physdev_map_pirq map_irq;
> 	int shareable = 0;
> 	char *name;
>-	bool gsi_override = false;
>
> 	if (!xen_pv_domain())
> 		return -1;
>@@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering)
> 		shareable = 1;
> 		name = "ioapic-level";
> 	}
>-
> 	pirq = xen_allocate_pirq_gsi(gsi);
> 	if (pirq < 0)
> 		goto out;
>
>-	/* Before we bind the GSI to a Linux IRQ, check whether
>-	 * we need to override it with bus_irq (IRQ) value. Usually for
>-	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
>-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
>-	 * but there are oddballs where the IRQ != GSI:
>-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
>-	 * which ends up being: gsi_to_irq[9] == 20
>-	 * (which is what acpi_gsi_to_irq ends up calling when starting the
>-	 * the ACPI interpreter and keels over since IRQ 9 has not been
>-	 * setup as we had setup IRQ 20 for it).
>-	 */
>-	if (gsi == acpi_sci_override_gsi) {
>-		/* Check whether the GSI != IRQ */
>-		acpi_gsi_to_irq(gsi, &irq);
>-		if (irq != gsi)
>-			/* Bugger, we MUST have that IRQ. */
>-			gsi_override = true;
>-	}
>-	if (gsi_override)
>-		irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name);
>+	if (gsi_override >= 0)
>+		irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name);
> 	else
> 		irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
> 	if (irq < 0)
>@@ -392,7 +372,7 @@ out:
> 	return irq;
> }
>
>-static int xen_register_gsi(u32 gsi, int triggering, int polarity)
>+static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
> {
> 	int rc, irq;
> 	struct physdev_setup_gsi setup_gsi;
>@@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity)
> 	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
> 			gsi, triggering, polarity);
>
>-	irq = xen_register_pirq(gsi, triggering);
>+	irq = xen_register_pirq(gsi, gsi_override, triggering);
>
> 	setup_gsi.gsi = gsi;
> 	setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
>@@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void)
> 	int rc;
> 	int trigger, polarity;
> 	int gsi = acpi_sci_override_gsi;
>+	int irq = -1;
>+	int gsi_override = -1;
>
> 	if (!gsi)
> 		return;
>@@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void)
> 	printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
> 			"polarity=%d\n", gsi, trigger, polarity);
>
>-	gsi = xen_register_gsi(gsi, trigger, polarity);
>+	/* Before we bind the GSI to a Linux IRQ, check whether
>+	 * we need to override it with bus_irq (IRQ) value. Usually for
>+	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
>+	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
>+	 * but there are oddballs where the IRQ != GSI:
>+	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
>+	 * which ends up being: gsi_to_irq[9] == 20
>+	 * (which is what acpi_gsi_to_irq ends up calling when starting the
>+	 * the ACPI interpreter and keels over since IRQ 9 has not been
>+	 * setup as we had setup IRQ 20 for it).
>+	 */
>+	/* Check whether the GSI != IRQ */
>+	if (acpi_gsi_to_irq(gsi, &irq) == 0) {
>+		if (irq >= 0 && irq != gsi)
>+			/* Bugger, we MUST have that IRQ. */
>+			gsi_override = irq;
>+	}
>+
>+	gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
> 	printk(KERN_INFO "xen: acpi sci %d\n", gsi);
>
> 	return;
>@@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void)
> static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
> 				 int trigger, int polarity)
> {
>-	return xen_register_gsi(gsi, trigger, polarity);
>+	return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
> }
>
> static int __init pci_xen_initial_domain(void)
>@@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void)
> 		if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
> 			continue;
>
>-		xen_register_pirq(irq,
>+		xen_register_pirq(irq, -1 /* no GSI override */,
> 			trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE);
> 	}
> }
>
Tested it. Works now.
Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net>

Also,
The condition for acpi_gsi_to_irq can be removed since it always returns zero.
============================================
  diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
  index f567965..3faa55b 100644
  --- a/arch/x86/pci/xen.c
  +++ b/arch/x86/pci/xen.c
  @@ -405,7 +405,7 @@ static __init void xen_setup_acpi_sci(void)
          int rc;
          int trigger, polarity;
          int gsi = acpi_sci_override_gsi;
  -       int irq = -1;
  +       unsigned int irq = 0u;
          int gsi_override = -1;

          if (!gsi)
  @@ -435,11 +435,10 @@ static __init void xen_setup_acpi_sci(void)
           * setup as we had setup IRQ 20 for it).
           */
          /* Check whether the GSI != IRQ */
  -       if (acpi_gsi_to_irq(gsi, &irq) == 0) {
  -               if (irq >= 0 && irq != gsi)
  -                       /* Bugger, we MUST have that IRQ. */
  -                       gsi_override = irq;
  -       }
  +       acpi_gsi_to_irq(gsi, &irq);
  +       if (irq != gsi)
  +               /* Bugger, we MUST have that IRQ. */
  +               gsi_override = irq;

          gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
          printk(KERN_INFO "xen: acpi sci %d\n", gsi);


--------------------------
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

[-- Attachment #1.1: Type: text/plain, Size: 6959 bytes --]

* On Tue, Jul 05, 2011 at 05:32:43PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote:
>> > >>xen_register_gsi and hence, xen_register_pirq are called from
>> > >>init (with xen_setup_acpi_sci) and non-init (with
>> > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
>> > >>acpi_sci_override_gsi and is marked __init, the best way would be to
>> > >>call xen_register_gsi and xen_register_pirq with a boolean argument like
>> > >>sci_override to obviate the need to use acpi_sci_override_gsi in
>> > >>register_pirq. I will send the patch with this change if it looks good.

>> > >Hold on, let me rebase #stable/pci.cleanups and see if the issue
>> > >here disappears.
>> > Thanks, will wait until the rebase and test after that.

>> Hm, it actually looks like it wont do the trick. Why don't you send
>> a patch against 3.0-rc6 with the outlined mechanism mentioned above.
>
>Or this patch (against 3.0-rc6) might do the trick:
>
>diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>index fe00830..f567965 100644
>--- a/arch/x86/pci/xen.c
>+++ b/arch/x86/pci/xen.c
>@@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void)
> }
>
> #ifdef CONFIG_XEN_DOM0
>-static int xen_register_pirq(u32 gsi, int triggering)
>+static int xen_register_pirq(u32 gsi, int gsi_override, int triggering)
> {
> 	int rc, pirq, irq = -1;
> 	struct physdev_map_pirq map_irq;
> 	int shareable = 0;
> 	char *name;
>-	bool gsi_override = false;
>
> 	if (!xen_pv_domain())
> 		return -1;
>@@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering)
> 		shareable = 1;
> 		name = "ioapic-level";
> 	}
>-
> 	pirq = xen_allocate_pirq_gsi(gsi);
> 	if (pirq < 0)
> 		goto out;
>
>-	/* Before we bind the GSI to a Linux IRQ, check whether
>-	 * we need to override it with bus_irq (IRQ) value. Usually for
>-	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
>-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
>-	 * but there are oddballs where the IRQ != GSI:
>-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
>-	 * which ends up being: gsi_to_irq[9] == 20
>-	 * (which is what acpi_gsi_to_irq ends up calling when starting the
>-	 * the ACPI interpreter and keels over since IRQ 9 has not been
>-	 * setup as we had setup IRQ 20 for it).
>-	 */
>-	if (gsi == acpi_sci_override_gsi) {
>-		/* Check whether the GSI != IRQ */
>-		acpi_gsi_to_irq(gsi, &irq);
>-		if (irq != gsi)
>-			/* Bugger, we MUST have that IRQ. */
>-			gsi_override = true;
>-	}
>-	if (gsi_override)
>-		irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name);
>+	if (gsi_override >= 0)
>+		irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name);
> 	else
> 		irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
> 	if (irq < 0)
>@@ -392,7 +372,7 @@ out:
> 	return irq;
> }
>
>-static int xen_register_gsi(u32 gsi, int triggering, int polarity)
>+static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
> {
> 	int rc, irq;
> 	struct physdev_setup_gsi setup_gsi;
>@@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity)
> 	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
> 			gsi, triggering, polarity);
>
>-	irq = xen_register_pirq(gsi, triggering);
>+	irq = xen_register_pirq(gsi, gsi_override, triggering);
>
> 	setup_gsi.gsi = gsi;
> 	setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
>@@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void)
> 	int rc;
> 	int trigger, polarity;
> 	int gsi = acpi_sci_override_gsi;
>+	int irq = -1;
>+	int gsi_override = -1;
>
> 	if (!gsi)
> 		return;
>@@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void)
> 	printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
> 			"polarity=%d\n", gsi, trigger, polarity);
>
>-	gsi = xen_register_gsi(gsi, trigger, polarity);
>+	/* Before we bind the GSI to a Linux IRQ, check whether
>+	 * we need to override it with bus_irq (IRQ) value. Usually for
>+	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
>+	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
>+	 * but there are oddballs where the IRQ != GSI:
>+	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
>+	 * which ends up being: gsi_to_irq[9] == 20
>+	 * (which is what acpi_gsi_to_irq ends up calling when starting the
>+	 * the ACPI interpreter and keels over since IRQ 9 has not been
>+	 * setup as we had setup IRQ 20 for it).
>+	 */
>+	/* Check whether the GSI != IRQ */
>+	if (acpi_gsi_to_irq(gsi, &irq) == 0) {
>+		if (irq >= 0 && irq != gsi)
>+			/* Bugger, we MUST have that IRQ. */
>+			gsi_override = irq;
>+	}
>+
>+	gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
> 	printk(KERN_INFO "xen: acpi sci %d\n", gsi);
>
> 	return;
>@@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void)
> static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
> 				 int trigger, int polarity)
> {
>-	return xen_register_gsi(gsi, trigger, polarity);
>+	return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
> }
>
> static int __init pci_xen_initial_domain(void)
>@@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void)
> 		if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
> 			continue;
>
>-		xen_register_pirq(irq,
>+		xen_register_pirq(irq, -1 /* no GSI override */,
> 			trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE);
> 	}
> }
>
Tested it. Works now.
Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net>

Also,
The condition for acpi_gsi_to_irq can be removed since it always returns zero.
============================================
  diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
  index f567965..3faa55b 100644
  --- a/arch/x86/pci/xen.c
  +++ b/arch/x86/pci/xen.c
  @@ -405,7 +405,7 @@ static __init void xen_setup_acpi_sci(void)
          int rc;
          int trigger, polarity;
          int gsi = acpi_sci_override_gsi;
  -       int irq = -1;
  +       unsigned int irq = 0u;
          int gsi_override = -1;

          if (!gsi)
  @@ -435,11 +435,10 @@ static __init void xen_setup_acpi_sci(void)
           * setup as we had setup IRQ 20 for it).
           */
          /* Check whether the GSI != IRQ */
  -       if (acpi_gsi_to_irq(gsi, &irq) == 0) {
  -               if (irq >= 0 && irq != gsi)
  -                       /* Bugger, we MUST have that IRQ. */
  -                       gsi_override = irq;
  -       }
  +       acpi_gsi_to_irq(gsi, &irq);
  +       if (irq != gsi)
  +               /* Bugger, we MUST have that IRQ. */
  +               gsi_override = irq;

          gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
          printk(KERN_INFO "xen: acpi sci %d\n", gsi);


--------------------------
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 184 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] Modpost section mismatch fix

[Konrad Rzeszutek Wilk]

On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote:
> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> >>[Sorry if duplicate, one earlier was corrupt]
> 
> >>Hi,
> >>     I got section mismatches reported by modpost in latest build. It got
> >>     reported for xen_register_pirq and xen_unplug_emulated_devices
> >>     functions.
> >
> >
> >> xen_register_pirq makes reference to
> >>     acpi_sci_override_gsi in init.data section; marking
> >>     xen_register_pirq with __init is not feasible since calls are made
> >>     to it from acpi_register_gsi in non-init contexts. So marking it
> >>     __refdata based on assumption that when acpi_sci_override_gsi is
> >>     referenced, it is in  early stages where it is alive.
> >
> >I don't think this assumption holds, since xen_register_pirq can be
> >called at any time and basically unconditionally references
> >acpi_sci_override_gsi.
> 
> Yeah, that has been my guess as well, however I am not privy to the
> inner workings of Xen, so was not sure.
> >
> >If we don't want to remove the __init from acpi_sci_override_gsi then
> >perhaps xen_setup_acpi_sci needs to stash it somewhere?
> >
> >Or maybe xen_register_pirq could take an "int force_irq" which, if not
> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
> >(actually via xen_register_gsi so the param would need to be propagated
> >there) would be the only actual user?
> 
> xen_register_gsi and hence, xen_register_pirq are called from
> init (with xen_setup_acpi_sci) and non-init (with
> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> acpi_sci_override_gsi and is marked __init, the best way would be to
> call xen_register_gsi and xen_register_pirq with a boolean argument like
> sci_override to obviate the need to use acpi_sci_override_gsi in
> register_pirq. I will send the patch with this change if it looks good.

Hold on, let me rebase #stable/pci.cleanups and see if the issue
here disappears.

Re: [PATCH] Modpost section mismatch fix

[Konrad Rzeszutek Wilk]

On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote:
> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> >>[Sorry if duplicate, one earlier was corrupt]
> 
> >>Hi,
> >>     I got section mismatches reported by modpost in latest build. It got
> >>     reported for xen_register_pirq and xen_unplug_emulated_devices
> >>     functions.
> >
> >
> >> xen_register_pirq makes reference to
> >>     acpi_sci_override_gsi in init.data section; marking
> >>     xen_register_pirq with __init is not feasible since calls are made
> >>     to it from acpi_register_gsi in non-init contexts. So marking it
> >>     __refdata based on assumption that when acpi_sci_override_gsi is
> >>     referenced, it is in  early stages where it is alive.
> >
> >I don't think this assumption holds, since xen_register_pirq can be
> >called at any time and basically unconditionally references
> >acpi_sci_override_gsi.
> 
> Yeah, that has been my guess as well, however I am not privy to the
> inner workings of Xen, so was not sure.
> >
> >If we don't want to remove the __init from acpi_sci_override_gsi then
> >perhaps xen_setup_acpi_sci needs to stash it somewhere?
> >
> >Or maybe xen_register_pirq could take an "int force_irq" which, if not
> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
> >(actually via xen_register_gsi so the param would need to be propagated
> >there) would be the only actual user?
> 
> xen_register_gsi and hence, xen_register_pirq are called from
> init (with xen_setup_acpi_sci) and non-init (with
> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> acpi_sci_override_gsi and is marked __init, the best way would be to
> call xen_register_gsi and xen_register_pirq with a boolean argument like
> sci_override to obviate the need to use acpi_sci_override_gsi in
> register_pirq. I will send the patch with this change if it looks good.

Hold on, let me rebase #stable/pci.cleanups and see if the issue
here disappears.

Re: [PATCH] Modpost section mismatch fix

[Konrad Rzeszutek Wilk]

On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote:
> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> >>[Sorry if duplicate, one earlier was corrupt]
> 
> >>Hi,
> >>     I got section mismatches reported by modpost in latest build. It got
> >>     reported for xen_register_pirq and xen_unplug_emulated_devices
> >>     functions.
> >
> >
> >> xen_register_pirq makes reference to
> >>     acpi_sci_override_gsi in init.data section; marking
> >>     xen_register_pirq with __init is not feasible since calls are made
> >>     to it from acpi_register_gsi in non-init contexts. So marking it
> >>     __refdata based on assumption that when acpi_sci_override_gsi is
> >>     referenced, it is in  early stages where it is alive.
> >
> >I don't think this assumption holds, since xen_register_pirq can be
> >called at any time and basically unconditionally references
> >acpi_sci_override_gsi.
> 
> Yeah, that has been my guess as well, however I am not privy to the
> inner workings of Xen, so was not sure.
> >
> >If we don't want to remove the __init from acpi_sci_override_gsi then
> >perhaps xen_setup_acpi_sci needs to stash it somewhere?
> >
> >Or maybe xen_register_pirq could take an "int force_irq" which, if not
> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
> >(actually via xen_register_gsi so the param would need to be propagated
> >there) would be the only actual user?
> 
> xen_register_gsi and hence, xen_register_pirq are called from
> init (with xen_setup_acpi_sci) and non-init (with
> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> acpi_sci_override_gsi and is marked __init, the best way would be to
> call xen_register_gsi and xen_register_pirq with a boolean argument like
> sci_override to obviate the need to use acpi_sci_override_gsi in
> register_pirq. I will send the patch with this change if it looks good.

Hold on, let me rebase #stable/pci.cleanups and see if the issue
here disappears.

Re: [PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

* On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
>On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
>> [Sorry if duplicate, one earlier was corrupt]

>> Hi,
>>      I got section mismatches reported by modpost in latest build. It got
>>      reported for xen_register_pirq and xen_unplug_emulated_devices
>>      functions.
>
>
>>  xen_register_pirq makes reference to
>>      acpi_sci_override_gsi in init.data section; marking
>>      xen_register_pirq with __init is not feasible since calls are made
>>      to it from acpi_register_gsi in non-init contexts. So marking it
>>      __refdata based on assumption that when acpi_sci_override_gsi is
>>      referenced, it is in  early stages where it is alive.
>
>I don't think this assumption holds, since xen_register_pirq can be
>called at any time and basically unconditionally references
>acpi_sci_override_gsi.

Yeah, that has been my guess as well, however I am not privy to the
inner workings of Xen, so was not sure.
>
>If we don't want to remove the __init from acpi_sci_override_gsi then
>perhaps xen_setup_acpi_sci needs to stash it somewhere?
>
>Or maybe xen_register_pirq could take an "int force_irq" which, if not
>-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
>(actually via xen_register_gsi so the param would need to be propagated
>there) would be the only actual user?

xen_register_gsi and hence, xen_register_pirq are called from
init (with xen_setup_acpi_sci) and non-init (with
acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
acpi_sci_override_gsi and is marked __init, the best way would be to
call xen_register_gsi and xen_register_pirq with a boolean argument like
sci_override to obviate the need to use acpi_sci_override_gsi in
register_pirq. I will send the patch with this change if it looks good.


>
>The xen_unplug_emulated_devices change looks correct to me since
>xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.
>
>Ian.
>


>> --------------------------
>> Raghavendra Prabhu
>> GPG Id : 0xD72BE977
>> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
>> www: wnohang.net
>> _______________________________________________
>> Virtualization mailing list
>> Virtualization@lists.linux-foundation.org
>> https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

* On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
>On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
>> [Sorry if duplicate, one earlier was corrupt]

>> Hi,
>>      I got section mismatches reported by modpost in latest build. It got
>>      reported for xen_register_pirq and xen_unplug_emulated_devices
>>      functions.
>
>
>>  xen_register_pirq makes reference to
>>      acpi_sci_override_gsi in init.data section; marking
>>      xen_register_pirq with __init is not feasible since calls are made
>>      to it from acpi_register_gsi in non-init contexts. So marking it
>>      __refdata based on assumption that when acpi_sci_override_gsi is
>>      referenced, it is in  early stages where it is alive.
>
>I don't think this assumption holds, since xen_register_pirq can be
>called at any time and basically unconditionally references
>acpi_sci_override_gsi.

Yeah, that has been my guess as well, however I am not privy to the
inner workings of Xen, so was not sure.
>
>If we don't want to remove the __init from acpi_sci_override_gsi then
>perhaps xen_setup_acpi_sci needs to stash it somewhere?
>
>Or maybe xen_register_pirq could take an "int force_irq" which, if not
>-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
>(actually via xen_register_gsi so the param would need to be propagated
>there) would be the only actual user?

xen_register_gsi and hence, xen_register_pirq are called from
init (with xen_setup_acpi_sci) and non-init (with
acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
acpi_sci_override_gsi and is marked __init, the best way would be to
call xen_register_gsi and xen_register_pirq with a boolean argument like
sci_override to obviate the need to use acpi_sci_override_gsi in
register_pirq. I will send the patch with this change if it looks good.


>
>The xen_unplug_emulated_devices change looks correct to me since
>xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.
>
>Ian.
>


>> --------------------------
>> Raghavendra Prabhu
>> GPG Id : 0xD72BE977
>> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
>> www: wnohang.net
>> _______________________________________________
>> Virtualization mailing list
>> Virtualization@lists.linux-foundation.org
>> https://lists.linux-foundation.org/mailman/listinfo/virtualization

Re: [PATCH] Modpost section mismatch fix

[Ian Campbell]

On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> [Sorry if duplicate, one earlier was corrupt]
> 
> Hi,
>      I got section mismatches reported by modpost in latest build. It got
>      reported for xen_register_pirq and xen_unplug_emulated_devices
>      functions.


>  xen_register_pirq makes reference to
>      acpi_sci_override_gsi in init.data section; marking
>      xen_register_pirq with __init is not feasible since calls are made
>      to it from acpi_register_gsi in non-init contexts. So marking it
>      __refdata based on assumption that when acpi_sci_override_gsi is
>      referenced, it is in  early stages where it is alive.

I don't think this assumption holds, since xen_register_pirq can be
called at any time and basically unconditionally references
acpi_sci_override_gsi.

If we don't want to remove the __init from acpi_sci_override_gsi then
perhaps xen_setup_acpi_sci needs to stash it somewhere?

Or maybe xen_register_pirq could take an "int force_irq" which, if not
-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
(actually via xen_register_gsi so the param would need to be propagated
there) would be the only actual user?

The xen_unplug_emulated_devices change looks correct to me since
xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.

Ian.

> 
> 
> --------------------------
> Raghavendra Prabhu
> GPG Id : 0xD72BE977
> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
> www: wnohang.net
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization

-- 
Ian Campbell
Current Noise: Crowbar - Remember Tomorrow (A Tribute To Iron Maiden)

SANTA CLAUS comes down a FIRE ESCAPE wearing bright blue LEG WARMERS
... He scrubs the POPE with a mild soap or detergent for 15 minutes,
starring JANE FONDA!!

Re: [PATCH] Modpost section mismatch fix

[Ian Campbell]

On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> [Sorry if duplicate, one earlier was corrupt]
> 
> Hi,
>      I got section mismatches reported by modpost in latest build. It got
>      reported for xen_register_pirq and xen_unplug_emulated_devices
>      functions.


>  xen_register_pirq makes reference to
>      acpi_sci_override_gsi in init.data section; marking
>      xen_register_pirq with __init is not feasible since calls are made
>      to it from acpi_register_gsi in non-init contexts. So marking it
>      __refdata based on assumption that when acpi_sci_override_gsi is
>      referenced, it is in  early stages where it is alive.

I don't think this assumption holds, since xen_register_pirq can be
called at any time and basically unconditionally references
acpi_sci_override_gsi.

If we don't want to remove the __init from acpi_sci_override_gsi then
perhaps xen_setup_acpi_sci needs to stash it somewhere?

Or maybe xen_register_pirq could take an "int force_irq" which, if not
-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
(actually via xen_register_gsi so the param would need to be propagated
there) would be the only actual user?

The xen_unplug_emulated_devices change looks correct to me since
xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.

Ian.

> 
> 
> --------------------------
> Raghavendra Prabhu
> GPG Id : 0xD72BE977
> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
> www: wnohang.net
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization

-- 
Ian Campbell
Current Noise: Crowbar - Remember Tomorrow (A Tribute To Iron Maiden)

SANTA CLAUS comes down a FIRE ESCAPE wearing bright blue LEG WARMERS
... He scrubs the POPE with a mild soap or detergent for 15 minutes,
starring JANE FONDA!!

Re: [PATCH] Modpost section mismatch fix

[Ian Campbell]

On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> [Sorry if duplicate, one earlier was corrupt]
> 
> Hi,
>      I got section mismatches reported by modpost in latest build. It got
>      reported for xen_register_pirq and xen_unplug_emulated_devices
>      functions.


>  xen_register_pirq makes reference to
>      acpi_sci_override_gsi in init.data section; marking
>      xen_register_pirq with __init is not feasible since calls are made
>      to it from acpi_register_gsi in non-init contexts. So marking it
>      __refdata based on assumption that when acpi_sci_override_gsi is
>      referenced, it is in  early stages where it is alive.

I don't think this assumption holds, since xen_register_pirq can be
called at any time and basically unconditionally references
acpi_sci_override_gsi.

If we don't want to remove the __init from acpi_sci_override_gsi then
perhaps xen_setup_acpi_sci needs to stash it somewhere?

Or maybe xen_register_pirq could take an "int force_irq" which, if not
-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
(actually via xen_register_gsi so the param would need to be propagated
there) would be the only actual user?

The xen_unplug_emulated_devices change looks correct to me since
xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.

Ian.

> 
> 
> --------------------------
> Raghavendra Prabhu
> GPG Id : 0xD72BE977
> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
> www: wnohang.net
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization

-- 
Ian Campbell
Current Noise: Crowbar - Remember Tomorrow (A Tribute To Iron Maiden)

SANTA CLAUS comes down a FIRE ESCAPE wearing bright blue LEG WARMERS
... He scrubs the POPE with a mild soap or detergent for 15 minutes,
starring JANE FONDA!!

[PATCH] Modpost section mismatch fix

[Raghavendra D Prabhu]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

[Sorry if duplicate, one earlier was corrupt]

Hi,
     I got section mismatches reported by modpost in latest build. It got
     reported for xen_register_pirq and xen_unplug_emulated_devices
     functions. xen_register_pirq makes reference to
     acpi_sci_override_gsi in init.data section; marking
     xen_register_pirq with __init is not feasible since calls are made
     to it from acpi_register_gsi in non-init contexts. So marking it
     __refdata based on assumption that when acpi_sci_override_gsi is
     referenced, it is in  early stages where it is alive.


--------------------------
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net

[-- Attachment #2: 0001-xen-pci-Fix-modpost-warnings-regarding-section-misma.patch --]
[-- Type: text/plain, Size: 1731 bytes --]

>From 7dfd47575fd695fe8ddba951515cfac01a2ab8bc Mon Sep 17 00:00:00 2001
Message-Id: <7dfd47575fd695fe8ddba951515cfac01a2ab8bc.1309641255.git.rprabhu@wnohang.net>
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Sun, 3 Jul 2011 01:48:50 +0530
Subject: [PATCH] xen/pci: Fix modpost warnings regarding section mismatch

Marking xen_register_pirq with __refdata since it is referencing
acpi_sci_override_gsi, an __initdata variable.  Removing __init from
check_platform_magic since it is called by xen_unplug_emulated_devices in
non-init contexts (It probably gets inlined because of
-finline-functions-called-once, removing __init is more to avoid mismatch being
reported).

Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
 arch/x86/pci/xen.c                 |    2 +-
 arch/x86/xen/platform-pci-unplug.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index fe00830..fb5eeb0 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -327,7 +327,7 @@ int __init pci_xen_hvm_init(void)
 }
 
 #ifdef CONFIG_XEN_DOM0
-static int xen_register_pirq(u32 gsi, int triggering)
+static  __refdata  int xen_register_pirq(u32 gsi, int triggering)
 {
 	int rc, pirq, irq = -1;
 	struct physdev_map_pirq map_irq;
diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
index 25c52f9..ffcf261 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
 #ifdef CONFIG_XEN_PVHVM
 static int xen_emul_unplug;
 
-static int __init check_platform_magic(void)
+static int check_platform_magic(void)
 {
 	short magic;
 	char protocol;
-- 
1.7.6