On Tue, Nov 30, 2004 at 04:35:29PM +0200, Meelis Roos wrote: > >This patch (compile tested only) moves handling of USBDEVFS_GETDRIVER, > >USBDEVFS_CONNECTINFO, USBDEVFS_REAPURB, USBDEVFS_REAPURBNDELAY, > >USBDEVFS_DISCSIGNAL out of the ps->dev->exclusive_access lock. > > Well, it takes the ps->dev->exclusive_access also to see that the > ioctl does not exist. Someone who checks ioctls of different kernels to > see which succeeds might get into a livelock with this.
I'm not sure if this is really important. At least such process will
not block anything other for a long time. And avoiding the lock for
an unknown ioctl will lead to lots of duplicated code (locking will be
pushed to the lower level and duplicated for all switch branches) -
will this really be better?
However, that down(&ps->dev->exclusive_access) should probably be
down_interruptible() instead - unkillable processes are bad. This
patch is on top of my previous patch.
=======================================================================
Summary: [USB] usbfs: make the wait for exclusive_access interruptible
Signed-off-by: Sergey Vlasov <[EMAIL PROTECTED]>
down(&ps->dev->exclusive_access) can sleep for a long time (e.g., when
some blocking usbfs operation like USBDEVFS_BULK is in progress).
Having an uninterruptible sleep here is not nice.
diff -urNp -x'*~' linux-2.4.29-pre1-usb-2/drivers/usb/devio.c
linux-2.4.29-pre1-usb-2-1/drivers/usb/devio.c
--- linux-2.4.29-pre1-usb-2/drivers/usb/devio.c 2004-11-30 14:58:26 +0300
+++ linux-2.4.29-pre1-usb-2-1/drivers/usb/devio.c 2004-11-30 18:11:35
+0300
@@ -1255,9 +1255,11 @@ static int usbdev_ioctl(struct inode *in
break;
default:
- down(&ps->dev->exclusive_access);
- ret = usbdev_ioctl_exclusive(ps, inode, cmd, arg);
- up(&ps->dev->exclusive_access);
+ ret = -ERESTARTSYS;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = usbdev_ioctl_exclusive(ps, inode, cmd, arg);
+ up(&ps->dev->exclusive_access);
+ }
break;
}
up_read(&ps->devsem);
pgpU1PO34uhMm.pgp
Description: PGP signature
