lkml.org 
[lkml]   [2005]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] drivers/usb/devio.c, against ioctl bug in 2.4.28 & 2.4.29
Hi!
Here is the tested patch against modem_run and eciadsl hang since 2.4.28.
Longer discussion about it is in:
http://sourceforge.net/mailarchive/forum.php?thread_id=6054671&forum_id=5398
and feedback from users is in:
http://www.mail-archive.com/speedtouch%40ml.free.fr/msg06848.html
The patch itself is also located in:
http://linux.ee/~kaups/devio.patch

It:
- prevent grabbing exclusive_access mutex for ioctls that doesn't need it
- prevent grabbing exclusive_access mutex for non existing ioctls
- use interruptible sleep instead uninterruptible

PS. keep me in CC since I'm not subscribed...
--
best regards,
Kaupo Arulo--- devio.c.orig 2004-11-28 22:24:49.000000000 +0200
+++ devio.c 2004-12-01 12:47:02.000000000 +0200
@@ -1153,45 +1153,62 @@ static int usbdev_ioctl(struct inode *in

if (!(file->f_mode & FMODE_WRITE))
return -EPERM;
- down_read(&ps->devsem);
+ down_read(&ps->devsem); /* FIXME: should we set devsem also per "case"
+ like exclusive_access to avoid
+ blocking nonexistent ioctls ? */
if (!ps->dev) {
up_read(&ps->devsem);
return -ENODEV;
}
-
- /*
- * grab device's exclusive_access mutex to prevent its driver from
- * using this device while it is being accessed by us.
+ /*
+ * Some ioctls don't touch the device and can be called without
+ * grabbing its exclusive_access mutex; they are handled together
+ * in same switch with ioctls which need it. Exclusive_access is handled in
+ * particular switch branches, so we grab device's exclusive_access
+ * mutex ONLY if needed and WHEN actually needed!!!
*/
- down(&ps->dev->exclusive_access);
-
switch (cmd) {
case USBDEVFS_CONTROL:
- ret = proc_control(ps, (void *)arg);
- if (ret >= 0)
- inode->i_mtime = CURRENT_TIME;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_control(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ if (ret >= 0)
+ inode->i_mtime = CURRENT_TIME;
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_BULK:
- ret = proc_bulk(ps, (void *)arg);
- if (ret >= 0)
- inode->i_mtime = CURRENT_TIME;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_bulk(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ if (ret >= 0)
+ inode->i_mtime = CURRENT_TIME;
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_RESETEP:
- ret = proc_resetep(ps, (void *)arg);
- if (ret >= 0)
- inode->i_mtime = CURRENT_TIME;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_resetep(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ if (ret >= 0)
+ inode->i_mtime = CURRENT_TIME;
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_RESET:
- ret = proc_resetdevice(ps);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_resetdevice(ps);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_CLEAR_HALT:
- ret = proc_clearhalt(ps, (void *)arg);
- if (ret >= 0)
- inode->i_mtime = CURRENT_TIME;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_clearhalt(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ if (ret >= 0)
+ inode->i_mtime = CURRENT_TIME;
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_GETDRIVER:
@@ -1203,21 +1220,33 @@ static int usbdev_ioctl(struct inode *in
break;

case USBDEVFS_SETINTERFACE:
- ret = proc_setintf(ps, (void *)arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_setintf(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_SETCONFIGURATION:
- ret = proc_setconfig(ps, (void *)arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_setconfig(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_SUBMITURB:
- ret = proc_submiturb(ps, (void *)arg);
- if (ret >= 0)
- inode->i_mtime = CURRENT_TIME;
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_submiturb(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ if (ret >= 0)
+ inode->i_mtime = CURRENT_TIME;
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_DISCARDURB:
- ret = proc_unlinkurb(ps, (void *)arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_unlinkurb(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_REAPURB:
@@ -1233,18 +1262,26 @@ static int usbdev_ioctl(struct inode *in
break;

case USBDEVFS_CLAIMINTERFACE:
- ret = proc_claiminterface(ps, (void *)arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_claiminterface(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_RELEASEINTERFACE:
- ret = proc_releaseinterface(ps, (void *)arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_releaseinterface(ps, (void *)arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;

case USBDEVFS_IOCTL:
- ret = proc_ioctl(ps, (void *) arg);
+ if (down_interruptible(&ps->dev->exclusive_access) == 0) {
+ ret = proc_ioctl(ps, (void *) arg);
+ up(&ps->dev->exclusive_access);
+ } else ret = -ERESTARTSYS;
break;
}
- up(&ps->dev->exclusive_access);
up_read(&ps->devsem);
if (ret >= 0)
inode->i_atime = CURRENT_TIME;
\
 
 \ /
  Last update: 2005-03-22 14:09    [W:2.059 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site