public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-27  2:48 Li, Shaohua
  2004-10-27  2:49 ` Nigel Cunningham
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Li, Shaohua @ 2004-10-27  2:48 UTC (permalink / raw)
  To: ncunningham, Pavel Machek; +Cc: Patrick Mochel, Linux Kernel Mailing List

>
>Hi!
>
>I've just had a go at fixing the issue with my implementation not
>suspending the sysdevs (I believe swsusp does the same). In the
process,
>I reworked the MTRR support so it's not treated as a sysdev. Instead,
>when we're saving cpu state, the mtrr_save function function is called.
>When we go to restore CPU state, each CPU calls a function that resets
>it's MTRRs and the 'main' cpu then frees the saved data. This is
working
>well here (did a dozen plus suspends on the trot), but I want to check
>that it sounds like the right solution to you.
>
>Perhaps this method should be made more generic? (Are there likely to
be
>other per-cpu state savers needed?)
>
>One thing I have noticed is that by adding the sysdev suspend/resume
>calls, I've gained a few seconds delay. I'll see if I can track down
the
>cause.
Is the problem MTRR resume must be with IRQ enabled, right? Could we
implement a method sysdev resume with IRQ enabled? MTRR driver isn't the
only case. The ACPI Link device is another case, it's a sysdev (it must
resume before any PCI device resumed), but its resume (it uses semaphore
and non-atomic kmalloc) can't invoked with IRQ enabled. I guess cpufreq
driver is another case when suspend/resume SMP is supported.

Thanks,
Shaohua

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-29  1:41 Li, Shaohua
  0 siblings, 0 replies; 16+ messages in thread
From: Li, Shaohua @ 2004-10-29  1:41 UTC (permalink / raw)
  To: Pavel Machek; +Cc: ncunningham, Patrick Mochel, Linux Kernel Mailing List

>> >> Is the problem MTRR resume must be with IRQ enabled, right? Could
we
>> >> implement a method sysdev resume with IRQ enabled? MTRR driver
isn't
>> >> the
>> >
>> >MTRR does not deserve to be sysdev. It is not essential for the
>> >system, it only makes it slow.
>> It's a CPU driver, cpufreq driver is the same.
>
>Well, it drives part of cpu. Fortunately that part of cpu is not
required
>for
>other drivers. cpufreq definitely should not be sysdev. If mtrr is not
>needed for drivers (I think it is not), it should not be a sysdev.
mtrr can not be a sysdev, but this is current cpufreq driver model. I
guess you need convince Dominik.

Thanks,
Shaohua

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-28  2:40 Li, Shaohua
  0 siblings, 0 replies; 16+ messages in thread
From: Li, Shaohua @ 2004-10-28  2:40 UTC (permalink / raw)
  To: Pallipadi, Venkatesh, Pavel Machek
  Cc: ncunningham, Patrick Mochel, Linux Kernel Mailing List

>>
>>> >One thing I have noticed is that by adding the sysdev
suspend/resume
>>> >calls, I've gained a few seconds delay. I'll see if I can track
down
>>> the
>>> >cause.
>>> Is the problem MTRR resume must be with IRQ enabled, right? Could we
>>> implement a method sysdev resume with IRQ enabled? MTRR driver isn't
>>> the
>>
>>MTRR does not deserve to be sysdev. It is not essential for the
>>system, it only makes it slow.
>>
>>> only case. The ACPI Link device is another case, it's a
>>sysdev (it must
>>> resume before any PCI device resumed), but its resume (it
>>uses semaphore
>>> and non-atomic kmalloc) can't invoked with IRQ enabled. I
>>guess cpufreq
>>> driver is another case when suspend/resume SMP is supported.
>>
>>I do not see how enabling interrupts before setting up IRQs is good
>>idea.
>>
>>What about this one, instead?
>>
>>* ACPI Link device should allocate with GFP_ATOMIC
>>
>>* during suspend, locks can't be taken. (We stop userland, etc). So it
>>should be okay to down_trylock() and panic if that does not work.
>
>
>Actually, I am trying another approach for Link Device.
>
>- Temporarily enable interrupts during Link Device resume. Turn off all
the
>external interrupts at suspend time. They will remain suspended until
>interrupt device resumes.
>
>Something like the patch below. Only part I don't like is controlling
the
>resume order by Makefiles and the link order. Probably we can fix that
by
>sorting the sysdev list at the boottime, depending on our ordering
>requirements. I think, the resume order we need to maintain is
something
>like this: irqrouter, pit/timer, i8259, lapic, ioapic, others

Turn off PIC/IOAPIC in suspend time doesn't mean the PIC/IOAPIC is
disabled in resume. BIOS possibly turns them on in resume.

Thanks,
Shaohua

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-28  1:54 Pallipadi, Venkatesh
  2004-10-28  9:00 ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Pallipadi, Venkatesh @ 2004-10-28  1:54 UTC (permalink / raw)
  To: Pavel Machek, Li, Shaohua
  Cc: ncunningham, Patrick Mochel, Linux Kernel Mailing List


>-----Original Message-----
>From: linux-kernel-owner@vger.kernel.org 
>[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Pavel Machek
>Sent: Wednesday, October 27, 2004 3:01 AM
>To: Li, Shaohua
>Cc: ncunningham@linuxmail.org; Patrick Mochel; Linux Kernel 
>Mailing List
>Subject: Re: Fixing MTRR smp breakage and suspending sysdevs.
>
>Hi!
>
>> >One thing I have noticed is that by adding the sysdev suspend/resume
>> >calls, I've gained a few seconds delay. I'll see if I can track down
>> the
>> >cause.
>> Is the problem MTRR resume must be with IRQ enabled, right? Could we
>> implement a method sysdev resume with IRQ enabled? MTRR driver isn't
>> the
>
>MTRR does not deserve to be sysdev. It is not essential for the
>system, it only makes it slow.
>
>> only case. The ACPI Link device is another case, it's a 
>sysdev (it must
>> resume before any PCI device resumed), but its resume (it 
>uses semaphore
>> and non-atomic kmalloc) can't invoked with IRQ enabled. I 
>guess cpufreq
>> driver is another case when suspend/resume SMP is supported.
>
>I do not see how enabling interrupts before setting up IRQs is good
>idea.
>
>What about this one, instead?
>
>* ACPI Link device should allocate with GFP_ATOMIC
>
>* during suspend, locks can't be taken. (We stop userland, etc). So it
>should be okay to down_trylock() and panic if that does not work.


Actually, I am trying another approach for Link Device.

- Temporarily enable interrupts during Link Device resume. Turn off all
the external interrupts at suspend time. They will remain suspended
until interrupt device resumes.

Something like the patch below. Only part I don't like is controlling
the resume order by Makefiles and the link order. Probably we can fix
that by sorting the sysdev list at the boottime, depending on our
ordering requirements. I think, the resume order we need to maintain is
something like this: irqrouter, pit/timer, i8259, lapic, ioapic, others

Thanks,
-Venki

--- linux-2.6.9-latest/arch/i386/kernel/i8259.c.org	2004-10-26
21:58:32.000000000 -0700
+++ linux-2.6.9-latest/arch/i386/kernel/i8259.c	2004-10-27
13:06:12.000000000 -0700
@@ -266,6 +266,10 @@ static int i8259A_resume(struct sys_devi
 static int i8259A_suspend(struct sys_device *dev, u32 state)
 {
 	save_ELCR(irq_trigger);
+	/* Stop all the interrupts from PIC until resume */
+	outb(0xff, PIC_MASTER_IMR);
+	outb(0xff, PIC_SLAVE_IMR);
+
 	return 0;
 }
 
--- linux-2.6.9-latest/arch/i386/kernel/io_apic.c.org	2004-10-26
22:05:58.000000000 -0700
+++ linux-2.6.9-latest/arch/i386/kernel/io_apic.c	2004-10-26
22:06:53.000000000 -0700
@@ -2302,6 +2302,7 @@ static int ioapic_suspend(struct sys_dev
 		*(((int *)entry) + 1) = io_apic_read(dev->id, 0x11 + 2 *
i);
 		*(((int *)entry) + 0) = io_apic_read(dev->id, 0x10 + 2 *
i);
 	}
+	clear_IO_APIC();
 	spin_unlock_irqrestore(&ioapic_lock, flags);
 
 	return 0;
--- linux-2.6.9-latest/drivers/acpi/pci_link.c.org	2004-10-26
22:13:20.000000000 -0700
+++ linux-2.6.9-latest/drivers/acpi/pci_link.c	2004-10-27
13:31:43.000000000 -0700
@@ -714,9 +714,12 @@ irqrouter_resume(
 {
 	struct list_head        *node = NULL;
 	struct acpi_pci_link    *link = NULL;
+	unsigned long 		flags;
 
 	ACPI_FUNCTION_TRACE("irqrouter_resume");
 
+	local_save_flags(flags);
+	local_irq_enable();
 	list_for_each(node, &acpi_link.entries) {
 
 		link = list_entry(node, struct acpi_pci_link, node);
@@ -727,6 +730,7 @@ irqrouter_resume(
 
 		acpi_pci_link_resume(link);
 	}
+	local_irq_restore(flags);
 	return_VALUE(0);
 }
 
--- linux-2.6.9-latest/Makefile.org	2004-10-27 12:38:19.000000000
-0700
+++ linux-2.6.9-latest/Makefile	2004-10-27 13:06:10.000000000 -0700
@@ -571,7 +571,7 @@ libs-y		:= $(libs-y1) $(libs-y2)
 # System.map is generated to document addresses of all kernel symbols
 
 vmlinux-init := $(head-y) $(init-y)
-vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
+vmlinux-main := $(drivers-y) $(core-y) $(libs-y) $(net-y)
 vmlinux-all  := $(vmlinux-init) $(vmlinux-main)
 vmlinux-lds  := arch/$(ARCH)/kernel/vmlinux.lds
 

^ permalink raw reply	[flat|nested] 16+ messages in thread
* RE: Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-27 12:23 Li, Shaohua
  2004-10-28 10:20 ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Li, Shaohua @ 2004-10-27 12:23 UTC (permalink / raw)
  To: Pavel Machek; +Cc: ncunningham, Patrick Mochel, Linux Kernel Mailing List

>
>> >One thing I have noticed is that by adding the sysdev suspend/resume
>> >calls, I've gained a few seconds delay. I'll see if I can track down
>> the
>> >cause.
>> Is the problem MTRR resume must be with IRQ enabled, right? Could we
>> implement a method sysdev resume with IRQ enabled? MTRR driver isn't
>> the
>
>MTRR does not deserve to be sysdev. It is not essential for the
>system, it only makes it slow.
It's a CPU driver, cpufreq driver is the same.

>
>> only case. The ACPI Link device is another case, it's a sysdev (it
must
>> resume before any PCI device resumed), but its resume (it uses
semaphore
>> and non-atomic kmalloc) can't invoked with IRQ enabled. I guess
cpufreq
>> driver is another case when suspend/resume SMP is supported.
>
>I do not see how enabling interrupts before setting up IRQs is good
>idea.
>
>What about this one, instead?
>
>* ACPI Link device should allocate with GFP_ATOMIC
>
>* during suspend, locks can't be taken. (We stop userland, etc). So it
>should be okay to down_trylock() and panic if that does not work.
Hmm, the only problem is ACPI link device resume code will call into
ACPI CA code. We can't change ACPI CA (its code is very huge).

Thanks,
Shaohua

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Fixing MTRR smp breakage and suspending sysdevs.
@ 2004-10-26  2:39 Nigel Cunningham
  0 siblings, 0 replies; 16+ messages in thread
From: Nigel Cunningham @ 2004-10-26  2:39 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Patrick Mochel, Linux Kernel Mailing List

Hi!

I've just had a go at fixing the issue with my implementation not
suspending the sysdevs (I believe swsusp does the same). In the process,
I reworked the MTRR support so it's not treated as a sysdev. Instead,
when we're saving cpu state, the mtrr_save function function is called.
When we go to restore CPU state, each CPU calls a function that resets
it's MTRRs and the 'main' cpu then frees the saved data. This is working
well here (did a dozen plus suspends on the trot), but I want to check
that it sounds like the right solution to you.

Perhaps this method should be made more generic? (Are there likely to be
other per-cpu state savers needed?)

One thing I have noticed is that by adding the sysdev suspend/resume
calls, I've gained a few seconds delay. I'll see if I can track down the
cause.

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Everyone lives by faith. Some people just don't believe it.
Want proof? Try to prove that the theory of evolution is true.


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

end of thread, other threads:[~2004-10-29  1:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-27  2:48 Fixing MTRR smp breakage and suspending sysdevs Li, Shaohua
2004-10-27  2:49 ` Nigel Cunningham
2004-10-27  3:20 ` Dmitry Torokhov
2004-10-27  3:17   ` Nigel Cunningham
2004-10-27  3:50     ` Dmitry Torokhov
2004-10-27  3:55       ` Nigel Cunningham
2004-10-27 10:00 ` Pavel Machek
2004-10-28 22:38   ` time and suspending sysdevs [was Re: Fixing MTRR smp breakage and suspending sysdevs.] Pavel Machek
2004-10-29  0:18     ` Nigel Cunningham
  -- strict thread matches above, loose matches on Subject: below --
2004-10-29  1:41 Fixing MTRR smp breakage and suspending sysdevs Li, Shaohua
2004-10-28  2:40 Li, Shaohua
2004-10-28  1:54 Pallipadi, Venkatesh
2004-10-28  9:00 ` Pavel Machek
2004-10-27 12:23 Li, Shaohua
2004-10-28 10:20 ` Pavel Machek
2004-10-26  2:39 Nigel Cunningham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox