On Fri, 16 Feb 2007, Oliver Neukum wrote: > Am Freitag, 16. Februar 2007 16:45 schrieb Alan Stern: > > On Thu, 15 Feb 2007, Oliver Neukum wrote: > > > > imho caring about the reason for a suspension is a bit messy. The current > > > code is nice. It just introduces a new method. > > > > But it's a new method that essentially duplicates the purpose of an > > existing method: > > > > The current suspend() method tells the driver that the device is > > about to be suspended, gives the driver a chance to prepare, and > > allows the driver to report an error (possibly aborting the > > suspend). > > > > Your new method tells the driver that the device is about to be > > autosuspended, gives the driver a chance to prepare (by stopping > > the output queue), and allows the driver to report an error > > (aborting the autosuspend). > > This is true. You are making a very good point. However taking it to its > logical conclusion we'd remove any care about idleness from usbcore > and let the drivers fail suspend if they are busy, subject to auto_pm.
We're getting away from the real question: Why is your new method any better than a simple bitflag to indicate idleness together with the existing method? Yes, the bitflag isn't a reliable synchronization mechanism, and races can force the driver to fail an autosuspend. Unless you think these races will be common, I don't see any advantage to adding a new method. (And I also don't see much advantage in having the new method abort an autosuspend as opposed to having the existing method abort it.) > [..] > > As far is the hardware is concerned, there are exactly two states: running > > and suspended. There's no difference between autosuspend, suspend because > > the user wrote to a sysfs file, and suspend because the system as a whole > > is going to sleep. Like the hardware, the software doesn't keep track of > > the reason for a suspend once the suspend has taken place. > > Begging the question about snapshotting during suspend to disk. > It seems to me that remote wakeup must be disabled. How do we > do that? Good question. The answer is simple: We don't do it. The PM core arranges that no new requests will be made, hence no autoresume upon demand (since there will be no demand). Remote wakeup is enabled, but remote-wakeup requests won't be delivered, since these requests are handled by khubd and khubd will be frozen. (In fact, there's no reason why we shouldn't leave remote wakeup disabled during the snapshotting phase, that is, when we get a PM_EVENT_FREEZE or PM_EVENT_PRETHAW message. However nobody has bothered to implement this small optimization.) Or to put it a slightly different way... Suspend and resume can occur only in process context. During snapshotting the only important process still running is the STD process itself; everything else that matters is frozen. > > > > It does in the usb-storage case. We raise the count whenever a I/O > > > > transfer starts, and decrement it when the transfer finishes (setting > > > > the > > > > DEVICE_BUSY flag as well). These are lightweight operations, without > > > > much > > > > overhead other than acquiring the pm_mutex. If the timer expires while > > > > an > > > > operation is in progress, usbcore sees the elevated count and restarts > > > > the > > > > > > Why restart? You could always restated if you lower the count again. > > > > That's the whole point. I _don't want_ to restart the timer every time > > the count gets lowered, because the count will be lowered after every I/O > > transfer -- which can mean hundreds of times per second. > > You will have to do that only if you kill the timer upon incrementing > the count. Yes. I don't want to kill the timer upon incrementing the count or restart it upon decrementing the count. Hence the timer needs to be restarted whenever it expires and the device is still not idle (the count is elevated) -- which is what I wrote above. We probably are in agreement on this point, although it may not seem that way... Alan Stern ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
