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

["Rafael J. Wysocki" <rjw@sisk.pl>]

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

Hi,

On Wednesday 26 April 2006 00:18, Nigel Cunningham wrote:
> On Wednesday 26 April 2006 08:06, Rafael J. Wysocki wrote:
> > On Tuesday 25 April 2006 23:03, Nigel Cunningham wrote:
> > > On Wednesday 26 April 2006 06:53, Rafael J. Wysocki wrote:
> > > > On Tuesday 25 April 2006 22:28, Nigel Cunningham wrote:
> > > > > On Wednesday 26 April 2006 04:56, Rafael J. Wysocki wrote:
> > > > > > You're saying that (9) is wrong, so could you please suggest what
> > > > > > to do instead of it?
> > > > >
> > > > > 'scuse me for butting in, but here's my suggested modified version:
> > > > > > Well, suppose we have a modular driver that's not loaded before
> > > > > > resume. 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.
> > > > > > (7) (without your change) swsusp calls .suspend() for all device
> > > > > > drivers that are present at that time, but our driver is not there,
> > > > > > so its .suspend() _won't_ be called.  [Of course with your change
> > > > > > .suspend() won't be called for any driver.]
> > > > >
> > > > > We also make a list that is safely available after the atomic restore
> > > > > of drivers that have had .suspend methods called.
> > > >
> > > > Do you mean we place the list in a __nosave area?
> > >
> > > Well, I wasn't wanting to get bogged down in the details yet, but let's
> > > see what we can come up with. How about this:
> > >
> > > At the point where we do the drivers resume at resume time, we're still
> > > atomic, right? Since that's the case, we can just use get_safe_page()
> > > prior to the restore to store the list and a __nosave pointer to retain
> > > the location across the atomic restore. If we are concerned about
> > > resuming drivers allocating memory and overwriting our list (and I think
> > > that's a valid concern), space could be allocated and the list copied
> > > between doing the atomic restore and the device pwoerup/resume.
> >
> > This one seems to look better.
> >
> > Still the problem is we need to know what to do with the devices that have
> > had .suspend() routines called.  Some of them would have to be reset, some
> > of them might be left in the current state, and for some of them we'd have
> > to do something special.  Frankly, only the driver writer will know what's
> > appropriate.
> 
> I saw the 2 suspends, 1 resume comment in another part of the thread, but 
> don't believe a driver would be able to detect that 2 suspends and 1 resume 
> call had been made - at least not without some non volatile ram.

In fact we know when the second .suspend() is going to be called from the
driver, so we can tell it to take care, for example by passing a special
value to .suspend().  If that value is passed to .suspend(), the driver should
prepare the device for .resume() being called after the snapshot image is
restored (eg. reset it).

Greetings,
Rafael

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