On Fri, 16 Nov 2018, Mayuresh Kulkarni wrote:
> Thanks for the comments. Based on the info so far, attempting to summarize the
> probable solution, to ensure that I understand it clearly -
>
> Facts -
> 1. USBFS driver grabs a PM ref-count in .open and drops it in .close which
> means
> USB device cannot suspend untill user-space closes it (even if all interface
> drivers report "idle" to usb-core).
> 2. Since the .ioctl "knows" that .open has ensured to keep device active, it
> does not call PM runtime APIs.
>
> Proposal -
> 1. Add new ioctl: suspend & wait-for-resume
> 2. suspend ioctl: decrements PM ref count and return
> 3. wait-for-resume ioctl: wait for resume or timeout or signal
Do you really want to have a timeout for this ioctl? Maybe it isn't
important -- I don't know.
> 4. Modify .ioctl implementation to do PM runtime calls except for above "new"
> ioctl calls (so pm_runtime_get_sync -> ioctl -> response ->
> pm_runtime_put_sync). This also means, pm runtime get/put will be in both
> .open/.close.
That's not exactly what I had in mind. Open will do:
ps->runtime_active = true;
The new suspend ioctl will do this:
if (ps->runtime_active) {
usb_autosuspend_device(ps->dev);
ps->runtime_active = false;
}
and the old ioctls (and close) will do this at the start:
if (!ps->runtime_active) {
if (usb_autoresume_device(ps->dev))
return -EIO; /* Could not resume */
ps->runtime_active = true;
}
This means that after any interaction with the device, you will have to
call the suspend ioctl again if you want the device to go back to
sleep.
> Use-case analysis -
> 1. Remote-wake: Due to device's remote wake, wait-for-resume will return
> successfully. The user space caller then need to queue a request to "know" the
> reason of remote-wake.
> 2. Host-wake: The user-space caller issues any ioctl supported by .ioctl
> method.
> Due to (4) above, the device will be resumed and the ioctl will be performed.
Correct.
> For (2) in use-case analysis, the user-space caller's wait-for-resume will
> also
> return, but since it knows that it has initiated the ioctl, it may or may not
> decide to queue a request. Instead, when ioctl returns it can call wait-for-
> resume again.
Yes. Of course, your app will have some way to check for user
interaction with the device. Doing these checks while the device is
suspended would be counter-productive, since the check itself would
wake up the device. So you will probably want to do a check as soon as
you know the device has woken up, regardless of the cause. If you
don't, you run the risk of not noticing a user interaction.
> Am I getting in sync with your comments?
>
> What issue(s) you anticipate in above proposal due to inherent race condition
> between host and remote-wake?
Only what I mentioned above, that your program should check for user
interaction whenever it knows the device has woken up.
> Based on my meagre understanding of usb-core, it feels
> like usb_lock_device/usb_unlock_device calls around remote-wake and usbfs
> ioctl
> should help with race condition, right?
No, they will not help. This is not a race between two different parts
of the kernel both trying to communicate with the device; it is a race
between the kernel and the user. usb_lock_device doesn't prevent the
user from interacting with the device. :-)
Alan Stern
