Re: [PATCH] Remove process freezer from suspend to RAM pathway

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

On Sunday, 8 July 2007 23:20, Benjamin Herrenschmidt wrote:
> 
> > But I'm not sure it's a good idea in the long run.  Think of a printer 
> > daemon, for example.  It shouldn't have to experience unexpected I/O 
> > problems merely because someone has decided to put the system to sleep.
> 
> Why not ? Printer is offline when machine is asleep... trying to print
> errors out, I don't see the problem there. At one point, we'll need a
> cleaner way to also notify userland in which case our daemon could
> become more intelligent and stop servicings things before sleep and
> resume afterward :-)
>  
> > This will be up to the people responsible for the subsystems.  I can 
> > take care of USB.
> 
> USB is not that much of a problem in the sense that for most "leaf"
> drivers, USB is a provider (ie, the bus they sit on), not the client
> (like the network stack is to network drivers).
> 
> In most cases, that "helper" thing would sit on the client subsystem,
> since it's the one feeding drivers with requests. The main ones I see at
> hand are block, alsa, net, fb/drm... Some of them already have
> infrastructure to do it, some my need some more work.
> 
> > > I think it's a fairly significant change from the current freezer and I
> > > also think it's a very good idea. The more I think about it, the more I
> > > like it, in the sense that it's a simple drop-in that you could put in a
> > > lot of the ioctl path of drivers to just block tasks that are trying to
> > > call in while suspending, and could be used selectively by things like
> > > the USB hub threads.
> > 
> > That's what I had in mind.  Rafael, can we add an "icebox" routine?  
> > Like Ben says, it doesn't need to be much more than a waitqueue
> > that the current task puts itself on if a suspend is in progress.  
> > Callers arriving at a time when the icebox isn't activated should
> > simply return without blocking.  Basically the icebox should be active 
> > at the same times as the existing freezer.
> 
> There is still the race of:
> 
> 	drivers_sysfs_write()
> 		try_to_icebox()
> 		<---- <sleep request gets here>
> 		hit hardware
> 
> Those are akin, in some ways, to the freezer races.

I'm not sure what races you're referring to, but never mind. :-)

This is the reason why the freezer waits for tasks to freeze, actually.

> Some kind of RCU might take care of them if we enable the icebox,
> then wait for all tasks to hit an explicit schedule point once (or return to
> userland).

This is what the freezer does, isn't it? ;-)

> That would mean that drivers need to try_to_icebox() again if they do
> something that may schedule (such as __get_user). So it's not a magic
> solution, it has issues, but it can handle a lot of the simplest cases. 

Well, can't we do:

 	drivers_sysfs_write()
		while(!suspend_trylock())
			try_to_icebox() --> or even try_to_freeze(), what's the difference?

 		hit hardware
		unlock_suspend()

where the PM core must wait for the suspend lock to get released (say with a
timeout)?
 
> > Here's a wacky idea which just might work:
> > 
> > In order to prevent binding and unbinding, while suspending devices all
> > the PM core has to do is avoid dropping the device semaphores!  It can
> > release the semaphores as it resumes the devices.
> > 
> > Of course, for this to work it's necessary to avoid changes to the 
> > device list during the suspend.  However I believe the iteration can be 
> > made safe against unregistration, so we only have to prevent device 
> > registration.  (And anyway, it won't be possible to unregister a device 
> > while the PM core is holding its semaphore.)
> > 
> > If we are willing to be somewhat non-transparent, this is easy to
> > accomplish.  After the notifier chain has been alerted about the
> > upcoming suspend, we tell the driver core to disallow adding new
> > devices.  Maybe use SRCU to synchronize with registration calls that
> > are in progress.  Thus, until the suspend is over device_add() will
> > immediately return an error.  We could even add a new ESUSPENDING code
> > to errno.h; it would come in handy in a few places.
> > 
> > Drivers are already prepared for device registration to fail (or they
> > ought to be), so this change shouldn't knock the bottom out of things.  
> > device_add() isn't on a hot path, so adding an extra check and
> > srcu_read_lock() won't hurt.
> 
> True. Also, bus drivers could just flag the port with something saying
> "try registering again later". Don't underestimate the power of "try
> later" constructs :-)
> 
> > I have had the same thought, that unbinding and unregistration would be 
> > easier to handle than binding and registration.  As it happens, holding 
> > the device semaphore will block both all three -- which makes life 
> > simpler.
> 
> Yup. Fair and simple.
> 
> > There are other possibilities too.  For example, instead of using
> > keventd these attributes could use a separate workqueue which would put
> > itself in the icebox during a suspend.  Or maybe sysfs can be reworked 
> > so that they don't need to use a workqueue at all.
> 
> I think having a facility for a given workqueue entry to requeue itself
> for after resume might be of use for drivers too.

Hmm, perhaps we can use a special workqueue that's created before the suspend
(say by the PM core) and starts processing jobs after the resume?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth