more x86/apic changes for v2.6.40

GIT PULL Ingo Molnar 3 participants 11 messages May 23, 2011 – May 24, 2011

Subsystems (5)

x86 ACPI User-mode Linux (UML) ptrace support +1 more subsystems

lore

Share & tools

lore

Ingo Molnar

author

2011-05-23 19:31 UTC #0 lore raw

Linus,

Please pull the latest x86-apic-for-linus git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-apic-for-linus

These arrived early in the merge window so i delayed them - but they are looking good and it would be nice to have them in v2.6.40.

Thanks,

Ingo

------------------>
Cyrill Gorcunov (3):
x86, x2apic: Track the x2apic cluster sibling map x86, x2apic: Minimize IPI register writes using cluster groups x86, x2apic: Move the common bits to x2apic.h

Daniel J Blueman (1):
x86, ioapic: Fix potential resume deadlock

Randy Dunlap (1):
x86, apic: Include module.h header in apic_flat_64.c

Suresh Siddha (14):
x86, ioapic: Allocate ioapic_saved_data early
x86, ioapic: Use ioapic_saved_data while enabling intr-remapping x86, ioapic: Remove duplicate code for saving/restoring RTEs x86, ioapic: Add struct ioapic
x86, ioapic: Consolidate ioapic_saved_data[] into 'struct ioapic' x86, ioapic: Consolidate mp_ioapics[] into 'struct ioapic' x86, ioapic: Consolidate gsi routing info into 'struct ioapic' x86, ioapic: Consolidate mp_ioapic_routing[] into 'struct ioapic' x86, apic: Use probe routines to simplify apic selection
x86, x2apic: Remove duplicate code for IPI mask routines
x86, apic: Introduce .apicdrivers section to find the list of apic drivers x86, apic: Use .apicdrivers section for the apic drivers list x86, apic: Clean up bigsmp apic selection code
x86, apic: Make apic drivers static

 arch/x86/include/asm/apic.h           |   29 +++-
 arch/x86/include/asm/io_apic.h        |   28 +--
 arch/x86/include/asm/x2apic.h         |   62 +++++++
 arch/x86/kernel/acpi/boot.c           |    8 +-
 arch/x86/kernel/apic/Makefile         |   17 ++-
 arch/x86/kernel/apic/apic.c           |   48 ++----
 arch/x86/kernel/apic/apic_flat_64.c   |   26 +++-
 arch/x86/kernel/apic/bigsmp_32.c      |   12 ++-
 arch/x86/kernel/apic/es7000_32.c      |   10 +-
 arch/x86/kernel/apic/io_apic.c        |  309 ++++++++++++++-------------------
 arch/x86/kernel/apic/numaq_32.c       |    6 +-
 arch/x86/kernel/apic/probe_32.c       |  117 +++++--------
 arch/x86/kernel/apic/probe_64.c       |   61 ++-----
 arch/x86/kernel/apic/summit_32.c      |    4 +-
 arch/x86/kernel/apic/x2apic_cluster.c |  222 +++++++++++++-----------
 arch/x86/kernel/apic/x2apic_phys.c    |  115 +++---------
 arch/x86/kernel/apic/x2apic_uv_x.c    |   13 ++-
 arch/x86/kernel/devicetree.c          |    6 +-
 arch/x86/kernel/mpparse.c             |    2 +-
 arch/x86/kernel/vmlinux.lds.S         |    7 +
 20 files changed, 539 insertions(+), 563 deletions(-)
 create mode 100644 arch/x86/include/asm/x2apic.h

arch/x86/include/asm/apic.h +21 -8

index a0c46f0..4a0b7c7 100644
@@ -381,6 +381,26 @@ struct apic {
 extern struct apic *apic;
 
 /*
+ * APIC drivers are probed based on how they are listed in the .apicdrivers
+ * section. So the order is important and enforced by the ordering
+ * of different apic driver files in the Makefile.
+ *
+ * For the files having two apic drivers, we use apic_drivers()
+ * to enforce the order with in them.
+ */
+#define apic_driver(sym)					\
+	static struct apic *__apicdrivers_##sym __used		\
+	__aligned(sizeof(struct apic *))			\
+	__section(.apicdrivers) = { &sym }
+
+#define apic_drivers(sym1, sym2)					\
+	static struct apic *__apicdrivers_##sym1##sym2[2] __used	\
+	__aligned(sizeof(struct apic *))				\
+	__section(.apicdrivers) = { &sym1, &sym2 }
+
+extern struct apic *__apicdrivers[], *__apicdrivers_end[];
+
+/*
  * APIC functionality to boot other CPUs - only used on SMP:
  */
 #ifdef CONFIG_SMP

@@ -458,15 +478,10 @@ static inline unsigned default_get_apic_id(unsigned long x)
 #define DEFAULT_TRAMPOLINE_PHYS_HIGH		0x469
 
 #ifdef CONFIG_X86_64
-extern struct apic apic_flat;
-extern struct apic apic_physflat;
-extern struct apic apic_x2apic_cluster;
-extern struct apic apic_x2apic_phys;
 extern int default_acpi_madt_oem_check(char *, char *);
 
 extern void apic_send_IPI_self(int vector);
 
-extern struct apic apic_x2apic_uv_x;
 DECLARE_PER_CPU(int, x2apic_extra_bits);
 
 extern int default_cpu_present_to_apicid(int mps_cpu);

@@ -480,7 +495,7 @@ static inline void default_wait_for_init_deassert(atomic_t *deassert)
 	return;
 }
 
