Messages in this thread Patch in this message | | | Date | Fri, 21 Jan 2005 01:45:23 +0200 (EET) | From | Kaupo Arulo <> | 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; | |