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

[Alan Stern]

On Mon, 9 Jul 2007, Benjamin Herrenschmidt wrote:

> > 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).

That's why I thought I would be able to handle it!  :-)

> 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. 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). 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. 

It depends on what kind of race you're worried about.  In general a 
sysfs access that causes I/O isn't much different from a char device's 
read or write.  For such cases the driver would have to do something 
like this:

	drivers_sysfs_write()
	{
	 restart:
		mutex_lock(private_io_mutex);
		if (device is suspended) {
			mutex_unlock(private_io_mutex);
			icebox();
			goto restart;
		}
		... hit hardware ...
		mutex_unlock(private_io_mutex);
	}

And of course the suspend method would have to acquire the
private_io_mutex.  Some drivers might be able to use the device
semaphore instead of adding a new mutex.

I'm more concerned about races involving device registration.  The best 
solution I can come up with is to use an rwsem:

	drivers_sysfs_write()
	{
		down_read(private_rwsem);
		... register/unregister devices ...
		up_read(private_rwsem);
	}

where the suspend notifer callout would do down_write() and the resume
callout would do up_write().  Can you suggest anything better?

> > 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 :-)

-ESUSPENDING would naturally imply that the caller should retry after 
the resume.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/