Hi,
On 06-06-16 19:03, Alan Stern wrote:
On Mon, 6 Jun 2016, Hans de Goede wrote:
Hi,
On 06-06-16 16:48, Greg Kroah-Hartman wrote:
On Mon, Jun 06, 2016 at 01:44:23PM +0200, Hans de Goede wrote:
Hi All,
While looking at libusb today I ended up looking at the
reap-after-disconnect code.
What stands out is that libusb expects to be able to
reap all outstanding urbs on a device on receiving
a POLL_ERR status from poll (on supported kernels).
But the usbfs poll implementation will return POLL_ERR
as soon as ps->dev->state == USB_STATE_NOTATTACHED,
and the kernel usb code sets state = USB_STATE_NOTATTACHED
some time before usbdev_remove() gets called and moves
all pending urbs to the async_completed list.
This means that URBs can't be reaped immediately. But it will be
possible to reap them shortly after libusb gets the POLLERR status,
once the khubd work routine has processed the disconnection.
Ack.
So if libusb ends up calling poll between the
setting state = USB_STATE_NOTATTACHED and usbdev_remove()
being called then it ends up not reaping all urbs
since it will stop monitoring the device after
the first POLL_ERR.
I'll write a libusb fix for this, but I'm wondering if this
is something which we also want to fix on the kernel side?
We could. Would it be worthwhile to do so?
That is exactly the question this mail thread is intended to answer :)
Would fixing this in the kernel mean that libusb fixes are not needed?
If so, I would recommend doing this.
A kernel side fix is non trivial, whereas the libusb fix is trivial
since we also need to deal with kernels which do not support
reaping urbs after disconnect at all.
Basically the question is, do we consider not being able to
reap all urbs after the first POLL_ERR a kernel bug, or should
userspace keep track of how many urbs it has outstanding and
call the reap ioctl (with some msleep after failure) until it
has reaped all urbs ?
We already have POLLOUT to indicate when a completed URB is available
for reaping. Can libusb use that?
libusb already use that normally, but on a POLLERR it will remove
the fd from the pollfds since POLLERR will stay set once set, so
leaving it in pollfds means we will just set there busy-waiting.
Strictly speaking, POLLERR just indicates an error condition, of any
kind.
Ack, but the current semantics for how this works with usbfs are
somewhat sub-optimal from a libusb pov. The fix for this is easy
since libusb already has code in place to deal with older kernels
which do not support REAP after disconnect at all and I plan to
use that (rather then wait if we hit the race), but it does
make the whole REAP after disconnect function a whole lot
less useful.
Regards,
Hans
--
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
