RFC 0/4: make ACPI interpret safe for suspend/resume

RFC 0/4: make ACPI interpret safe for suspend/resume

[Li Shaohua]

Hi,
I'm sending the patches not for merging but for comments. We currently
encounter a big issue for suspend/resume. I take S3 for an example, but
S4 (or S4BIOS) is the same. The detail is:
1. PCI link device acts as a sysdev, that means its .resume method will
be executed with IRQ disabled. The .resume method will invoke ACPI _SRS
method, and possibly execute any AML code. The log (at the bottom of the
email) is a failure case caused by this issue.
2. Current suspend/resume code will freeze all processes first and then
start doing suspend/resume. SO 'acpi_pm_prepare', 'acpi_pm_enter' and
acpi_pm_finish' will be invoked with other processes are frozen. The 3
(at least the first and the last ones) will enter ACPI interpret. ACPI
interpret actually will do memory allocating, semaphore handling,
sleeping and memory mapping, the actual actions depend on BIOS. Consider
one case: if a user process acquire an ACPI semaphore but sleep after
suspend, and 'acpi_pm_finish' requires the semaphore, we will have
terrible deadlock.

Case 1 actually isn't severe, I basically think we should remove the PCI
link device suspend/resume code if all PCI driver suspend/resume code
call 'pci_enable_device', which will call acpi_pci_irq_enable' and do
the same thing like the link device resume code. So case 1 actually is
similar with case 2.

Changing ACPI interpret is impossible, since it includes too many code.
Fortunately, all OS dependent code of ACPI interpret is in osl.c, below
patches try to change osl.c and make ACPI interpret safe.
patch 1: introduce a new system state to shadow 'might_sleep' complain.
patch 2: solve the memory allocating issue. we suspend only if there is
enough free memory.
patch 3: solve the acpi_os_sleep issue.
patch 4: solve the semaphore issue. We track the semaphore usage of
ACPICA, and suspend wait till all semaphroes are free.
Unresolved issue is 'acpi_os_memory_mapping' and 'acpi_os_read_memory'.
We possibly must introduce new memory APIs, which will be something like
'atomic_vmap'.
The principle here is we can tolerate suspend failure but we can't
tolerate resume failure. I did test the patches but they are not
matured. Comments and suggestions will be welcome!

Thanks,
Shaohua

P.S. Did anybody know why we should freeze all processes for S3 and
S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
all device drivers can freeze request to them, it's possibly not
required to me.


Debug: sleeping function called from invalid context at mm/slab.c:2091
in_atomic():0, irqs_disabled():1
 [<c011877f>] __might_sleep+0x8a/0x94
 [<c0146108>] kmem_cache_alloc+0x1b/0x6b
 [<c0205e6c>] acpi_pci_link_set+0x4a/0x1a2
 [<c02062d2>] irqrouter_resume+0x1c/0x24
 [<c0244d97>] sysdev_resume+0x3e/0xa5
 [<c0248066>] device_power_up+0x5/0xa
 [<c01379ce>] suspend_enter+0x25/0x2d
 [<c0137a34>] enter_state+0x37/0x53
 [<c0203417>] acpi_suspend+0x28/0x35
 [<c02034e6>] acpi_system_write_sleep+0x5a/0x6b
 [<c015eecd>] vfs_write+0xba/0x115
 [<c015efc6>] sys_write+0x3c/0x62
 [<c010338b>] syscall_call+0x7/0xb
ACPI: PCI interrupt 0000:00:1f.1[A] -> GSI 7 (level, low) -> IRQ 7
Restarting tasks... done



-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Nigel Cunningham]

Hi.

On Wed, 2005-01-19 at 14:17, Li Shaohua wrote:
> Hi,
> I'm sending the patches not for merging but for comments. We currently
> encounter a big issue for suspend/resume. I take S3 for an example, but
> S4 (or S4BIOS) is the same. The detail is:
> 1. PCI link device acts as a sysdev, that means its .resume method will
> be executed with IRQ disabled. The .resume method will invoke ACPI _SRS
> method, and possibly execute any AML code. The log (at the bottom of the
> email) is a failure case caused by this issue.
> 2. Current suspend/resume code will freeze all processes first and then
> start doing suspend/resume. SO 'acpi_pm_prepare', 'acpi_pm_enter' and
> acpi_pm_finish' will be invoked with other processes are frozen. The 3
> (at least the first and the last ones) will enter ACPI interpret. ACPI
> interpret actually will do memory allocating, semaphore handling,
> sleeping and memory mapping, the actual actions depend on BIOS. Consider
> one case: if a user process acquire an ACPI semaphore but sleep after
> suspend, and 'acpi_pm_finish' requires the semaphore, we will have
> terrible deadlock.