-extern void generic_bigsmp_probe(void);
+extern struct apic *generic_bigsmp_probe(void);
 
 
 #ifdef CONFIG_X86_LOCAL_APIC

@@ -516,8 +531,6 @@ extern struct apic apic_noop;
 
 #ifdef CONFIG_X86_32
 
-extern struct apic apic_default;
-
 static inline int noop_x86_32_early_logical_apicid(int cpu)
 {
 	return BAD_APICID;

arch/x86/include/asm/io_apic.h +10 ~~-18~~

index a97a240..690d1cc 100644
@@ -105,12 +105,12 @@ struct IR_IO_APIC_route_entry {
  * # of IO-APICs and # of IRQ routing registers
  */
 extern int nr_ioapics;
-extern int nr_ioapic_registers[MAX_IO_APICS];
 
-#define MP_MAX_IOAPIC_PIN 127
+extern int mpc_ioapic_id(int ioapic);
+extern unsigned int mpc_ioapic_addr(int ioapic);
+extern struct mp_ioapic_gsi *mp_ioapic_gsi_routing(int ioapic);
 
-/* I/O APIC entries */
-extern struct mpc_ioapic mp_ioapics[MAX_IO_APICS];
+#define MP_MAX_IOAPIC_PIN 127
 
 /* # of MP IRQ source entries */
 extern int mp_irq_entries;

@@ -152,11 +152,9 @@ extern void ioapic_insert_resources(void);
 
 int io_apic_setup_irq_pin_once(unsigned int irq, int node, struct io_apic_irq_attr *attr);
 
-extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
-extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
-extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
+extern int save_ioapic_entries(void);
+extern void mask_ioapic_entries(void);
+extern int restore_ioapic_entries(void);
 
 extern int get_nr_irqs_gsi(void);

@@ -192,19 +190,13 @@ struct io_apic_irq_attr;
 static inline int io_apic_set_pci_routing(struct device *dev, int irq,
 		 struct io_apic_irq_attr *irq_attr) { return 0; }
 
-static inline struct IO_APIC_route_entry **alloc_ioapic_entries(void)
-{
-	return NULL;
-}
-
-static inline void free_ioapic_entries(struct IO_APIC_route_entry **ent) { }
-static inline int save_IO_APIC_setup(struct IO_APIC_route_entry **ent)
+static inline int save_ioapic_entries(void)
 {
 	return -ENOMEM;
 }
 
-static inline void mask_IO_APIC_setup(struct IO_APIC_route_entry **ent) { }
-static inline int restore_IO_APIC_setup(struct IO_APIC_route_entry **ent)
+static inline void mask_ioapic_entries(void) { }
+static inline int restore_ioapic_entries(void)
 {
 	return -ENOMEM;
 }

arch/x86/include/asm/x2apic.h new file +62 -0

62 lines not on this page · raw message

arch/x86/kernel/acpi/boot.c +4 -4

index 9a966c5..4558f0d 100644
@@ -970,7 +970,7 @@ void __init mp_override_legacy_irq(u8 bus_irq, u8 polarity, u8 trigger, u32 gsi)
 	mp_irq.irqflag = (trigger << 2) | polarity;
 	mp_irq.srcbus = MP_ISA_BUS;
 	mp_irq.srcbusirq = bus_irq;	/* IRQ */
-	mp_irq.dstapic = mp_ioapics[ioapic].apicid; /* APIC ID */
+	mp_irq.dstapic = mpc_ioapic_id(ioapic); /* APIC ID */
 	mp_irq.dstirq = pin;	/* INTIN# */
 
 	mp_save_irq(&mp_irq);

@@ -1021,7 +1021,7 @@ void __init mp_config_acpi_legacy_irqs(void)
 		if (ioapic < 0)
 			continue;
 		pin = mp_find_ioapic_pin(ioapic, gsi);
-		dstapic = mp_ioapics[ioapic].apicid;
+		dstapic = mpc_ioapic_id(ioapic);
 
 		for (idx = 0; idx < mp_irq_entries; idx++) {
 			struct mpc_intsrc *irq = mp_irqs + idx;

@@ -1082,7 +1082,7 @@ static int mp_config_acpi_gsi(struct device *dev, u32 gsi, int trigger,
 	mp_irq.srcbus = number;
 	mp_irq.srcbusirq = (((devfn >> 3) & 0x1f) << 2) | ((pin - 1) & 3);
 	ioapic = mp_find_ioapic(gsi);
-	mp_irq.dstapic = mp_ioapics[ioapic].apicid;
+	mp_irq.dstapic = mpc_ioapic_id(ioapic);
 	mp_irq.dstirq = mp_find_ioapic_pin(ioapic, gsi);
 
 	mp_save_irq(&mp_irq);

@@ -1113,7 +1113,7 @@ int mp_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
 
 	if (ioapic_pin > MP_MAX_IOAPIC_PIN) {
 		printk(KERN_ERR "Invalid reference to IOAPIC pin "
-		       "%d-%d\n", mp_ioapics[ioapic].apicid,
+		       "%d-%d\n", mpc_ioapic_id(ioapic),
 		       ioapic_pin);
 		return gsi;
 	}

arch/x86/kernel/apic/Makefile +11 -6

index 3966b56..767fd04 100644
@@ -2,20 +2,25 @@
 # Makefile for local APIC drivers and for the IO-APIC code
 #
 
-obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o apic_noop.o probe_$(BITS).o ipi.o
+obj-$(CONFIG_X86_LOCAL_APIC)	+= apic.o apic_noop.o ipi.o
 obj-y				+= hw_nmi.o
 
 obj-$(CONFIG_X86_IO_APIC)	+= io_apic.o
 obj-$(CONFIG_SMP)		+= ipi.o
 
 ifeq ($(CONFIG_X86_64),y)
-obj-y				+= apic_flat_64.o
-obj-$(CONFIG_X86_X2APIC)	+= x2apic_cluster.o
-obj-$(CONFIG_X86_X2APIC)	+= x2apic_phys.o
+# APIC probe will depend on the listing order here
 obj-$(CONFIG_X86_UV)		+= x2apic_uv_x.o
+obj-$(CONFIG_X86_X2APIC)	+= x2apic_phys.o
+obj-$(CONFIG_X86_X2APIC)	+= x2apic_cluster.o
+obj-y				+= apic_flat_64.o
 endif
 
-obj-$(CONFIG_X86_BIGSMP)	+= bigsmp_32.o
+# APIC probe will depend on the listing order here
 obj-$(CONFIG_X86_NUMAQ)		+= numaq_32.o
-obj-$(CONFIG_X86_ES7000)	+= es7000_32.o
 obj-$(CONFIG_X86_SUMMIT)	+= summit_32.o
+obj-$(CONFIG_X86_BIGSMP)	+= bigsmp_32.o
+obj-$(CONFIG_X86_ES7000)	+= es7000_32.o
+
+# For 32bit, probe_32 need to be listed last
+obj-$(CONFIG_X86_LOCAL_APIC)	+= probe_$(BITS).o

arch/x86/kernel/apic/apic.c +13 ~~-35~~

95 lines not on this page · raw message

arch/x86/kernel/apic/apic_flat_64.c +23 -3

index 5652d31..f7a41e4 100644
@@ -16,6 +16,7 @@
 #include <linux/ctype.h>
 #include <linux/init.h>
 #include <linux/hardirq.h>
+#include <linux/module.h>
 #include <asm/smp.h>
 #include <asm/apic.h>
 #include <asm/ipi.h>

@@ -24,6 +25,12 @@
 #include <acpi/acpi_bus.h>
 #endif
 
+static struct apic apic_physflat;
+static struct apic apic_flat;
+
+struct apic __read_mostly *apic = &apic_flat;
+EXPORT_SYMBOL_GPL(apic);
+
 static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	return 1;

@@ -164,7 +171,7 @@ static int flat_phys_pkg_id(int initial_apic_id, int index_msb)
 	return initial_apic_id >> index_msb;
 }
 
-struct apic apic_flat =  {
+static struct apic apic_flat =  {
 	.name				= "flat",
 	.probe				= NULL,
 	.acpi_madt_oem_check		= flat_acpi_madt_oem_check,

@@ -312,10 +319,18 @@ physflat_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 	return per_cpu(x86_cpu_to_apicid, cpu);
 }
 
-struct apic apic_physflat =  {
+static int physflat_probe(void)
+{
+	if (apic == &apic_physflat || num_possible_cpus() > 8)
+		return 1;
+
+	return 0;
+}
+
+static struct apic apic_physflat =  {
 
 	.name				= "physical flat",
-	.probe				= NULL,
+	.probe				= physflat_probe,
 	.acpi_madt_oem_check		= physflat_acpi_madt_oem_check,
 	.apic_id_registered		= flat_apic_id_registered,

@@ -369,3 +384,8 @@ struct apic apic_physflat =  {
 	.wait_icr_idle			= native_apic_wait_icr_idle,
 	.safe_wait_icr_idle		= native_safe_apic_wait_icr_idle,
 };
+
+/*
+ * We need to check for physflat first, so this order is important.
+ */
+apic_drivers(apic_physflat, apic_flat);

arch/x86/kernel/apic/bigsmp_32.c +11 -1

index d84ac5a5..efd737e 100644
@@ -193,7 +193,7 @@ static int probe_bigsmp(void)
 	return dmi_bigsmp;
 }
 
-struct apic apic_bigsmp = {
+static struct apic apic_bigsmp = {
 
 	.name				= "bigsmp",
 	.probe				= probe_bigsmp,

@@ -254,3 +254,13 @@ struct apic apic_bigsmp = {
 
 	.x86_32_early_logical_apicid	= bigsmp_early_logical_apicid,
 };
+
+struct apic * __init generic_bigsmp_probe(void)
+{
+	if (probe_bigsmp())
+		return &apic_bigsmp;
+
+	return NULL;
+}
+
+apic_driver(apic_bigsmp);

arch/x86/kernel/apic/es7000_32.c +8 -2

index 70533de..9536b3f 100644
@@ -620,7 +620,7 @@ static int es7000_mps_oem_check_cluster(struct mpc_table *mpc, char *oem,
 }
 
 /* We've been warned by a false positive warning.Use __refdata to keep calm. */
-struct apic __refdata apic_es7000_cluster = {
+static struct apic __refdata apic_es7000_cluster = {
 
 	.name				= "es7000",
 	.probe				= probe_es7000,

@@ -685,7 +685,7 @@ struct apic __refdata apic_es7000_cluster = {
 	.x86_32_early_logical_apicid	= es7000_early_logical_apicid,
 };
 
-struct apic __refdata apic_es7000 = {
+static struct apic __refdata apic_es7000 = {
 
 	.name				= "es7000",
 	.probe				= probe_es7000,

@@ -747,3 +747,9 @@ struct apic __refdata apic_es7000 = {
 
 	.x86_32_early_logical_apicid	= es7000_early_logical_apicid,
 };
+
+/*
+ * Need to check for es7000 followed by es7000_cluster, so this order
+ * in apic_drivers is important.
+ */
+apic_drivers(apic_es7000, apic_es7000_cluster);

arch/x86/kernel/apic/io_apic.c +130 ~~-179~~

624 lines not on this page · raw message

arch/x86/kernel/apic/numaq_32.c +4 -2

index 30f1331..c4a61ca 100644
@@ -472,8 +472,8 @@ static void numaq_setup_portio_remap(void)
 		(u_long) xquad_portio, (u_long) num_quads*XQUAD_PORTIO_QUAD);
 }
 
-/* Use __refdata to keep false positive warning calm.	*/
-struct apic __refdata apic_numaq = {
+/* Use __refdata to keep false positive warning calm.  */
+static struct apic __refdata apic_numaq = {
 
 	.name				= "NUMAQ",
 	.probe				= probe_numaq,

@@ -537,3 +537,5 @@ struct apic __refdata apic_numaq = {
 	.x86_32_early_logical_apicid	= noop_x86_32_early_logical_apicid,
 	.x86_32_numa_cpu_node		= numaq_numa_cpu_node,
 };
+
+apic_driver(apic_numaq);

arch/x86/kernel/apic/probe_32.c +45 ~~-72~~

197 lines not on this page · raw message

arch/x86/kernel/apic/probe_64.c +18 ~~-43~~

84 lines not on this page · raw message

arch/x86/kernel/apic/summit_32.c +3 -1

index 35bcd7d..1911442 100644
@@ -491,7 +491,7 @@ void setup_summit(void)
 }
 #endif
 
-struct apic apic_summit = {
+static struct apic apic_summit = {
 
 	.name				= "summit",
 	.probe				= probe_summit,

@@ -552,3 +552,5 @@ struct apic apic_summit = {
 
 	.x86_32_early_logical_apicid	= summit_early_logical_apicid,
 };
+
+apic_driver(apic_summit);

arch/x86/kernel/apic/x2apic_cluster.c +124 ~~-98~~

299 lines not on this page · raw message

arch/x86/kernel/apic/x2apic_phys.c +29 ~~-86~~

179 lines not on this page · raw message

arch/x86/kernel/apic/x2apic_uv_x.c +11 -2

index 7acd2d2..f450b68 100644
@@ -58,6 +58,8 @@ unsigned int uv_apicid_hibits;
 EXPORT_SYMBOL_GPL(uv_apicid_hibits);
 static DEFINE_SPINLOCK(uv_nmi_lock);
 
+static struct apic apic_x2apic_uv_x;
+
 static unsigned long __init uv_early_read_mmr(unsigned long addr)
 {
 	unsigned long val, *mmr;

@@ -326,10 +328,15 @@ static void uv_send_IPI_self(int vector)
 	apic_write(APIC_SELF_IPI, vector);
 }
 
-struct apic __refdata apic_x2apic_uv_x = {
+static int uv_probe(void)
+{
+	return apic == &apic_x2apic_uv_x;
+}
+
+static struct apic __refdata apic_x2apic_uv_x = {
 
 	.name				= "UV large system",
-	.probe				= NULL,
+	.probe				= uv_probe,
 	.acpi_madt_oem_check		= uv_acpi_madt_oem_check,
 	.apic_id_registered		= uv_apic_id_registered,

@@ -859,3 +866,5 @@ void __init uv_system_init(void)
 	if (is_kdump_kernel())
 		reboot_type = BOOT_ACPI;
 }
+
+apic_driver(apic_x2apic_uv_x);

arch/x86/kernel/devicetree.c +4 -2

index e90f084..690bc84 100644
@@ -369,6 +369,7 @@ static struct of_ioapic_type of_ioapic_type[] =
 static int ioapic_xlate(struct irq_domain *id, const u32 *intspec, u32 intsize,
 			u32 *out_hwirq, u32 *out_type)
 {
+	struct mp_ioapic_gsi *gsi_cfg;
 	struct io_apic_irq_attr attr;
 	struct of_ioapic_type *it;
 	u32 line, idx, type;

@@ -378,7 +379,8 @@ static int ioapic_xlate(struct irq_domain *id, const u32 *intspec, u32 intsize,
 
 	line = *intspec;
 	idx = (u32) id->priv;
-	*out_hwirq = line + mp_gsi_routing[idx].gsi_base;
+	gsi_cfg = mp_ioapic_gsi_routing(idx);
+	*out_hwirq = line + gsi_cfg->gsi_base;
 
 	intspec++;
 	type = *intspec;

@@ -407,7 +409,7 @@ static void __init ioapic_add_ofnode(struct device_node *np)
 	}
 
 	for (i = 0; i < nr_ioapics; i++) {
-		if (r.start == mp_ioapics[i].apicaddr) {
+		if (r.start == mpc_ioapic_addr(i)) {
 			struct irq_domain *id;
 
 			id = kzalloc(sizeof(*id), GFP_KERNEL);

arch/x86/kernel/mpparse.c +1 -1

index 6f9bfff..9103b89 100644
@@ -285,7 +285,7 @@ static void __init construct_default_ioirq_mptable(int mpc_default_type)
 	intsrc.type = MP_INTSRC;
 	intsrc.irqflag = 0;	/* conforming */
 	intsrc.srcbus = 0;
-	intsrc.dstapic = mp_ioapics[0].apicid;
+	intsrc.dstapic = mpc_ioapic_id(0);
 
 	intsrc.irqtype = mp_INT;

arch/x86/kernel/vmlinux.lds.S +7 -0

index 624a201..49927a8 100644
@@ -306,6 +306,13 @@ SECTIONS
 	}
 
 	. = ALIGN(8);
+	.apicdrivers : AT(ADDR(.apicdrivers) - LOAD_OFFSET) {
+		__apicdrivers = .;
+		*(.apicdrivers);
+		__apicdrivers_end = .;
+	}
+
+	. = ALIGN(8);
 	/*
 	 * .exit.text is discard at runtime, not link time, to deal with
 	 *  references from .altinstructions and .eh_frame

Linus Torvalds

2011-05-24 05:59 UTC #1 lore raw

Quoted 4 lines · Ingo Molnar

Please pull the latest x86-apic-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-apic-for-linus

Hmm. Right now my bisect is going (and on that slow atom thing I'm just about to give up for the evening and continue tomorrow), but it *looks* like this pull breaks resume on my eeepc netbook.

I've bisected it down to three merges:
- sched-core-for-linus (looks unlikely)
- 'for-linus' of master.kernel.org:/home/rmk/linux-2.6-arm
- x86-apic-for-linus (I don't see anything wrong, but it seems to be
the most likely of the three)

Any idea why anything there would break resume?

Linus

Ingo Molnar

author

2011-05-24 06:03 UTC #2 lore raw

Quoted 10 lines · Linus Torvalds

Quoted 4 lines · Ingo Molnar
Please pull the latest x86-apic-for-linus git tree from:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-apic-for-linus
Hmm. Right now my bisect is going (and on that slow atom thing I'm just about to give up for the evening and continue tomorrow), but it *looks* like this pull breaks resume on my eeepc netbook.

Quoted 7 lines

I've bisected it down to three merges:
- sched-core-for-linus (looks unlikely)
- 'for-linus' of master.kernel.org:/home/rmk/linux-2.6-arm
- x86-apic-for-linus (I don't see anything wrong, but it seems to be
the most likely of the three)
Any idea why anything there would break resume?

Possibly the resume fix in the APIC merge :-/

It's these commits:

31dce14a3269: x86, ioapic: Use ioapic_saved_data while enabling intr-remapping 4c79185cdb14: x86, ioapic: Allocate ioapic_saved_data early
b64ce24daffb: x86, ioapic: Fix potential resume deadlock

The first one fixes the resume bug in an easily backportable way (although the GFP_ATOMIC is not nice), the later two do it cleaner.

So if b64ce24daffb works for you and 31dce14a3269 breaks this would signal that the fix from Suresh is the source of the Atom regression.

Thanks,

Ingo

Linus Torvalds

2011-05-24 06:09 UTC #3 lore raw

Quoted 2 lines · Ingo Molnar

Possibly the resume fix in the APIC merge :-/

Well, that's certainly in the right area..

Quoted 5 lines

It's these commits:
31dce14a3269: x86, ioapic: Use ioapic_saved_data while enabling intr-remapping 4c79185cdb14: x86, ioapic: Allocate ioapic_saved_data early
b64ce24daffb: x86, ioapic: Fix potential resume deadlock

Those are all in my bisect window, yes. But because it now jumped into that branch, all the header files changed, and it will take forever to compile on that machine (I used to do a "build image on my main machine, copy it over" when I got really fed up with bisecting multiple things, but now I'm doing it on that machine itself)

I'll get it fully bisected tomorrow, in the meantime maybe Daniel and Suresh can think about their patches and see if they come up with something.

Linus

Ingo Molnar

author

2011-05-24 06:15 UTC #4 lore raw

In reply to Ingo Molnar

Quoted 8 lines · Ingo Molnar

Possibly the resume fix in the APIC merge :-/
It's these commits:
31dce14a3269: x86, ioapic: Use ioapic_saved_data while enabling intr-remapping 4c79185cdb14: x86, ioapic: Allocate ioapic_saved_data early
b64ce24daffb: x86, ioapic: Fix potential resume deadlock

Quoted 5 lines

The first one fixes the resume bug in an easily backportable way (although the GFP_ATOMIC is not nice), the later two do it cleaner.
So if b64ce24daffb works for you and 31dce14a3269 breaks this would signal that the fix from Suresh is the source of the Atom regression.

In particular the lapic_resume() bit looks suspect. Does save_ioapic_entries() get called? It's called in enable_IR_x2apic() but your Atom is probably not x2apic. Non-x2apic system do not get save_ioapic_entries() called, so there's nothing to restore at resume time AFAICS ... Suresh?

Thanks,

Ingo

Suresh Siddha

2011-05-24 17:45 UTC #5 lore raw

Quoted 13 lines · Ingo Molnar

Quoted 11 lines · Ingo Molnar
Possibly the resume fix in the APIC merge :-/
It's these commits:
31dce14a3269: x86, ioapic: Use ioapic_saved_data while enabling intr-remapping 4c79185cdb14: x86, ioapic: Allocate ioapic_saved_data early
b64ce24daffb: x86, ioapic: Fix potential resume deadlock
The first one fixes the resume bug in an easily backportable way (although the GFP_ATOMIC is not nice), the later two do it cleaner.

Quoted 7 lines

Quoted 2 lines
So if b64ce24daffb works for you and 31dce14a3269 breaks this would signal that the fix from Suresh is the source of the Atom regression.
In particular the lapic_resume() bit looks suspect. Does save_ioapic_entries() get called? It's called in enable_IR_x2apic() but your Atom is probably not x2apic. Non-x2apic system do not get save_ioapic_entries() called, so there's nothing to restore at resume time AFAICS ... Suresh?

Patch appended(I am sorry, should have caught this earlier. Looks like my system does keep the ioapic state intact during suspend/resume.)

---
From: Suresh Siddha <[email protected]>
Subject: x86, ioapic: restore ioapic entries during resume properly

In mask/restore_ioapic_entries, we should be restoring ioapic entries when ioapics[apic].saved_registers is not NULL. Fix the typo and address the resume regression.

Reported-by Linus Torvalds <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---

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

arch/x86/kernel/apic/io_apic.c +2 -2

index 9488dcf..e529339 100644
@@ -676,7 +676,7 @@ void mask_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {

@@ -699,7 +699,7 @@ int restore_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++)

Linus Torvalds

2011-05-24 18:35 UTC #6 lore raw

Quoted 3 lines · Suresh Siddha

Patch appended(I am sorry, should have caught this earlier. Looks like my system does keep the ioapic state intact during suspend/resume.)

Looks likely. My bisect just finished as I got this email, with

57a6f74023c7fd943160d7635bbc8d9f66e2ab54 is the first bad commit commit 57a6f74023c7fd943160d7635bbc8d9f66e2ab54

Author: Suresh Siddha <[email protected]>
Date:   Wed May 18 16:31:36 2011 -0700

x86, ioapic: Consolidate ioapic_saved_data[] into 'struct ioapic'

Signed-off-by Suresh Siddha <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

:040000 040000 0ae77b7536a88d0ce4f789a7c870aa3103ccc1d5
3460a9ec2b6a017d91b8fa6465e8d7d7cd8be71d M arch

and I'm now going back to top-of-git and applying your patch to verify that it fixes it. But it looks rather likely that it does.

Linus

Ingo Molnar

author

2011-05-24 18:37 UTC #7 lore raw

Quoted 20 lines · Linus Torvalds

Quoted 3 lines · Suresh Siddha
Patch appended(I am sorry, should have caught this earlier. Looks like my system does keep the ioapic state intact during suspend/resume.)
Looks likely. My bisect just finished as I got this email, with
57a6f74023c7fd943160d7635bbc8d9f66e2ab54 is the first bad commit commit 57a6f74023c7fd943160d7635bbc8d9f66e2ab54
Author: Suresh Siddha <[email protected]>
Date:   Wed May 18 16:31:36 2011 -0700
x86, ioapic: Consolidate ioapic_saved_data[] into 'struct ioapic'
Signed-off-by Suresh Siddha <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>

Quoted 5 lines

:040000 040000 0ae77b7536a88d0ce4f789a7c870aa3103ccc1d5
3460a9ec2b6a017d91b8fa6465e8d7d7cd8be71d M arch
and I'm now going back to top-of-git and applying your patch to verify that it fixes it. But it looks rather likely that it does.

Ok, great: i also just sent the pull request with Suresh's fix included so you can pull that if the fix works fine.

Thanks,

Ingo

Ingo Molnar

author

2011-05-24 18:36 UTC #8 lore raw

[conditional GIT PULL] x86 fixes

In reply to Suresh Siddha

Quoted 17 lines · Suresh Siddha

Quoted 16 lines · Ingo Molnar
Quoted 13 lines · Ingo Molnar
Possibly the resume fix in the APIC merge :-/
It's these commits:
31dce14a3269: x86, ioapic: Use ioapic_saved_data while enabling intr-remapping 4c79185cdb14: x86, ioapic: Allocate ioapic_saved_data early
b64ce24daffb: x86, ioapic: Fix potential resume deadlock
The first one fixes the resume bug in an easily backportable way (although the GFP_ATOMIC is not nice), the later two do it cleaner.
So if b64ce24daffb works for you and 31dce14a3269 breaks this would signal that the fix from Suresh is the source of the Atom regression.

Quoted 8 lines

Quoted 5 lines
In particular the lapic_resume() bit looks suspect. Does save_ioapic_entries() get called? It's called in enable_IR_x2apic() but your Atom is probably not x2apic. Non-x2apic system do not get save_ioapic_entries() called, so there's nothing to restore at resume time AFAICS ... Suresh?
Patch appended(I am sorry, should have caught this earlier. Looks like my system does keep the ioapic state intact during suspend/resume.)

Thanks Suresh!

Linus, if the patch from Suresh works on your Atom box then please pull the latest x86 fixes tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-urgent-for-linus

I've added your Reported-and-tested-by tag on the assumption that this resolves the bug. (If it does not then we'll throw away that commit anyway)

out-of-topic modifications in x86-urgent-for-linus:
---------------------------------------------------
arch/um/Kconfig.x86 # 3a36790: um: Use RWSEM_GENERIC_SPINLOCK on
include/linux/linkage.h # 1b4ac2a: x86: Get rid of asmregparm

Thanks,

Ingo

------------------>
Richard Weinberger (2):
um: Use RWSEM_GENERIC_SPINLOCK on x86
x86: Get rid of asmregparm

Suresh Siddha (1):
x86, ioapic: Restore ioapic entries during resume properly

 arch/um/Kconfig.x86            |    4 ++--
 arch/x86/include/asm/linkage.h |    5 -----
 arch/x86/kernel/apic/io_apic.c |    4 ++--
 arch/x86/kernel/ptrace.c       |    4 ++--
 include/linux/linkage.h        |    4 ----
 5 files changed, 6 insertions(+), 15 deletions(-)

arch/um/Kconfig.x86 +2 -2

index a9da516..795ea8e 100644
@@ -29,10 +29,10 @@ config X86_64
 	def_bool 64BIT
 
 config RWSEM_XCHGADD_ALGORITHM
-	def_bool X86_XADD
+	def_bool X86_XADD && 64BIT
 
 config RWSEM_GENERIC_SPINLOCK
-	def_bool !X86_XADD
+	def_bool !RWSEM_XCHGADD_ALGORITHM
 
 config 3_LEVEL_PGTABLES
 	bool "Three-level pagetables (EXPERIMENTAL)" if !64BIT

arch/x86/include/asm/linkage.h +0 -5

index 12d55e7..4814297 100644
@@ -8,11 +8,6 @@
 
 #ifdef CONFIG_X86_32
 #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
-/*
- * For 32-bit UML - mark functions implemented in assembly that use
- * regparm input parameters:
- */
-#define asmregparm __attribute__((regparm(3)))
 
 /*
  * Make sure the compiler doesn't do anything stupid with the

arch/x86/kernel/apic/io_apic.c +2 -2

index 9488dcf..e529339 100644
@@ -676,7 +676,7 @@ void mask_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {

@@ -699,7 +699,7 @@ int restore_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++)

arch/x86/kernel/ptrace.c +2 -2

index f65e5b5..807c2a2 100644
@@ -1363,7 +1363,7 @@ void send_sigtrap(struct task_struct *tsk, struct pt_regs *regs,
  * We must return the syscall number to actually look up in the table.
  * This can be -1L to skip running any syscall at all.
  */
-asmregparm long syscall_trace_enter(struct pt_regs *regs)
+long syscall_trace_enter(struct pt_regs *regs)
 {
 	long ret = 0;

@@ -1408,7 +1408,7 @@ asmregparm long syscall_trace_enter(struct pt_regs *regs)
 	return ret ?: regs->orig_ax;
 }
 
-asmregparm void syscall_trace_leave(struct pt_regs *regs)
+void syscall_trace_leave(struct pt_regs *regs)
 {
 	bool step;

include/linux/linkage.h +0 -4

index 7135ebc..3f46aed 100644
@@ -14,10 +14,6 @@
 #define asmlinkage CPP_ASMLINKAGE
 #endif
 
-#ifndef asmregparm
-# define asmregparm
-#endif
-
 #define __page_aligned_data	__section(.data..page_aligned) __aligned(PAGE_SIZE)
 #define __page_aligned_bss	__section(.bss..page_aligned) __aligned(PAGE_SIZE)

Linus Torvalds

2011-05-24 19:18 UTC #9 lore raw

Re: [conditional GIT PULL] x86 fixes

Quoted 3 lines · Ingo Molnar

Linus, if the patch from Suresh works on your Atom box then please pull the latest x86 fixes tree from:

Yup, confirmed. Resume works again.

Linus

tip-bot for Suresh Siddha

2011-05-24 19:25 UTC #10 lore raw

[tip:x86/urgent] x86, ioapic: Restore ioapic entries during resume properly

In reply to Suresh Siddha

Commit-ID:  2f344d2e51888785be551a756d8a8aa58269aaa8
Gitweb:     http://git.kernel.org/tip/2f344d2e51888785be551a756d8a8aa58269aaa8
Author:     Suresh Siddha <[email protected]>
AuthorDate: Tue, 24 May 2011 10:45:31 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 24 May 2011 20:32:41 +0200

x86, ioapic: Restore ioapic entries during resume properly

In mask/restore_ioapic_entries() we should be restoring ioapic entries when ioapics[apic].saved_registers is not NULL.

Fix the typo and address the resume hang regression reported by Linus.

This was not found sooner because the systems where these changes were tested on kept the IO-APIC entries intact over resume.

Reported-and-tested-by Linus Torvalds <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

Cc Daniel J Blueman <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

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

arch/x86/kernel/apic/io_apic.c +2 -2

index 9488dcf..e529339 100644
@@ -676,7 +676,7 @@ void mask_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {

@@ -699,7 +699,7 @@ int restore_ioapic_entries(void)
 	int apic, pin;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (ioapics[apic].saved_registers)
+		if (!ioapics[apic].saved_registers)
 			continue;
 
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++)