All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 1/4] x86, ioapic: fix potential resume deadlock
@ 2011-05-16 18:56 Suresh Siddha
  2011-05-16 18:56 ` [patch 2/4] x86, ioapic: allocate ioapic_saved_data early Suresh Siddha
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Suresh Siddha @ 2011-05-16 18:56 UTC (permalink / raw)
  To: mingo, tglx, hpa; +Cc: linux-kernel, daniel.blueman, suresh.b.siddha, stable

[-- Attachment #1: fix_gfp_kernel_alloc_with_irqs_disabled.patch --]
[-- Type: text/plain, Size: 1409 bytes --]

From: Daniel J Blueman <daniel.blueman@gmail.com>
Subject: x86, ioapic: fix potential resume deadlock

Fix a potential deadlock when resuming; here the calling function
has disabled interrupts, so we cannot sleep.

Change the memory allocation flag from GFP_KERNEL to GFP_ATOMIC.

TODO: We can do away with this memory allocation during resume by
reusing the ioapic suspend/resume code that uses boot time allocated
buffers.

Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: stable@kernel.org	[v2.6.39]
---
 arch/x86/kernel/apic/io_apic.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -621,14 +621,14 @@ struct IO_APIC_route_entry **alloc_ioapi
 	struct IO_APIC_route_entry **ioapic_entries;
 
 	ioapic_entries = kzalloc(sizeof(*ioapic_entries) * nr_ioapics,
-				GFP_KERNEL);
+				GFP_ATOMIC);
 	if (!ioapic_entries)
 		return 0;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 		ioapic_entries[apic] =
 			kzalloc(sizeof(struct IO_APIC_route_entry) *
-				nr_ioapic_registers[apic], GFP_KERNEL);
+				nr_ioapic_registers[apic], GFP_ATOMIC);
 		if (!ioapic_entries[apic])
 			goto nomem;
 	}



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 2/4] x86, ioapic: allocate ioapic_saved_data early
  2011-05-16 18:56 [patch 1/4] x86, ioapic: fix potential resume deadlock Suresh Siddha
@ 2011-05-16 18:56 ` Suresh Siddha
  2011-05-16 18:56 ` [patch 3/4] x86, ioapic: use ioapic_saved_data while enabling intr-remapping Suresh Siddha
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Suresh Siddha @ 2011-05-16 18:56 UTC (permalink / raw)
  To: mingo, tglx, hpa; +Cc: linux-kernel, daniel.blueman, suresh.b.siddha

[-- Attachment #1: allocate_ioapic_saved_data_early.patch --]
[-- Type: text/plain, Size: 2093 bytes --]

This allows re-using this buffer for enabling interrupt-remapping during
boot and resume. And thus allow for conslidating the code between
ioapic suspend/resume and interrupt-remapping.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/kernel/apic/io_apic.c |   27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -100,6 +100,11 @@ int mp_irq_entries;
 /* GSI interrupts */
 static int nr_irqs_gsi = NR_IRQS_LEGACY;
 
+/*
+ * Saved I/O APIC state during suspend/resume.
+*/
+static struct IO_APIC_route_entry *ioapic_saved_data[MAX_IO_APICS];
+
 #if defined (CONFIG_MCA) || defined (CONFIG_EISA)
 int mp_bus_id_to_type[MAX_MP_BUSSES];
 #endif
@@ -179,6 +184,14 @@ int __init arch_early_irq_init(void)
 		io_apic_irqs = ~0UL;
 	}
 
+	for (i = 0; i < nr_ioapics; i++) {
+		ioapic_saved_data[i] =
+			kzalloc(sizeof(struct IO_APIC_route_entry) *
+				nr_ioapic_registers[i], GFP_KERNEL);
+		if (!ioapic_saved_data[i])
+			pr_err("IOAPIC %d: suspend/resume impossible!\n", i);
+	}
+
 	cfg = irq_cfgx;
 	count = ARRAY_SIZE(irq_cfgx);
 	node = cpu_to_node(0);
@@ -2918,8 +2931,6 @@ static int __init io_apic_bug_finalize(v
 
 late_initcall(io_apic_bug_finalize);
 
-static struct IO_APIC_route_entry *ioapic_saved_data[MAX_IO_APICS];
-
 static void suspend_ioapic(int ioapic_id)
 {
 	struct IO_APIC_route_entry *saved_data = ioapic_saved_data[ioapic_id];
@@ -2978,18 +2989,6 @@ static struct syscore_ops ioapic_syscore
 
 static int __init ioapic_init_ops(void)
 {
-	int i;
-
-	for (i = 0; i < nr_ioapics; i++) {
-		unsigned int size;
-
-		size = nr_ioapic_registers[i]
-			* sizeof(struct IO_APIC_route_entry);
-		ioapic_saved_data[i] = kzalloc(size, GFP_KERNEL);
-		if (!ioapic_saved_data[i])
-			pr_err("IOAPIC %d: suspend/resume impossible!\n", i);
-	}
-
 	register_syscore_ops(&ioapic_syscore_ops);
 
 	return 0;



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 3/4] x86, ioapic: use ioapic_saved_data while enabling intr-remapping
  2011-05-16 18:56 [patch 1/4] x86, ioapic: fix potential resume deadlock Suresh Siddha
  2011-05-16 18:56 ` [patch 2/4] x86, ioapic: allocate ioapic_saved_data early Suresh Siddha