I'm not sure that there should be any problem here: processes should
drop semaphores before entering the signal handler on the way to the
freezer, shouldn't they? Have any freezes been observed relating to
these calls?

> Case 1 actually isn't severe, I basically think we should remove the PCI
> link device suspend/resume code if all PCI driver suspend/resume code
> call 'pci_enable_device', which will call acpi_pci_irq_enable' and do
> the same thing like the link device resume code. So case 1 actually is
> similar with case 2.
> 
> Changing ACPI interpret is impossible, since it includes too many code.
> Fortunately, all OS dependent code of ACPI interpret is in osl.c, below
> patches try to change osl.c and make ACPI interpret safe.
> patch 1: introduce a new system state to shadow 'might_sleep' complain.

Pavel knows far better than me here, so I will leave it to him to
comment.

> patch 2: solve the memory allocating issue. we suspend only if there is
> enough free memory.

This one shouldn't be a problem either. Pavel, do you still have the
half-of-memory limit? It is certainly redundant in my case - I already
make sure the number of available pages is much higher than 100.

> patch 3: solve the acpi_os_sleep issue.

Does this interact with the recent patches for the pic suspend/resume
code? If so, how?

> patch 4: solve the semaphore issue. We track the semaphore usage of
> ACPICA, and suspend wait till all semaphroes are free.
> Unresolved issue is 'acpi_os_memory_mapping' and 'acpi_os_read_memory'.
> We possibly must introduce new memory APIs, which will be something like
> 'atomic_vmap'.

(See above)

> The principle here is we can tolerate suspend failure but we can't
> tolerate resume failure. I did test the patches but they are not
> matured. Comments and suggestions will be welcome!

Hope mine are helpful.

> Thanks,
> Shaohua
> 
> P.S. Did anybody know why we should freeze all processes for S3 and
> S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
> all device drivers can freeze request to them, it's possibly not
> required to me.

I'll leave this one for Pavel too.

Nigel
-- 
Nigel Cunningham
Software Engineer
Cyclades Corporation

http://cyclades.com



-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Li Shaohua]

On Wed, 2005-01-19 at 15:21, Nigel Cunningham wrote:
> Hi.
> 
> On Wed, 2005-01-19 at 14:17, Li Shaohua wrote:
> > Hi,
> > I'm sending the patches not for merging but for comments. We currently
> > encounter a big issue for suspend/resume. I take S3 for an example, but
> > S4 (or S4BIOS) is the same. The detail is:
I would correct, this affects S3 and S4BIOS. But if PCI link device is a
sysdev (so S4 will call its resume method), it also affects S4, but main
effect is for S3.
 
> > 1. PCI link device acts as a sysdev, that means its .resume method will
> > be executed with IRQ disabled. The .resume method will invoke ACPI _SRS
> > method, and possibly execute any AML code. The log (at the bottom of the
> > email) is a failure case caused by this issue.
> > 2. Current suspend/resume code will freeze all processes first and then
> > start doing suspend/resume. SO 'acpi_pm_prepare', 'acpi_pm_enter' and
> > acpi_pm_finish' will be invoked with other processes are frozen. The 3
> > (at least the first and the last ones) will enter ACPI interpret. ACPI
> > interpret actually will do memory allocating, semaphore handling,
> > sleeping and memory mapping, the actual actions depend on BIOS. Consider
> > one case: if a user process acquire an ACPI semaphore but sleep after
> > suspend, and 'acpi_pm_finish' requires the semaphore, we will have
> > terrible deadlock.
> 
> I'm not sure that there should be any problem here: processes should
> drop semaphores before entering the signal handler on the way to the
> freezer, shouldn't they? Have any freezes been observed relating to
> these calls?
The semaphores actually are in kernel (in ACPI interpret). A process can
sleep with semaphore hold (in kernel), how it knows to drop semaphores
before entering the freeze fake signal handler? The case is one
userspace task request some ACPI services and sleep or switched out in
kernel after get ACPI semaphores. Am I completely missing anything? No,
I didn't observe any failure case caused by this one, it's potential.

