Attempted summary of suspend-blockers LKML thread
in [Kernel]
|
Prev: [PATCH 6/7] usb: iowarrior: fix misuse of return value of copy_to_user()
Next: Get Back To Me Immediately....
From: Alan Stern on 8 Aug 2010 15:50 On Sat, 7 Aug 2010, Arve Hj�nnev�g wrote: > The suspend blockers I added I my suspend blocker patchset were not > directly associated with a device. For the benefit of those who don't remember the details of that patchset, can you list again the suspend blockers/wakelocks that aren't directly associated with a device? (No need to mention things coming from userspace -- those are obviously unrelated to any device.) > The evdev changes could be modified > to share a device, but it would give less detail since a separate > queue is created for each client that opens the device. I don't understand enough about how the input layer works. Does it create a new queue each time an input device file is opened? And each input event gets added to all of the queues? And each queue gets a corresponding wakelock? If these surmises are right then I can see how relying on devices alone wouldn't give enough information. There could be two separate programs both reading input events, and you wouldn't know which one was responsible for failing to drain its queue. > The suspend > blocking work api would have to change so the caller to passes a > device in, which I think would make that api less flexible. Maybe. There aren't enough examples coded up yet to be sure. At this point we could easily make the device argument required instead of optional, and we could add a device argument to pm_relax. > Mostly the > problem is that we need separate stats for wakelocks created by a > single driver. For instance we will still need a user-space interface > to block suspend on android devices (lower level services than the > power manager need to block suspend), with the stats in the device > struct we have to create a new device for every wakelock user space > creates in the kernel. In the scheme we're talking about, the suspend-blocking interface for uesrspace would be located entirely in the power manager. Hence it could maintain all the necessary statistics, without involving the kernel. During early stages of system startup, before the power manager is running, lower-level services would not be able to block suspend. This wouldn't matter because at those times the system simply would not ever suspend -- because suspends have to be initiated by the power manager. > There is also the issue of reading the stats. It is a lot easier to > read a single stats file, than looping though every device on the > system (when most of the devices never block suspend). I agree, it should be possible to simplify this and perhaps at the same time avoid adding all the wakeup-related fields to every device structure. > >> > Does Android use any kernel-internal wakelocks both with a timer and > >> > with active cancellation? > >> > > >> > >> I don't know if they are all kernel-internal but these drivers appear > >> to use timeouts and active cancellation on the same wakelock: > >> wifi driver, mmc core, alarm driver, evdev (suspend blocker version > >> removes the timeout). Roughly speaking, what do the timings work out to be? That is, how long are the timeouts for these wakelocks, how long does it usually take for one of them to be actively cancelled, and what percentage of them end up timing out? > >> The wakelock code has an active flag. If we want to keep the > >> pm_stay_wake nesting (which I would argue against), we would need an > >> active count. It would also require a handle, which is a change Rafael > >> said would not fly. Never mind the handles for now. As for the nesting, we effectively don't have it when the pm_wakeup_event interface is used. The pm_stay_awake/pm_relax interface _is_ nested, but since that interface isn't used anywhere yet, we can't say whether it really should be nested or not. Maybe we'll find out in the end that it shouldn't be. Consider an input event queue as a typical case. You would call pm_stay_awake when an item is added to an empty queue, and you would call pm_relax when the last item is removed from a queue. The nesting count would never be > 1, so it wouldn't make any difference either way. > >> >> sleep_time, total time the wake lock has been active when the screen was off. > >> > > >> > Not applicable to general systems. �Is there anything like it that > >> > _would_ apply in general? > >> > > >> > >> The screen off is how it is used on android, the stats is keyed of > >> what user space wrote to /sys/power/state. If "on" was written the > >> sleep time is not updated. I guess we could keep track of pm_stay_awake or pm_wakeup_event calls made between a read and write of /sys/power/wakeup_count (i.e., while events_check_enabled is set). Would that be roughly equivalent? > >> The merged user space interface makes this unclear to me. When I first > >> used suspend on android I had a power manager process that opened all > >> the input devices and reset a screen off timeout every time there was > >> an input event. If the input layer uses pm_stay_awake to block suspend > >> when the queue is not empty, this will deadlock with the current > >> interface since reading the wake count will block forever if an input > >> event occurred right after the power manager decides to suspend. > > > > No, in that case suspend will be aborted, IIUC. > > > > How? Your pm_get_wakeup_count function loops until events_in_progress becomes 0. The problem is that the same task is reading /sys/power/wakeup_count (thus waiting for an input event queue to drain) and also reading an input event queue. Clearly this will result in deadlock, but there must be a reasonably simple way around it. For example, carry out those two activities in separate threads. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on 8 Aug 2010 16:00 On Saturday, August 07, 2010, Arve Hj�nnev�g wrote: > 2010/8/7 Rafael J. Wysocki <rjw(a)sisk.pl>: > > On Saturday, August 07, 2010, Arve Hj�nnev�g wrote: > >> 2010/8/6 Alan Stern <stern(a)rowland.harvard.edu>: > >> > On Thu, 5 Aug 2010, Arve Hj�nnev�g wrote: > >> > > >> >> count, tells you how many times the wakelock was activated. If a > >> >> wakelock prevented suspend for a long time a large count tells you it > >> >> handled a lot of events while a small count tells you it took a long > >> >> time to process the events, or the wakelock was not released properly. > >> > > >> > As noted, we already have this. > >> > > >> > >> Almost. We have it when a device is passed in. > > > > Sure. And what are the other cases (details, please)? > > > > The suspend blockers I added I my suspend blocker patchset were not > directly associated with a device. The evdev changes could be modified > to share a device, but it would give less detail since a separate > queue is created for each client that opens the device. OK, that's a good argument. > The suspend blocking work api would have to change so the caller to passes > a device in, which I think would make that api less flexible. Mostly the > problem is that we need separate stats for wakelocks created by a > single driver. For instance we will still need a user-space interface > to block suspend on android devices (lower level services than the > power manager need to block suspend), with the stats in the device > struct we have to create a new device for every wakelock user space > creates in the kernel. Well, what about managing these stats in user space? > There is also the issue of reading the stats. It is a lot easier to > read a single stats file, than looping though every device on the > system (when most of the devices never block suspend). It seems you can have a list of the "interesting" ones. > >> >> expire_count, tells you how many times the timeout expired. For the > >> >> input event wakelock in the android kernel (which has a timeout) an > >> >> expire count that matches the count tells you that someone opened an > >> >> input device but is not reading from it (this has happened several > >> >> times). > >> > > >> > This is a little tricky. Rafael's model currently does not allow > >> > wakeup events started by pm_wakeup_event() to be cancelled any way > >> > other than by having their timer expire. This essentially means that > >> > for some devices, expire_count will always be the same as count and for > >> > others it will always be 0. To change this would require adding an > >> > extra timer struct, which could be done (in fact, an earlier version of > >> > the code included it). It would be nice if we could avoid the need. > >> > > >> > Does Android use any kernel-internal wakelocks both with a timer and > >> > with active cancellation? > >> > > >> > >> I don't know if they are all kernel-internal but these drivers appear > >> to use timeouts and active cancellation on the same wakelock: > >> wifi driver, mmc core, alarm driver, evdev (suspend blocker version > >> removes the timeout). > > > > You previously said you didn't need timeouted wakelocks in the kernel, so > > I guess that was incorrect. > > > > I don't know what you are reffering to. We have always stated that we > need timeouts in the kernel to pass events through other kernel layers > that do not use wakelocks (that list is much longer than the list > above which mixes timeouts and unlock on the same wakelock). The only > feature we do not use is the timeout feature in the user space > interface to kernel wakelocks. Now that's more clear, thanks. > >> >> wake_count, tells you that this is the first wakelock that was > >> >> acquired in the resume path. This is currently less useful than I > >> >> would like on the Nexus One since it is usually "SMD_RPCCALL" which > >> >> does not tell me a lot. > >> > > >> > This could be done easily enough, but if it's not very useful then > >> > there's no point. > >> > > >> It is useful there is no other way to tell what triggered a wakeup, > >> but it would probably be better to just track wakeup interrupts/events > >> elsewhere. > >> > >> >> active_since, tells you how long a a still active wakelock has been > >> >> active. If someone activated a wakelock and never released it, it will > >> >> be obvious here. > >> > > >> > Easily added. But you didn't mention any field saying whether the > >> > wakelock is currently active. That could be added too (although it > >> > would be racy -- but for detecting unreleased wakelocks you wouldn't > >> > care). > >> > > >> > >> These are the reported stats, not the fields in the stats structure. > >> The wakelock code has an active flag. If we want to keep the > >> pm_stay_wake nesting (which I would argue against), we would need an > >> active count. It would also require a handle, which is a change Rafael > >> said would not fly. > >> > >> >> total_time, total time the wake lock has been active. This one should > >> >> be obvious. > >> > > >> > Also easily added. > >> > > >> Only with a handle passed to all the calls. > > > > Well, I'm kind of tired of this "my solution is the only acceptable one" > > mindset. IMHO, it's totally counter productive. > > > > How do you propose to track how long a driver has blocked suspend when > you have an unblock call that takes no arguments. You can extend pm_relax() to take a dev argument and measure the time between pm_stay_awake() and pm_relax() called for the same device. > >> >> sleep_time, total time the wake lock has been active when the screen was off. > >> > > >> > Not applicable to general systems. Is there anything like it that > >> > _would_ apply in general? > >> > > >> > >> The screen off is how it is used on android, the stats is keyed of > >> what user space wrote to /sys/power/state. If "on" was written the > >> sleep time is not updated. > >> > >> >> max_time, longest time the wakelock was active uninterrupted. This > >> >> used less often, but the battery on a device was draining fast, but > >> >> the problem went away before looking at the stats this will show if a > >> >> wakelock was active for a long time. > >> > > >> > Again, easily added. The only drawback is that all these additions > >> > will bloat the size of struct device. Of course, that's why you used > >> > separately-allocated structures for your wakelocks. Maybe we can > >> > change to do the same; it seems likely that the majority of device > >> > structures won't ever be used for wakeup events. > >> > > >> > >> Since many wakelocks are not associated with s struct device we need a > >> separate object for this anyway. > >> > >> >> >> and I would prefer that the kernel interfaces would > >> >> >> encourage drivers to block suspend until user space has consumed the > >> >> >> event, which works for the android user space, instead of just long > >> >> >> enough to work with a hypothetical user space power manager. > >> > > >> > Rafael doesn't _discourage_ drivers from doing this. However you have > >> > to keep in mind that many kernel developers are accustomed to working > >> > on systems (mostly PCs) with a different range of hardware devices from > >> > embedded systems like your phones. With PCI devices(*), for example, > >> > there's no clear point where a wakeup event gets handed off to > >> > userspace. > >> > > >> > On the other hand, there's no reason the input layer shouldn't use > >> > pm_stay_awake and pm_relax. It simply hasn't been implemented yet. > >> ... > >> > >> The merged user space interface makes this unclear to me. When I first > >> used suspend on android I had a power manager process that opened all > >> the input devices and reset a screen off timeout every time there was > >> an input event. If the input layer uses pm_stay_awake to block suspend > >> when the queue is not empty, this will deadlock with the current > >> interface since reading the wake count will block forever if an input > >> event occurred right after the power manager decides to suspend. > > > > No, in that case suspend will be aborted, IIUC. > > > > How? Your pm_get_wakeup_count function loops until events_in_progress becomes 0. So, to deadlock with it you'd have to call pm_stay_awake() and wait for it to complete. However, right now there are no means by which user space can call pm_stay_awake(), so this can't happen. Of course, if you add pm_stay_awake() to an ioctl() code path, you should make sure that whoever uses that ioctl() won't be waiting for the power manager to read from /sys/power/wakeup_count. I guess your point is that this isn't possible to achieve? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on 9 Aug 2010 01:00 2010/8/8 Alan Stern <stern(a)rowland.harvard.edu>: > On Sat, 7 Aug 2010, Arve Hj�nnev�g wrote: > >> The suspend blockers I added I my suspend blocker patchset were not >> directly associated with a device. > > For the benefit of those who don't remember the details of that > patchset, can you list again the suspend blockers/wakelocks that aren't > directly associated with a device? �(No need to mention things coming > from userspace -- those are obviously unrelated to any device.) Other than the user space interface and the internal main suspend blocker that patchset modified evdev, and added suspend blocking work. > >> The evdev changes could be modified >> to share a device, but it would give less detail since a separate >> queue is created for each client that opens the device. > > I don't understand enough about how the input layer works. �Does it > create a new queue each time an input device file is opened? �And each > input event gets added to all of the queues? �And each queue gets a > corresponding wakelock? > Yes, yes and yes. > If these surmises are right then I can see how relying on devices alone > wouldn't give enough information. �There could be two separate programs > both reading input events, and you wouldn't know which one was > responsible for failing to drain its queue. > We don't know which one now either since we don't give them unique names, but we know that one of them is not reading (and it is usually not the normal input path). >> The suspend >> blocking work api would have to change so the caller to passes a >> device in, which I think would make that api less flexible. > > Maybe. �There aren't enough examples coded up yet to be sure. �At this > point we could easily make the device argument required instead of > optional, and we could add a device argument to pm_relax. > I think that would be a good start. I can create dummy devices for now where I don't already have a device. >> Mostly the >> problem is that we need separate stats for wakelocks created by a >> single driver. For instance we will still need a user-space interface >> to block suspend on android devices (lower level services than the >> power manager need to block suspend), with the stats in the device >> struct we have to create a new device for every wakelock user space >> creates in the kernel. > > In the scheme we're talking about, the suspend-blocking interface for > uesrspace would be located entirely in the power manager. �Hence it > could maintain all the necessary statistics, without involving the > kernel. �During early stages of system startup, before the power > manager is running, lower-level services would not be able to block > suspend. �This wouldn't matter because at those times the system simply > would not ever suspend -- because suspends have to be initiated by the > power manager. If we do that, a low level process could try to block suspend while the power manager is not running. Then the power manager could start and decide to suspend not knowing that a low level process wanted to block suspend. > >> There is also the issue of reading the stats. It is a lot easier to >> read a single stats file, than looping though every device on the >> system (when most of the devices never block suspend). > > I agree, it should be possible to simplify this and perhaps at the same > time avoid adding all the wakeup-related fields to every device > structure. > >> >> > Does Android use any kernel-internal wakelocks both with a timer and >> >> > with active cancellation? >> >> > >> >> >> >> I don't know if they are all kernel-internal but these drivers appear >> >> to use timeouts and active cancellation on the same wakelock: >> >> wifi driver, mmc core, alarm driver, evdev (suspend blocker version >> >> removes the timeout). > > Roughly speaking, what do the timings work out to be? �That is, how > long are the timeouts for these wakelocks, how long does it usually > take for one of them to be actively cancelled, and what percentage of > them end up timing out? > The evdev timeout is a few seconds, and never timeout unless user space is misbehaving. I don't know how the wifi and mmc wakelocks are used. The alarm driver uses a 1 or 2 second timeout to abort suspend when an alarm is set to close to program an rtc alarm. I think this one is cancelled when the alarm triggers, and the timeout only handles the case where the client cancelled the alarm before it trigger, but I don't remember for sure. >> >> The wakelock code has an active flag. If we want to keep the >> >> pm_stay_wake nesting (which I would argue against), we would need an >> >> active count. It would also require a handle, which is a change Rafael >> >> said would not fly. > > Never mind the handles for now. �As for the nesting, we effectively > don't have it when the pm_wakeup_event interface is used. �The > pm_stay_awake/pm_relax interface _is_ nested, but since that interface > isn't used anywhere yet, we can't say whether it really should be > nested or not. �Maybe we'll find out in the end that it shouldn't be. > You cannot easily mix timeouts, cancellations and nesting. Wakelocks/suspend blockers allow you to mix timeouts and cancellations. > Consider an input event queue as a typical case. �You would call > pm_stay_awake when an item is added to an empty queue, and you would > call pm_relax when the last item is removed from a queue. �The nesting > count would never be > 1, so it wouldn't make any difference either > way. > The cleanup is a little simpler when you don't nest and there is less chance that a missed edge case prevents suspend forever (we had a few of those bugs in our userspace code that used nested wakelocks). If the above is all you do, then you block suspend forever if someone closes an non-empty input device. >> >> >> sleep_time, total time the wake lock has been active when the screen was off. >> >> > >> >> > Not applicable to general systems. �Is there anything like it that >> >> > _would_ apply in general? >> >> > >> >> >> >> The screen off is how it is used on android, the stats is keyed of >> >> what user space wrote to /sys/power/state. If "on" was written the >> >> sleep time is not updated. > > I guess we could keep track of pm_stay_awake or pm_wakeup_event calls > made between a read and write of /sys/power/wakeup_count (i.e., while > events_check_enabled is set). �Would that be roughly equivalent? > Roughly, yes. (or even just while blocking the read from /sys/power/wakeup_count). >> >> The merged user space interface makes this unclear to me. When I first >> >> used suspend on android I had a power manager process that opened all >> >> the input devices and reset a screen off timeout every time there was >> >> an input event. If the input layer uses pm_stay_awake to block suspend >> >> when the queue is not empty, this will deadlock with the current >> >> interface since reading the wake count will block forever if an input >> >> event occurred right after the power manager decides to suspend. >> > >> > No, in that case suspend will be aborted, IIUC. >> > >> >> How? Your pm_get_wakeup_count function loops until events_in_progress becomes 0. > > The problem is that the same task is reading /sys/power/wakeup_count > (thus waiting for an input event queue to drain) and also reading an > input event queue. �Clearly this will result in deadlock, but there > must be a reasonably simple way around it. �For example, carry out > those two activities in separate threads. > Yes, but my point is that user-space code that is safe if the driver does not block suspend until the queue is empty becomes unsafe if it does. Also note that it is not enough to just use two separate threads for this, the thread that reads /sys/power/wakeup_count cannot hold a lock while reading, or directly provide the user space wakelock api. I do think it is possible to use this api and provide a user space wakelock api (by having a dedicated thread that suspends), but it is more complicated than using the asynchronous wakelock/suspend blocker api. -- Arve Hj�nnev�g -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on 9 Aug 2010 01:10 2010/8/8 Rafael J. Wysocki <rjw(a)sisk.pl>: > On Saturday, August 07, 2010, Arve Hj�nnev�g wrote: .... >> The suspend blocking work api would have to change so the caller to passes >> a device in, which I think would make that api less flexible. Mostly the >> problem is that we need separate stats for wakelocks created by a >> single driver. For instance we will still need a user-space interface >> to block suspend on android devices (lower level services than the >> power manager need to block suspend), with the stats in the device >> struct we have to create a new device for every wakelock user space >> creates in the kernel. > > Well, what about managing these stats in user space? > The java wakelocks have stats in user space, but the lower level services that use the kernel api do not show up there. >> There is also the issue of reading the stats. It is a lot easier to >> read a single stats file, than looping though every device on the >> system (when most of the devices never block suspend). > > It seems you can have a list of the "interesting" ones. > I'm not sure what you mean. If you mean add a procfs or debugfs api that lists the stats for devices that have used the api, then yes that should work. If I have a devices where the battery drained too quickly, I run cat /proc/wakelocks which has all the reasons why it was not suspended. .... >> >> >> total_time, total time the wake lock has been active. This one should >> >> >> be obvious. >> >> > >> >> > Also easily added. >> >> > >> >> Only with a handle passed to all the calls. >> > >> > Well, I'm kind of tired of this "my solution is the only acceptable one" >> > mindset. �IMHO, it's totally counter productive. >> > >> >> How do you propose to track how long a driver has blocked suspend when >> you have an unblock call that takes no arguments. > > You can extend pm_relax() to take a dev argument and measure the time between > pm_stay_awake() and pm_relax() called for the same device. > As long as it is not optional. >> >> >> sleep_time, total time the wake lock has been active when the screen was off. >> >> > >> >> > Not applicable to general systems. �Is there anything like it that >> >> > _would_ apply in general? >> >> > >> >> >> >> The screen off is how it is used on android, the stats is keyed of >> >> what user space wrote to /sys/power/state. If "on" was written the >> >> sleep time is not updated. >> >> >> >> >> max_time, longest time the wakelock was active uninterrupted. This >> >> >> used less often, but the battery on a device was draining fast, but >> >> >> the problem went away before looking at the stats this will show if a >> >> >> wakelock was active for a long time. >> >> > >> >> > Again, easily added. �The only drawback is that all these additions >> >> > will bloat the size of struct device. �Of course, that's why you used >> >> > separately-allocated structures for your wakelocks. �Maybe we can >> >> > change to do the same; it seems likely that the majority of device >> >> > structures won't ever be used for wakeup events. >> >> > >> >> >> >> Since many wakelocks are not associated with s struct device we need a >> >> separate object for this anyway. >> >> >> >> >> >> and I would prefer that the kernel interfaces would >> >> >> >> encourage drivers to block suspend until user space has consumed the >> >> >> >> event, which works for the android user space, instead of just long >> >> >> >> enough to work with a hypothetical user space power manager. >> >> > >> >> > Rafael doesn't _discourage_ drivers from doing this. �However you have >> >> > to keep in mind that many kernel developers are accustomed to working >> >> > on systems (mostly PCs) with a different range of hardware devices from >> >> > embedded systems like your phones. �With PCI devices(*), for example, >> >> > there's no clear point where a wakeup event gets handed off to >> >> > userspace. >> >> > >> >> > On the other hand, there's no reason the input layer shouldn't use >> >> > pm_stay_awake and pm_relax. �It simply hasn't been implemented yet. >> >> ... >> >> >> >> The merged user space interface makes this unclear to me. When I first >> >> used suspend on android I had a power manager process that opened all >> >> the input devices and reset a screen off timeout every time there was >> >> an input event. If the input layer uses pm_stay_awake to block suspend >> >> when the queue is not empty, this will deadlock with the current >> >> interface since reading the wake count will block forever if an input >> >> event occurred right after the power manager decides to suspend. >> > >> > No, in that case suspend will be aborted, IIUC. >> > >> >> How? Your pm_get_wakeup_count function loops until events_in_progress becomes 0. > > So, to deadlock with it you'd have to call pm_stay_awake() and wait for it to > complete. �However, right now there are no means by which user space can call > pm_stay_awake(), so this can't happen. > No that is not how it deadlocks. The input driver calls pm_stay_awake which blocks the thread that reads from /sys/power/wakeup_count. If that threads needs to run before the user-space reads from the input device (either because it is the same thread, or because it provides a user-space wakelock api) you have a deadlock. > Of course, if you add pm_stay_awake() to an ioctl() code path, you should make > sure that whoever uses that ioctl() won't be waiting for the power manager to > read from /sys/power/wakeup_count. �I guess your point is that this isn't > possible to achieve? > My main point is that blocking suspend while the input event queue is not empty changes what else is safe for the user-space process reading /sys/power/wakeup_count. -- Arve Hj�nnev�g -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on 9 Aug 2010 01:40
2010/8/8 Rafael J. Wysocki <rjw(a)sisk.pl>: > On Saturday, August 07, 2010, Arve Hj�nnev�g wrote: >> 2010/8/7 Arve Hj�nnev�g <arve(a)android.com>: >> > 2010/8/7 Rafael J. Wysocki <rjw(a)sisk.pl>: >> >> On Saturday, August 07, 2010, Arve Hj�nnev�g wrote: >> >>> 2010/8/6 Alan Stern <stern(a)rowland.harvard.edu>: >> >>> > On Thu, 5 Aug 2010, Arve Hj�nnev�g wrote: >> ... >> >>> >> total_time, total time the wake lock has been active. This one should >> >>> >> be obvious. >> >>> > >> >>> > Also easily added. >> >>> > >> >>> Only with a handle passed to all the calls. >> >> >> >> Well, I'm kind of tired of this "my solution is the only acceptable one" >> >> mindset. �IMHO, it's totally counter productive. >> >> >> > >> > How do you propose to track how long a driver has blocked suspend when >> > you have an unblock call that takes no arguments. >> > >> >> Also, I did not not see a response to my question about why you don't >> want to pass a handle. > > It doesn't really matter what I personally want. �In fact, I'm not totally > opposed to that idea, although there are disadvantages (eg. a "handle" > would really mean a pointer to an object with certain life cycle that needs to > be managed by the caller and it's not that clear to me who should manage the > objects that the PCI wakeup code would pass to pm_wakeup_event(), for one Wouldn't a single global handle work for the way you are handling pci wakeup events? It looked like you just reset a global timeout every time a pci wakeup event occurs. > example). �I sent a pull request for your original patchset to Linus after all. :-) > > I said I didn't think "it would fly", meaning that I was afraid the other kernel > developers wouldn't like that change. > > The reason why I think so is that you'd like to add a whole new infrastructure > whose only purpose would be debugging that would only be useful to systems > using opportunistic suspend. �That, however, is only Android right now and it > cannot use the mainline kernel for other reasons, so basically we would add > infrastructure that's useful to no one. > I'm not sure what you mean by this. The debugging is useful for anyone using the api, not just Android, and a handle is also needed to mix timeouts and pm_relax. The handle can be the device, but some drivers need several handles per device. -- Arve Hj�nnev�g -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |