Re: [patch/rft 2.6.17-rc2] swsusp resume must not device_suspend()

Pavel Machek
2006-04-25T21:41:19Z

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

(aha, I see the mail now)

> > > I've begun thinking that calls like pm_should_I_spin_down_drives() would be a
> > > better structural approach than continually redefining this "freeze" thing so
> > > it makes less and less sense to all other drivers ... who nonethless need to
> > > clutter themselves up with a growing list of special cases, to accomodate
> > > rotating media that may not even exist in the target system.
> >
> > I think we should do something different to device_power_down(PMSG_FREEZE)
> > there, but I'm not sure it should be kernel_restart_prepare(NULL).
> >
> > Actually spinning down disks during resume is a problem for some users (yes,
> > we've had such bug reports recently), so it's better to avoid this.
>
> Well, if we had a pm_should_I_spin_down_drives() it would make sense to me
> that it return FALSE during kernel_restart_prepare() too ... surely kexec
> users have the same issues!

pm_should_I_spin_down_drives() is incredibly ugly hack, sorry.

> > Then it goes like that (approximately):
> > (1) We activate swsusp which calls .suspend() for all devices including our
> > driver (this is a real suspend).
> > (2) swsusp snapshots the system and creates the image.
> > (3) swsusp calls .resume() for all devices in order to be able to save the
> > image (.resume() for our driver is also called which is OK).
> > (4) swsusp turns off the system.
> > (5) (some time later) We start a new kernel and tell it to resume.
> > (6) It activates swsusp which reads the image.
>
> And assuming this is an x86 PC, at this point every device is in one of three states:
>
> - initialized by BIOS. This is a particular PITA for USB, but one that's
> handled OK (mostly) except when BIOS bugs kick in. There's some nasty
> code that kicks in along with PCI quirk handling, which ensures that by
> the time Linux-USB driver could see this state (or the input subsystem
> needs to care about it), the state has morphed to reset. Video cards
> have funky issues here too.
>
> - (powerup) reset. This is the ideal state, in terms of "truth" to convey
> to the image we're about to restore ... no ambiguity, every driver will
> need to re-init. As if there were (thank you!!) no BIOS.
>
> - initialized by Linux ... which leads to the case my patch addresses.
>
> Those first two states are legit for any resume() call, and they apply in
> your scenario restriction.
>
> The third state is the problem scenario, kicking in when the driver was
> statically linked (or modprobed from initramfs, etc), but not during
> your scenario.

Driver should be able to resume() from state #3. If driver is broken
and can't do that, modify suspend() call to power down hardware.

> My patch/suggestion just ensures that instead of that broken state, reset is used.
> in all cases ... not just the "driver not initialized before snapshot resume" case.

I guess I partially see what you are saying. Behaviour in
driver-modular and driver-non-modular is indeed different and that is
not nice.

But it would be nice to have less intrusive solution than call reboot
notifiers...
Pavel
--
Thanks for all the (sleeping) penguins.

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