On Wed, 24 Oct 2018 10:10:32 -0400 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > On Mon, 22 Oct 2018 10:24:46 -0400 > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > On Mon, 22 Oct 2018, Oliver Neukum wrote: > > > > > > > On Do, 2018-10-18 at 13:42 -0400, Alan Stern wrote: > > > > > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > > > > > > > > > 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 > > > > > > > > If and only if you want to do this with one ioctl() > > > > If you separate the runtime-PM put and the get, you can do it without > > > > the waiting part. > > > > > > Sure, but you still need a runtime-PM put at the start and a runtime-PM > > > get at the end. In fact, if you separate this into two calls then you > > > run the risk that the user may never perform the second call, and you > > > would end up with an unbalanced put. > > > > > > > I am tending to agree towards having a single ioctl call here. It is better > > to give minimal control to user-space w.r.t. runtime PM. The current > > proposal of user-space giving an hint to USB-FS/core that - it is not using > > the device, sounds better. > > We reviewed the proposal internally and looks like we have following cases: 1. Host wake from L2: This needs to be done when end-user wants to interact with device && device is in suspend/link in L2. 2. Remote wake from L2 by device: This happens when USB device detects some activity while link was in L2. As per my understanding, the current ioctl proposal seems to address (2) above. But (1) seems to have issue which is, the end-user has to wait for remote-wake from device to interact with it (since device is already open and blocked in new ioctl). With that said, it looks like we would need 3 ioctls as below: 1. suspend: to be called by user-space when it knows it is done using the device. Should be non-blocking. It should just drop the PM ref-count on device. 2. resume: to be called by user-space when it wants to actively interact with the device. Should be non-blocking. It should just get the PM ref-count on device. 3. wait-for-remote-wake: to be called by user-space when it wants to wait for remote wake of device && end user not actively use it. Should be blocking. Unblocked by resume. It should have different return value to indicate resume-by-host or resume-by-remote-wake (apart from signal info). Does this sound reasonable (considering one of my previous comments :-/)? > > > > > > /* > > > > > > * There are 3 possibilities here: > > > > > > * 1. Device did suspend and resume (success) > > > > > > * 2. Signal was received (failed suspend) > > > > > > * 3. Time-out happened (failed suspend) > > > > > > > > > > 4. Device did suspend but a signal was received before the device > > > > > resumed. > > > > > > > > > > > * In any of above cases, we need to resume device. > > > > > > */ > > > > > > usb_autoresume_device(dev); > > > > > > > > Yes and that is the problem. Why do you want to wait for the result > > > > of runtime-PM put ? If we need a channel for notifying user space > > > > about resume of a device, why wait for the result of suspend instead > > > > of using the same channel? > > > > > > This is not meant to be a general-purpose channel for notifying > > > userspace when a device resumes. Such a channel should be defined in > > > the runtime-PM layer, not in the USB layer. > > > > > > This is instead meant to be a special-purpose mechanism for adding a > > > runtime-suspend/resume interface to usbfs. > > > > > > > Just to be clear here - the worst case wait-time from user-space > > perspective will be <time-out - 1 HZ> + <time-to-resume>, right? > > That's right. > > > If yes then, timeout argument means "wait at-least timeout sec". > > Yes. Unless for a few unlikely cases, including: > > A signal is received before the timeout expires; > > The device suspends and then resumes before the timeout > expires. > > > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > We spend time internally to go over the "new" ioctl proposal. Overall > > it looks promising. > > > > However, we still have an issue as below - > > Consider a use-case where, user-space calls "new" ioctl, but suspend > > never happen (for various reasons) && async event happens on USB > > device side by the end-user. > > > > In such a case, since user-space is waiting in "new" ioctl, it is not > > in position to queue a request to read-out the async event info. It > > will be able to queue a request when the "new" ioctl returns which > > will be "time-out" later (in this case). Due to auto-suspend > > time-out's default's value of 2 sec, the user-space has to choose the > > time-out to "new" ioctl > 2 sec. > > Not so. That "2 second" value can be adjusted by the user; it can be > reduced to as little as 1 ms. > > Alan Stern