> > Case 1 actually isn't severe, I basically think we should remove the PCI
> > link device suspend/resume code if all PCI driver suspend/resume code
> > call 'pci_enable_device', which will call acpi_pci_irq_enable' and do
> > the same thing like the link device resume code. So case 1 actually is
> > similar with case 2.
> > 
> > Changing ACPI interpret is impossible, since it includes too many code.
> > Fortunately, all OS dependent code of ACPI interpret is in osl.c, below
> > patches try to change osl.c and make ACPI interpret safe.
> > patch 1: introduce a new system state to shadow 'might_sleep' complain.
> 
> Pavel knows far better than me here, so I will leave it to him to
> comment.
> 
> > patch 2: solve the memory allocating issue. we suspend only if there is
> > enough free memory.
> 
> This one shouldn't be a problem either. Pavel, do you still have the
> half-of-memory limit? It is certainly redundant in my case - I already
> make sure the number of available pages is much higher than 100.
Yep, S4 has freed memory before suspend, but S3 doesn't.

> > patch 3: solve the acpi_os_sleep issue.
> 
> Does this interact with the recent patches for the pic suspend/resume
> code? If so, how?
Which patch does you point to? The one just is related with case 1, but
as I said case 1 can be resolved by other way, so this patch isn't
required in future.
> 
> > patch 4: solve the semaphore issue. We track the semaphore usage of
> > ACPICA, and suspend wait till all semaphroes are free.
> > Unresolved issue is 'acpi_os_memory_mapping' and 'acpi_os_read_memory'.
> > We possibly must introduce new memory APIs, which will be something like
> > 'atomic_vmap'.
> 
> (See above)
Do you mean the memory allocation issue? The acpi_os_memory_mapping uses
ioremap, I'm afraid there is no free VMA space in resume time.
> 
> > The principle here is we can tolerate suspend failure but we can't
> > tolerate resume failure. I did test the patches but they are not
> > matured. Comments and suggestions will be welcome!
> 
> Hope mine are helpful.
I'm very appreciated with your replay, Nigel!

Thanks,
Shaohua



-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Pavel Machek]

Hi!

> I'm sending the patches not for merging but for comments. We currently
> encounter a big issue for suspend/resume. I take S3 for an example, but
> S4 (or S4BIOS) is the same. The detail is:
> 1. PCI link device acts as a sysdev, that means its .resume method will
> be executed with IRQ disabled. The .resume method will invoke ACPI _SRS
> method, and possibly execute any AML code. The log (at the bottom of the
> email) is a failure case caused by this issue.
> 2. Current suspend/resume code will freeze all processes first and then
> start doing suspend/resume. SO 'acpi_pm_prepare', 'acpi_pm_enter' and
> acpi_pm_finish' will be invoked with other processes are frozen. The 3
> (at least the first and the last ones) will enter ACPI interpret. ACPI
> interpret actually will do memory allocating, semaphore handling,
> sleeping and memory mapping, the actual actions depend on BIOS. Consider
> one case: if a user process acquire an ACPI semaphore but sleep after
> suspend, and 'acpi_pm_finish' requires the semaphore, we will have
> terrible deadlock.

Actually, this should not ever happen. Refrigerator only hits at
specific places, where we *know* no semaphores are held. If you can
find a counterexample, that's a bug to be fixed.

> Changing ACPI interpret is impossible, since it includes too many code.
> Fortunately, all OS dependent code of ACPI interpret is in osl.c, below
> patches try to change osl.c and make ACPI interpret safe.
> patch 1: introduce a new system state to shadow 'might_sleep'
> complain.

Ask akpm about this one (I once tried to add 4 or so system states to
solve driver model problem, and it was vetoed. One state actually
makes more sense, but... ask akpm, cc me). But you should better be
able to write comment describing how is STATE_SUSPEND different from
other states.