@ 2011-05-16 18:56 ` Suresh Siddha
  2011-05-16 18:56 ` [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs Suresh Siddha
  2011-05-16 19:53 ` [patch 1/4] x86, ioapic: fix potential resume deadlock Daniel J Blueman
  3 siblings, 0 replies; 6+ messages in thread
From: Suresh Siddha @ 2011-05-16 18:56 UTC (permalink / raw)
  To: mingo, tglx, hpa; +Cc: linux-kernel, daniel.blueman, suresh.b.siddha

[-- Attachment #1: use_ioapic_saved_data_for_intremap.patch --]
[-- Type: text/plain, Size: 8508 bytes --]

Code flow for enabling interrupt-remapping was allocating/freeing buffers
for saving/restoring io-apic RTE's. ioapic suspend/resume code uses boot
time allocated ioapic_saved_data that is a perfect match for reuse here.

This will remove the unnecessary allocation/free of the temporary buffers
during suspend/resume of interrupt-remapping enabled platforms aswell
as paving the way for further code consolidation.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/include/asm/io_apic.h |   20 +++-------
 arch/x86/kernel/apic/apic.c    |   48 ++++++------------------
 arch/x86/kernel/apic/io_apic.c |   80 +++++++++--------------------------------
 3 files changed, 37 insertions(+), 111 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -101,7 +101,7 @@ int mp_irq_entries;
 static int nr_irqs_gsi = NR_IRQS_LEGACY;
 
 /*
- * Saved I/O APIC state during suspend/resume.
+ * Saved I/O APIC state during suspend/resume, or while enabling intr-remap.
 */
 static struct IO_APIC_route_entry *ioapic_saved_data[MAX_IO_APICS];
 
@@ -628,74 +628,43 @@ static int __init ioapic_pirq_setup(char
 __setup("pirq=", ioapic_pirq_setup);
 #endif /* CONFIG_X86_32 */
 
-struct IO_APIC_route_entry **alloc_ioapic_entries(void)
-{
-	int apic;
-	struct IO_APIC_route_entry **ioapic_entries;
-
-	ioapic_entries = kzalloc(sizeof(*ioapic_entries) * nr_ioapics,
-				GFP_ATOMIC);
-	if (!ioapic_entries)
-		return 0;
-
-	for (apic = 0; apic < nr_ioapics; apic++) {
-		ioapic_entries[apic] =
-			kzalloc(sizeof(struct IO_APIC_route_entry) *
-				nr_ioapic_registers[apic], GFP_ATOMIC);
-		if (!ioapic_entries[apic])
-			goto nomem;
-	}
-
-	return ioapic_entries;
-
-nomem:
-	while (--apic >= 0)
-		kfree(ioapic_entries[apic]);
-	kfree(ioapic_entries);
-
-	return 0;
-}
-
 /*
  * Saves all the IO-APIC RTE's
  */
-int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+int save_ioapic_entries(void)
 {
 	int apic, pin;
-
-	if (!ioapic_entries)
-		return -ENOMEM;
+	int err = 0;
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (!ioapic_entries[apic])
-			return -ENOMEM;
+		if (!ioapic_saved_data[apic]) {
+			err = -ENOMEM;
+			continue;
+		}
 
 		for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
-			ioapic_entries[apic][pin] =
+			ioapic_saved_data[apic][pin] =
 				ioapic_read_entry(apic, pin);
 	}
 
-	return 0;
+	return err;
 }
 
 /*
  * Mask all IO APIC entries.
  */
-void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+void mask_ioapic_entries(void)
 {
 	int apic, pin;
 
-	if (!ioapic_entries)
-		return;
-
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (!ioapic_entries[apic])
-			break;
+		if (!ioapic_saved_data[apic])
+			continue;
 
 		for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
 			struct IO_APIC_route_entry entry;
 
-			entry = ioapic_entries[apic][pin];
+			entry = ioapic_saved_data[apic][pin];
 			if (!entry.mask) {
 				entry.mask = 1;
 				ioapic_write_entry(apic, pin, entry);
@@ -705,36 +674,23 @@ void mask_IO_APIC_setup(struct IO_APIC_r
 }
 
 /*
- * Restore IO APIC entries which was saved in ioapic_entries.
+ * Restore IO APIC entries which was saved in ioapic_saved_data
  */
-int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
+int restore_ioapic_entries(void)
 {
 	int apic, pin;
 
-	if (!ioapic_entries)
-		return -ENOMEM;
-
 	for (apic = 0; apic < nr_ioapics; apic++) {
-		if (!ioapic_entries[apic])
-			return -ENOMEM;
+		if (!ioapic_saved_data[apic])
+			continue;
 
 		for (pin = 0; pin < nr_ioapic_registers[apic]; pin++)
 			ioapic_write_entry(apic, pin,
-					ioapic_entries[apic][pin]);
+					   ioapic_saved_data[apic][pin]);
 	}
 	return 0;
 }
 
-void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
-{
-	int apic;
-
-	for (apic = 0; apic < nr_ioapics; apic++)
-		kfree(ioapic_entries[apic]);
-
-	kfree(ioapic_entries);
-}
-
 /*
  * Find the IRQ entry number of a certain pin.
  */
Index: linux-2.6-tip/arch/x86/include/asm/io_apic.h
===================================================================
--- linux-2.6-tip.orig/arch/x86/include/asm/io_apic.h
+++ linux-2.6-tip/arch/x86/include/asm/io_apic.h
@@ -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;
 }
Index: linux-2.6-tip/arch/x86/kernel/apic/apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/apic.c
@@ -1461,7 +1461,6 @@ int __init enable_IR(void)
 void __init enable_IR_x2apic(void)
 {
 	unsigned long flags;
-	struct IO_APIC_route_entry **ioapic_entries;
 	int ret, x2apic_enabled = 0;
 	int dmar_table_init_ret;
 
@@ -1469,13 +1468,7 @@ void __init enable_IR_x2apic(void)
 	if (dmar_table_init_ret && !x2apic_supported())
 		return;
 
-	ioapic_entries = alloc_ioapic_entries();
-	if (!ioapic_entries) {
-		pr_err("Allocate ioapic_entries failed\n");
-		goto out;
-	}
-
-	ret = save_IO_APIC_setup(ioapic_entries);
+	ret = save_ioapic_entries();
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
 		goto out;
@@ -1483,7 +1476,7 @@ void __init enable_IR_x2apic(void)
 
 	local_irq_save(flags);
 	legacy_pic->mask_all();
-	mask_IO_APIC_setup(ioapic_entries);
+	mask_ioapic_entries();
 
 	if (dmar_table_init_ret)
 		ret = 0;
@@ -1514,14 +1507,11 @@ void __init enable_IR_x2apic(void)
 
 nox2apic:
 	if (!ret) /* IR enabling failed */
-		restore_IO_APIC_setup(ioapic_entries);
+		restore_ioapic_entries();
 	legacy_pic->restore_mask();
 	local_irq_restore(flags);
 
 out:
-	if (ioapic_entries)
-		free_ioapic_entries(ioapic_entries);
-
 	if (x2apic_enabled)
 		return;
 
@@ -2095,28 +2085,20 @@ static void lapic_resume(void)
 {
 	unsigned int l, h;
 	unsigned long flags;
-	int maxlvt, ret;
-	struct IO_APIC_route_entry **ioapic_entries = NULL;
+	int maxlvt;
 
 	if (!apic_pm_state.active)
 		return;
 
 	local_irq_save(flags);
 	if (intr_remapping_enabled) {
-		ioapic_entries = alloc_ioapic_entries();
-		if (!ioapic_entries) {
-			WARN(1, "Alloc ioapic_entries in lapic resume failed.");
-			goto restore;
-		}
-
-		ret = save_IO_APIC_setup(ioapic_entries);
-		if (ret) {
-			WARN(1, "Saving IO-APIC state failed: %d\n", ret);
-			free_ioapic_entries(ioapic_entries);
-			goto restore;
-		}
-
-		mask_IO_APIC_setup(ioapic_entries);
+		/*
+		 * IO-APIC and PIC have their own resume routines.
+		 * We just mask them here to make sure the interrupt
+		 * subsystem is completely quiet while we enable x2apic
+		 * and interrupt-remapping.
+		 */
+		mask_ioapic_entries();
 		legacy_pic->mask_all();
 	}
 
@@ -2159,13 +2141,9 @@ static void lapic_resume(void)
 	apic_write(APIC_ESR, 0);
 	apic_read(APIC_ESR);
 
-	if (intr_remapping_enabled) {
+	if (intr_remapping_enabled)
 		reenable_intr_remapping(x2apic_mode);
-		legacy_pic->restore_mask();
-		restore_IO_APIC_setup(ioapic_entries);
-		free_ioapic_entries(ioapic_entries);
-	}
-restore:
+
 	local_irq_restore(flags);
 }
 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs
  2011-05-16 18:56 [patch 1/4] x86, ioapic: fix potential resume deadlock Suresh Siddha
  2011-05-16 18:56 ` [patch 2/4] x86, ioapic: allocate ioapic_saved_data early Suresh Siddha
  2011-05-16 18:56 ` [patch 3/4] x86, ioapic: use ioapic_saved_data while enabling intr-remapping Suresh Siddha
@ 2011-05-16 18:56 ` Suresh Siddha
  2011-05-17  9:08   ` Ingo Molnar
  2011-05-16 19:53 ` [patch 1/4] x86, ioapic: fix potential resume deadlock Daniel J Blueman
  3 siblings, 1 reply; 6+ messages in thread
From: Suresh Siddha @ 2011-05-16 18:56 UTC (permalink / raw)
  To: mingo, tglx, hpa; +Cc: linux-kernel, daniel.blueman, suresh.b.siddha

[-- Attachment #1: use_saved_ioapic_for_suspend.patch --]
[-- Type: text/plain, Size: 2185 bytes --]

Code flow for enabling interrupt-remapping has its own routines for saving and
restoring io-apic RTE's. ioapic suspend/resume code flow also has similar
routines. Remove the duplicate code.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 arch/x86/kernel/apic/io_apic.c |   36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
@@ -2887,37 +2887,11 @@ static int __init io_apic_bug_finalize(v
 
 late_initcall(io_apic_bug_finalize);
 
-static void suspend_ioapic(int ioapic_id)
+static void resume_ioapic_id(int ioapic_id)
 {
-	struct IO_APIC_route_entry *saved_data = ioapic_saved_data[ioapic_id];
-	int i;
-
-	if (!saved_data)
-		return;
-
-	for (i = 0; i < nr_ioapic_registers[ioapic_id]; i++)
-		saved_data[i] = ioapic_read_entry(ioapic_id, i);
-}
-
-static int ioapic_suspend(void)
-{
-	int ioapic_id;
-
-	for (ioapic_id = 0; ioapic_id < nr_ioapics; ioapic_id++)
-		suspend_ioapic(ioapic_id);
-
-	return 0;
-}
-
-static void resume_ioapic(int ioapic_id)
-{
-	struct IO_APIC_route_entry *saved_data = ioapic_saved_data[ioapic_id];
 	unsigned long flags;
 	union IO_APIC_reg_00 reg_00;
-	int i;
 
-	if (!saved_data)
-		return;
 
 	raw_spin_lock_irqsave(&ioapic_lock, flags);
 	reg_00.raw = io_apic_read(ioapic_id, 0);
@@ -2926,8 +2900,6 @@ static void resume_ioapic(int ioapic_id)
 		io_apic_write(ioapic_id, 0, reg_00.raw);
 	}
 	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
-	for (i = 0; i < nr_ioapic_registers[ioapic_id]; i++)
-		ioapic_write_entry(ioapic_id, i, saved_data[i]);
 }
 
 static void ioapic_resume(void)
@@ -2935,11 +2907,13 @@ static void ioapic_resume(void)
 	int ioapic_id;
 
 	for (ioapic_id = nr_ioapics - 1; ioapic_id >= 0; ioapic_id--)
-		resume_ioapic(ioapic_id);
+		resume_ioapic_id(ioapic_id);
+
+	restore_ioapic_entries();
 }
 
 static struct syscore_ops ioapic_syscore_ops = {
-	.suspend = ioapic_suspend,
+	.suspend = save_ioapic_entries,
 	.resume = ioapic_resume,
 };
 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 1/4] x86, ioapic: fix potential resume deadlock
  2011-05-16 18:56 [patch 1/4] x86, ioapic: fix potential resume deadlock Suresh Siddha
                   ` (2 preceding siblings ...)
  2011-05-16 18:56 ` [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs Suresh Siddha
@ 2011-05-16 19:53 ` Daniel J Blueman
  3 siblings, 0 replies; 6+ messages in thread
From: Daniel J Blueman @ 2011-05-16 19:53 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: mingo, tglx, hpa, linux-kernel, stable

On 16 May 2011 19:56, Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> From: Daniel J Blueman <daniel.blueman@gmail.com>
> Subject: x86, ioapic: fix potential resume deadlock
>
> Fix a potential deadlock when resuming; here the calling function
> has disabled interrupts, so we cannot sleep.
>
> Change the memory allocation flag from GFP_KERNEL to GFP_ATOMIC.
>
> TODO: We can do away with this memory allocation during resume by
> reusing the ioapic suspend/resume code that uses boot time allocated
> buffers.
>
> Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: stable@kernel.org   [v2.6.39]
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
> ===================================================================
> --- linux-2.6-tip.orig/arch/x86/kernel/apic/io_apic.c
> +++ linux-2.6-tip/arch/x86/kernel/apic/io_apic.c
> @@ -621,14 +621,14 @@ struct IO_APIC_route_entry **alloc_ioapi
>        struct IO_APIC_route_entry **ioapic_entries;
>
>        ioapic_entries = kzalloc(sizeof(*ioapic_entries) * nr_ioapics,
> -                               GFP_KERNEL);
> +                               GFP_ATOMIC);
>        if (!ioapic_entries)
>                return 0;
>
>        for (apic = 0; apic < nr_ioapics; apic++) {
>                ioapic_entries[apic] =
>                        kzalloc(sizeof(struct IO_APIC_route_entry) *
> -                               nr_ioapic_registers[apic], GFP_KERNEL);
> +                               nr_ioapic_registers[apic], GFP_ATOMIC);
>                if (!ioapic_entries[apic])
>                        goto nomem;
>        }

I just tested this patch series (1-3) and it does addresses the resume
allocation failures I was consistently seeing (triggering your
warning); it passes nicely with full lock, allocation etc debugging,
so looks all good. Tested-by: Daniel J Blueman
<daniel.blueman@gmail.com>

Thanks, Suresh!

Daniel
-- 
Daniel J Blueman

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs
  2011-05-16 18:56 ` [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs Suresh Siddha
@ 2011-05-17  9:08   ` Ingo Molnar
  0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2011-05-17  9:08 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: tglx, hpa, linux-kernel, daniel.blueman


ok, this series looks better, but as this bug has demonstrated it we need to do 
better to keep the ioapic code clean.

Firstly, please introduce a 'struct ioapic' structure that starts out with a 
nr_registers field, and add an ioapics[] array and consolidate 
nr_ioapic_registers into this.

The consolidate other ioapic driver state as well:

 - add a *saved_registers field and consolidate ioapic_saved_data into it

 - add a 'struct mpc_ioapic mp_config' entry and consolidate mp_ioapics[]

 - add a 'struct mp_ioapic_gsi gsi_config' entry and consolidate mp_gsi_routing[]

 - add a 'int pin_programmed' field and consolidate mp_ioapic_routing[] into it

ioapics[] itself should be static to io_apic.c.

Please create a separate patch for each change: that way it's bisectable and 
reviewable.

These changes alone will make the IO-APIC code a *lot* more readable, more 
extensible - and hopefully much less prone to suspend/resume bugs as well.

Feel free to do this on top of your current queue to keep your patch-shuffling 
overhead low.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-05-17  9:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-16 18:56 [patch 1/4] x86, ioapic: fix potential resume deadlock Suresh Siddha
2011-05-16 18:56 ` [patch 2/4] x86, ioapic: allocate ioapic_saved_data early Suresh Siddha
2011-05-16 18:56 ` [patch 3/4] x86, ioapic: use ioapic_saved_data while enabling intr-remapping Suresh Siddha
2011-05-16 18:56 ` [patch 4/4] x86, ioapic: remove duplicate code for saving/restoring RTEs Suresh Siddha
2011-05-17  9:08   ` Ingo Molnar
2011-05-16 19:53 ` [patch 1/4] x86, ioapic: fix potential resume deadlock Daniel J Blueman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.