On Monday, August 30, 2010, Alan Stern wrote: > On Mon, 30 Aug 2010, Rafael J. Wysocki wrote: > > > > One last thing... I'm definitely planning to make the suspend_delay > > > value affect suspends initiated by pm_schedule_suspend. (The routine > > > will always use the maximum of the specified delay and the value > > > calculated from last_busy plus suspend_delay. Also, > > > pm_suspend_timer_fn would check to see if last_busy has changed and > > > restart the timer if needed.) > > > > Hmm. What exactly is going to be the purpose of this? > > The usage case is a disk drive, or more generally any block device. > The driver handles requests in atomic context, and each each time a > request completes the driver will try to suspend. But of course we > don't want to put the device into low power until after a sufficiently > long idle period, so the driver will call pm_schedule_suspend. And > remember, a disk drive often needs to handle a _lot_ of requests > quickly, so we need to minimize the overhead of using runtime PM. > > Of course, the driver could do all the last_busy plus suspend_delay > calculations itself, but since these calculations will be the same for > all devices we might as well do them in the PM core. The driver can > then simply call pm_schedule_suspend with the delay argument set to 0, > and it will work properly. > > As an optimization, under reasonable conditions we might want to skip > updating the suspend timer. Rather than incur the overhead of > mod_timer every time a request completes, we can just let the timer > expire and have the timer function reschedule itself. Depending on how > often requests complete and how long the suspend timeout is, this could > add up to a big saving. (I don't know how much extra overhead is > needed when a timer expires, but I suspect it is relatively small -- > not too much larger than the overhead of mod_timer itself.) > > Since last_busy won't be protected by the runtime spinlock, if the > driver wants to be really safe it should repeat the last_busy + > suspend_idle check while holding a private lock. The PM core can > export a routine to make this easy. Regardless, repeating the check is > optional. Furthermore, failure of the check in the driver's > runtime_suspend callback has much higher overhead than failure in the > timer handler, because the invoking the runtime_suspend callback > requires a context switch to the PM workqueue thread. > > > > I'm not sure whether suspends initiated from pm_runtime_suspend > > > or pm_request_suspend should behave the same way. What do you think? > > > > I don't think they should at the moment, but that depends a good deal on the > > answer to the question above. :-) > > Did I provide a suitable answer? Your question was rather open-ended. I guess I'd need to know more about how the USB autosuspend mechanism works to understand it entirely. I really don't think it's a good idea to even change the way pm_schedule_suspend() works, because it's already been used by drivers that don't support device autosuspend. So, if autosuspend support is needed at the PM core level, I'd prefer it to be added in the form of a separate helper function (that may share code with the existing ones). Also, IMO, suspend_delay_ms should be accessible to user space only if the device's driver and/or subsystem support autosuspend, so that the meaning of the value written to this file by user space is always well defined. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
