On Wed, 17 Oct 2018 10:32:14 -0400
Alan Stern <[email protected]> wrote:
> On Wed, 17 Oct 2018, Oliver Neukum wrote:
>
> > On Di, 2018-10-16 at 10:46 -0400, Alan Stern wrote:
> > > On Tue, 16 Oct 2018, Mayuresh Kulkarni wrote:
> > >
> > > > 1. User space decides when it is time to suspend the device and calls
> > > > this ioctl. The implmentation takes care to ensure, no URB is in
> > > > flight from this caller to this device. Then proceed with suspend.
> > >
> > > Well, I did not imagine the ioctl would try to ensure that no URBs are
> > > in flight. The user would be responsible for that.
> >
> > Well, we have to check for URBs in flight. In fact we would have to
> > "plug" the device against new URBs. Now, we could use a new counter.
> > But if we check asynclist, we may just as well walk it and kill the
> > URBs. It just seems a bit silly not to do that.
>
> In fact, the USB core already flushes outstanding URBs when a device
> gets suspended.
>
> I don't know that "plugging" is needed. We could simply let new URBs
> fail (the core prevents URBs from being sent to a suspended device).
>
> > > Also, proceeding with the suspend is difficult, as Oliver has pointed
> > > out. There is no guarantee that the device will go into suspend,
> > > merely because the userspace driver isn't accessing it. (In fact,
> > > there's no guarantee that the device will _ever_ go into suspend.) The
> > > ioctl would probably have to include some sort of timeout; it would
> > > return early if the timeout expired before the device was suspended.
> >
> > Exactly. The alternative is to have an ioctl() to just allow or block
> > suspend, more or less just exporting autopm_get/put(). The disadvantage
> > is that
> >
> > 1. The kernel has to cancel URBs in flight
> > 2. There needs to be a mechanism to notify user space about events
> >
> > > > 2. In an another thread, the user-space calls poll(USB-device-fd).
> > > > When poll() returns, that means the device is active now. User space
> > > > can then request an URB to read out the async notification, if any.
> > >
> > > No; no other thread or polling is needed. The ioctl call returns when
> > > the device resumes. At that point the user program can check whether
> > > there is an async notification pending.
> >
> > This is problematic. For example, what do we do if a signal needs
> > to be delivered? The obvious solution of just repeating the call will
> > not work. It races with wakeup. You'd need to restart IO to query
> > the device. But the device may be suspended. Or do you want a signal
> > to up the PM counter again?
>
> The only way to make the ioctl work properly is to have it do a
> runtime-PM put at the start and then a runtime-PM get before it
> returns. This is true regardless of the reason for returning: normal
> termination, timeout, signal, whatever. Nothing else would be safe.
>
Will below steps work safely (sometimes pseudo-code/snippets help to align)? -
"new" ioctl -
timeout is parameter to ioctl.
/* attempt suspend of device */
usb_autosuspend_device(dev);
usb_unlock_device(dev);
r = wait_event_interruptible_timeout(ps->resume_wait,
(ps->resume_done == true), timeout * HZ);
usb_lock_device(dev);
/*
* There are 3 possibilities here:
* 1. Device did suspend and resume (success)
* 2. Signal was received (failed suspend)
* 3. Time-out happened (failed suspend)
* In any of above cases, we need to resume device.
*/
usb_autoresume_device(dev);
ps->resume_done = false;
"ps->resume_done = true;" will be done by the runtime resume call-back.
> > Making Ctrl-C wake up an unrelated device?
>
> I don't follow.
>
> > What do we do in case of a failed suspend or resume?
>
> The ioctl returns when the usbfs runtime-resume callback is invoked.
> This may or may not happen after a failed suspend. But if the device
> doesn't go into suspend within the ioctl's timeout period, the ioctl
> will return in any case.
>
> In case of a failed resume, what _can_ we do?
>
> > How about reset_resume?
>
> Indeed, what happens if a device in use by usbfs gets reset even while
> it isn't suspended? We should do the same thing as much as possible,
> regardless of the device's state when the reset occurred.
>
> Alan Stern