[Or perhaps you want this variable to be acpi-local?]

> patch 4: solve the semaphore issue. We track the semaphore usage of
> ACPICA, and suspend wait till all semaphroes are free.
> Unresolved issue is 'acpi_os_memory_mapping' and 'acpi_os_read_memory'.
> We possibly must introduce new memory APIs, which will be something like
> 'atomic_vmap'.

Process holding semaphore should not be able to enter
refrigerator. Can you track down how that can happen?

> P.S. Did anybody know why we should freeze all processes for S3 and
> S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
> all device drivers can freeze request to them, it's possibly not
> required to me.

Freezing all processes should not be required. I added it for i386
(BenH is not doing it on ppc) because I felt I do not want driver
authors to have to care about concurent userspace accesses. Like if
you are suspending and do ifconfig while machine is suspending, you do
not want to see network interface in the down state.

We could say "it is the driver problem".
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Li Shaohua]

On Wed, 2005-01-19 at 18:18, Pavel Machek wrote:
> > I'm sending the patches not for merging but for comments. We currently
> > encounter a big issue for suspend/resume. I take S3 for an example, but
> > S4 (or S4BIOS) is the same. The detail is:
> > 1. PCI link device acts as a sysdev, that means its .resume method will
> > be executed with IRQ disabled. The .resume method will invoke ACPI _SRS
> > method, and possibly execute any AML code. The log (at the bottom of the
> > email) is a failure case caused by this issue.
> > 2. Current suspend/resume code will freeze all processes first and then
> > start doing suspend/resume. SO 'acpi_pm_prepare', 'acpi_pm_enter' and
> > acpi_pm_finish' will be invoked with other processes are frozen. The 3
> > (at least the first and the last ones) will enter ACPI interpret. ACPI
> > interpret actually will do memory allocating, semaphore handling,
> > sleeping and memory mapping, the actual actions depend on BIOS. Consider
> > one case: if a user process acquire an ACPI semaphore but sleep after
> > suspend, and 'acpi_pm_finish' requires the semaphore, we will have
> > terrible deadlock.
> 
> Actually, this should not ever happen. Refrigerator only hits at
> specific places, where we *know* no semaphores are held. If you can
> find a counterexample, that's a bug to be fixed.
Ok, I now understand it. I made a big mistake. Thanks correcting me,
Pavel & Nigel.

> > Changing ACPI interpret is impossible, since it includes too many code.
> > Fortunately, all OS dependent code of ACPI interpret is in osl.c, below
> > patches try to change osl.c and make ACPI interpret safe.
> > patch 1: introduce a new system state to shadow 'might_sleep'
> > complain.
> 
> Ask akpm about this one (I once tried to add 4 or so system states to
> solve driver model problem, and it was vetoed. One state actually
> makes more sense, but... ask akpm, cc me). But you should better be
> able to write comment describing how is STATE_SUSPEND different from
> other states.
Ok, I will do it.

> [Or perhaps you want this variable to be acpi-local?]
For the memory allocating issue and semaphore issue, we actually can not
use the new system state, we can just do:
if (in_atomic() || in_interrupt())
	kmalloc(.., GFP_ATOMIC)
But this possibly will mask some real runtime errors, so we want to add
a new system state to make suspend/resume work and not break normal
process. But If new system state is a bad idea, we can make it
acpi-local.

> > patch 4: solve the semaphore issue. We track the semaphore usage of
> > ACPICA, and suspend wait till all semaphroes are free.
> > Unresolved issue is 'acpi_os_memory_mapping' and 'acpi_os_read_memory'.
> > We possibly must introduce new memory APIs, which will be something like
> > 'atomic_vmap'.
> 
> Process holding semaphore should not be able to enter
> refrigerator. Can you track down how that can happen?
> 
> > P.S. Did anybody know why we should freeze all processes for S3 and
> > S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
> > all device drivers can freeze request to them, it's possibly not
> > required to me.
> 
> Freezing all processes should not be required. 
How about just make suspend/resume task as a highest priority task with
FIFO scheduler policy? just like the stop_machine_run does. Actually we
have some idea that make stop_machine_run work for UP, and use it in
suspend/resume (so no refrigerator is required).
> I added it for i386
> (BenH is not doing it on ppc) because I felt I do not want driver
> authors to have to care about concurent userspace accesses. Like if
> you are suspending and do ifconfig while machine is suspending, you do
> not want to see network interface in the down state.
> We could say "it is the driver problem
Yep, this makes sense to me.
Thanks your comments.

