From 8e18e2941c53416aa219708e7dcad21fb4bd6794 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Wed, 27 Sep 2006 01:50:46 -0700 Subject: [PATCH] inode_diet: Replace inode.u.generic_ip with inode.i_private The following patches reduce the size of the VFS inode structure by 28 bytes on a UP x86. (It would be more on an x86_64 system). This is a 10% reduction in the inode size on a UP kernel that is configured in a production mode (i.e., with no spinlock or other debugging functions enabled; if you want to save memory taken up by in-core inodes, the first thing you should do is disable the debugging options; they are responsible for a huge amount of bloat in the VFS inode structure). This patch: The filesystem or device-specific pointer in the inode is inside a union, which is pretty pointless given that all 30+ users of this field have been using the void pointer. Get rid of the union and rename it to i_private, with a comment to explain who is allowed to use the void pointer. This is just a cleanup, but it allows us to reuse the union 'u' for something something where the union will actually be used. [judith@osdl.org: powerpc build fix] Signed-off-by: "Theodore Ts'o" Signed-off-by: Judith Lebzelter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/usb/core/devio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 218621b..32e0300 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -555,7 +555,7 @@ static int usbdev_open(struct inode *inode, struct file *file) if (imajor(inode) == USB_DEVICE_MAJOR) dev = usbdev_lookup_minor(iminor(inode)); if (!dev) - dev = inode->u.generic_ip; + dev = inode->i_private; if (!dev) { kfree(ps); goto out; -- cgit v1.1 From 4a2a8a2cce86b9d001378cc25acb5c61e6ca7d63 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Sat, 1 Jul 2006 22:05:01 -0400 Subject: usbfs: private mutex for open, release, and remove The usbfs code doesn't provide sufficient mutual exclusion among open, release, and remove. Release vs. remove is okay because they both acquire the device lock, but open is not exclusive with either one. All three routines modify the udev->filelist linked list, so they must not run concurrently. Apparently someone gave this a minimum amount of thought in the past by explicitly acquiring the BKL at the start of the usbdev_open routine. Oddly enough, there's a comment pointing out that locking is unnecessary because chrdev_open already has acquired the BKL. But this ignores the point that the files in /proc/bus/usb/* are not char device files; they are regular files and so they don't get any special locking. Furthermore it's necessary to acquire the same lock in the release and remove routines, which the code does not do. Yet another problem arises because the same file_operations structure is accessible through both the /proc/bus/usb/* and /dev/usb/usbdev* file nodes. Even when one of them has been removed, it's still possible for userspace to open the other. So simple locking around the individual remove routines is insufficient; we need to lock the entire usb_notify_remove_device notifier chain. Rather than rely on the BKL, this patch (as723) introduces a new private mutex for the purpose. Holding the BKL while invoking a notifier chain doesn't seem like a good idea. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 32e0300..d8b0476 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -59,6 +59,9 @@ #define USB_DEVICE_MAX USB_MAXBUS * 128 static struct class *usb_device_class; +/* Mutual exclusion for removal, open, and release */ +DEFINE_MUTEX(usbfs_mutex); + struct async { struct list_head asynclist; struct dev_state *ps; @@ -541,15 +544,13 @@ static int usbdev_open(struct inode *inode, struct file *file) struct dev_state *ps; int ret; - /* - * no locking necessary here, as chrdev_open has the kernel lock - * (still acquire the kernel lock for safety) - */ + /* Protect against simultaneous removal or release */ + mutex_lock(&usbfs_mutex); + ret = -ENOMEM; if (!(ps = kmalloc(sizeof(struct dev_state), GFP_KERNEL))) - goto out_nolock; + goto out; - lock_kernel(); ret = -ENOENT; /* check if we are called from a real node or usbfs */ if (imajor(inode) == USB_DEVICE_MAJOR) @@ -579,9 +580,8 @@ static int usbdev_open(struct inode *inode, struct file *file) list_add_tail(&ps->list, &dev->filelist); file->private_data = ps; out: - unlock_kernel(); - out_nolock: - return ret; + mutex_unlock(&usbfs_mutex); + return ret; } static int usbdev_release(struct inode *inode, struct file *file) @@ -591,7 +591,12 @@ static int usbdev_release(struct inode *inode, struct file *file) unsigned int ifnum; usb_lock_device(dev); + + /* Protect against simultaneous open */ + mutex_lock(&usbfs_mutex); list_del_init(&ps->list); + mutex_unlock(&usbfs_mutex); + for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed); ifnum++) { if (test_bit(ifnum, &ps->ifclaimed)) @@ -600,9 +605,8 @@ static int usbdev_release(struct inode *inode, struct file *file) destroy_all_async(ps); usb_unlock_device(dev); usb_put_dev(dev); - ps->dev = NULL; kfree(ps); - return 0; + return 0; } static int proc_control(struct dev_state *ps, void __user *arg) -- cgit v1.1 From 349710c3a79c0405911b8b604953f0c665e17756 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Sat, 1 Jul 2006 22:05:56 -0400 Subject: usbfs: detect device unregistration This patch (as711b) is a revised version of an earlier submission. It modifies the usbfs code to detect when a device has been unregistered from usbfs, even if the device is still connected. Although this can't happen now, it will be able to happen after the upcoming changes to usb_generic. Nobody objected to this patch when it was submitted before, so it should be okay to apply this version. The revision is merely to take into account the changes introduced by as723, which touches the same driver. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index d8b0476..e84f19d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -90,9 +90,10 @@ MODULE_PARM_DESC (usbfs_snoop, "true to log all usbfs traffic"); #define MAX_USBFS_BUFFER_SIZE 16384 -static inline int connected (struct usb_device *dev) +static inline int connected (struct dev_state *ps) { - return dev->state != USB_STATE_NOTATTACHED; + return (!list_empty(&ps->list) && + ps->dev->state != USB_STATE_NOTATTACHED); } static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig) @@ -130,7 +131,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l pos = *ppos; usb_lock_device(dev); - if (!connected(dev)) { + if (!connected(ps)) { ret = -ENODEV; goto err; } else if (pos < 0) { @@ -1326,7 +1327,7 @@ static int proc_ioctl(struct dev_state *ps, struct usbdevfs_ioctl *ctl) } } - if (!connected(ps->dev)) { + if (!connected(ps)) { kfree(buf); return -ENODEV; } @@ -1425,7 +1426,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd if (!(file->f_mode & FMODE_WRITE)) return -EPERM; usb_lock_device(dev); - if (!connected(dev)) { + if (!connected(ps)) { usb_unlock_device(dev); return -ENODEV; } @@ -1566,7 +1567,7 @@ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wai poll_wait(file, &ps->wait, wait); if (file->f_mode & FMODE_WRITE && !list_empty(&ps->async_completed)) mask |= POLLOUT | POLLWRNORM; - if (!connected(ps->dev)) + if (!connected(ps)) mask |= POLLERR | POLLHUP; return mask; } -- cgit v1.1 From 066202dd48cf3296b6cc22b5fcf89aef33fa0efc Mon Sep 17 00:00:00 2001 From: "Luiz Fernando N. Capitulino" Date: Sat, 5 Aug 2006 20:37:11 -0300 Subject: USB: Make file operations structs in drivers/usb const. Making structs const prevents accidental bugs and with the proper debug options they're protected against corruption. Signed-off-by: Luiz Fernando N. Capitulino Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index e84f19d..43c0872 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1572,7 +1572,7 @@ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wai return mask; } -struct file_operations usbfs_device_file_operations = { +const struct file_operations usbfs_device_file_operations = { .llseek = usbdev_lseek, .read = usbdev_read, .poll = usbdev_poll, -- cgit v1.1 From 01d883d44a1ca8dc77486635d428cba63e7fdadf Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 30 Aug 2006 15:47:18 -0400 Subject: usbcore: non-hub-specific uses of autosuspend This patch (as741) makes the non-hub parts of usbcore actually use the autosuspend facilities added by an earlier patch. Devices opened through usbfs are autoresumed and then autosuspended upon close. Likewise for usb-skeleton. Devices are autoresumed for usb_set_configuration. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 43c0872..fd345ad 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -558,10 +558,12 @@ static int usbdev_open(struct inode *inode, struct file *file) dev = usbdev_lookup_minor(iminor(inode)); if (!dev) dev = inode->i_private; - if (!dev) { - kfree(ps); + if (!dev) goto out; - } + ret = usb_autoresume_device(dev, 1); + if (ret) + goto out; + usb_get_dev(dev); ret = 0; ps->dev = dev; @@ -581,6 +583,8 @@ static int usbdev_open(struct inode *inode, struct file *file) list_add_tail(&ps->list, &dev->filelist); file->private_data = ps; out: + if (ret) + kfree(ps); mutex_unlock(&usbfs_mutex); return ret; } @@ -604,6 +608,7 @@ static int usbdev_release(struct inode *inode, struct file *file) releaseintf(ps, ifnum); } destroy_all_async(ps); + usb_autosuspend_device(dev, 1); usb_unlock_device(dev); usb_put_dev(dev); kfree(ps); -- cgit v1.1 From 1b21d5e166e104f8914441ef52e2cd50ce65b479 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 28 Aug 2006 11:43:25 -0700 Subject: USB: fix __must_check warnings in drivers/usb/core/ Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index fd345ad..71bbd25 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1359,7 +1359,7 @@ static int proc_ioctl(struct dev_state *ps, struct usbdevfs_ioctl *ctl) /* let kernel drivers try to (re)bind to the interface */ case USBDEVFS_CONNECT: usb_unlock_device(ps->dev); - bus_rescan_devices(intf->dev.bus); + retval = bus_rescan_devices(intf->dev.bus); usb_lock_device(ps->dev); break; -- cgit v1.1 From ec17cf1cfe0b557210b27313bd584e9b5187d4ca Mon Sep 17 00:00:00 2001 From: Tobias Klauser Date: Wed, 13 Sep 2006 21:38:41 +0200 Subject: USB: Remove unneeded void * casts in core files The patch removes unneeded casts for the following (void *) pointers: - struct file: private - struct urb: context - struct usb_bus: hcpriv - return value of kmalloc() The patch also contains some whitespace cleanup in the relevant areas. Signed-off-by: Tobias Klauser Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/usb/core/devio.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 71bbd25..a94c63b 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -122,7 +122,7 @@ static loff_t usbdev_lseek(struct file *file, loff_t offset, int orig) static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) { - struct dev_state *ps = (struct dev_state *)file->private_data; + struct dev_state *ps = file->private_data; struct usb_device *dev = ps->dev; ssize_t ret = 0; unsigned len; @@ -305,7 +305,7 @@ static void snoop_urb(struct urb *urb, void __user *userurb) static void async_completed(struct urb *urb, struct pt_regs *regs) { - struct async *as = (struct async *)urb->context; + struct async *as = urb->context; struct dev_state *ps = as->ps; struct siginfo sinfo; @@ -591,7 +591,7 @@ static int usbdev_open(struct inode *inode, struct file *file) static int usbdev_release(struct inode *inode, struct file *file) { - struct dev_state *ps = (struct dev_state *)file->private_data; + struct dev_state *ps = file->private_data; struct usb_device *dev = ps->dev; unsigned int ifnum; @@ -1423,7 +1423,7 @@ static int proc_ioctl_compat(struct dev_state *ps, compat_uptr_t arg) */ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg) { - struct dev_state *ps = (struct dev_state *)file->private_data; + struct dev_state *ps = file->private_data; struct usb_device *dev = ps->dev; void __user *p = (void __user *)arg; int ret = -ENOTTY; @@ -1566,8 +1566,8 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd /* No kernel lock - fine */ static unsigned int usbdev_poll(struct file *file, struct poll_table_struct *wait) { - struct dev_state *ps = (struct dev_state *)file->private_data; - unsigned int mask = 0; + struct dev_state *ps = file->private_data; + unsigned int mask = 0; poll_wait(file, &ps->wait, wait); if (file->f_mode & FMODE_WRITE && !list_empty(&ps->async_completed)) -- cgit v1.1