Shaohua



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Pavel Machek]

Hi!

> > [Or perhaps you want this variable to be acpi-local?]
> For the memory allocating issue and semaphore issue, we actually can not
> use the new system state, we can just do:
> if (in_atomic() || in_interrupt())
> 	kmalloc(.., GFP_ATOMIC)
> But this possibly will mask some real runtime errors, so we want to add
> a new system state to make suspend/resume work and not break normal
> process. But If new system state is a bad idea, we can make it
> acpi-local.

Actually, I do not quite like new system state, but introducing new
"in_suspend()" macro would certainly be ok. We actually talked about
that one on linux-pm, just noone got around to implement it.

> > > P.S. Did anybody know why we should freeze all processes for S3 and
> > > S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
> > > all device drivers can freeze request to them, it's possibly not
> > > required to me.
> > 
> > Freezing all processes should not be required. 
> How about just make suspend/resume task as a highest priority task with
> FIFO scheduler policy? 

That might do the trick for S3, but not for swsusp. Imagine normal
process holding some lock you want (for writing image to disk). You
can run on FIFO scheduler, but then you block on this lock, and
userspace process gets to run, and gets confused.
							Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Li Shaohua]

On Thu, 2005-01-20 at 17:12, Pavel Machek wrote:
> Hi!
> 
> > > [Or perhaps you want this variable to be acpi-local?]
> > For the memory allocating issue and semaphore issue, we actually can not
> > use the new system state, we can just do:
> > if (in_atomic() || in_interrupt())
> > 	kmalloc(.., GFP_ATOMIC)
> > But this possibly will mask some real runtime errors, so we want to add
> > a new system state to make suspend/resume work and not break normal
> > process. But If new system state is a bad idea, we can make it
> > acpi-local.
> 
> Actually, I do not quite like new system state, but introducing new
> "in_suspend()" macro would certainly be ok. We actually talked about
> that one on linux-pm, just noone got around to implement it.
I rethought the issue and get a new idea (I'll resend it later). Now we
don't require the new system state. But in_suspend is a good idea.

> > > > P.S. Did anybody know why we should freeze all processes for S3 and
> > > > S4BIOS? I checked FreeBSD code, it doesn't. I know it's safer, but if
> > > > all device drivers can freeze request to them, it's possibly not
> > > > required to me.
> > > 
> > > Freezing all processes should not be required. 
> > How about just make suspend/resume task as a highest priority task with
> > FIFO scheduler policy? 
> 
> That might do the trick for S3,
What I like it is a) it speeds up freeze process in a system with many
tasks, b) easy to move to SMP case on the future.

>  but not for swsusp. Imagine normal
> process holding some lock you want (for writing image to disk). You
> can run on FIFO scheduler, but then you block on this lock, and
> userspace process gets to run, and gets confused.
Ok, that makes sense. Could we split S3 from swsusp? That is S3 doesn't
use freeze_process.

Thanks,
Shaohua



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl

Re: RFC 0/4: make ACPI interpret safe for suspend/resume

[Pavel Machek]

Hi!

> > > > Freezing all processes should not be required. 
> > > How about just make suspend/resume task as a highest priority task with
> > > FIFO scheduler policy? 
> > 
> > That might do the trick for S3,
> What I like it is a) it speeds up freeze process in a system with many
> tasks, b) easy to move to SMP case on the future.
> 
> >  but not for swsusp. Imagine normal
> > process holding some lock you want (for writing image to disk). You
> > can run on FIFO scheduler, but then you block on this lock, and
> > userspace process gets to run, and gets confused.
> Ok, that makes sense. Could we split S3 from swsusp? That is S3 doesn't
> use freeze_process.

Yes, that's certainly an option. I liked the idea of code being
shared, and had refrigerator code handy...
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl