From 1037246cacd45d951227c8798f181b3ba5c8bcbe Mon Sep 17 00:00:00 2001 From: Kulikov Vasiliy Date: Tue, 3 Aug 2010 19:44:23 +0400 Subject: uio: do not use PCI resources before pci_enable_device() IRQ and resource[] may not have correct values until after PCI hotplug setup occurs at pci_enable_device() time. The semantic match that finds this problem is as follows: // @@ identifier x; identifier request ~= "pci_request.*|pci_resource.*"; @@ ( * x->irq | * x->resource | * request(x, ...) ) ... *pci_enable_device(x) // Signed-off-by: Kulikov Vasiliy Acked-by: Michael S. Tsirkin Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio_pci_generic.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c index 85c9884a67fd..fc22e1e6f215 100644 --- a/drivers/uio/uio_pci_generic.c +++ b/drivers/uio/uio_pci_generic.c @@ -128,12 +128,6 @@ static int __devinit probe(struct pci_dev *pdev, struct uio_pci_generic_dev *gdev; int err; - if (!pdev->irq) { - dev_warn(&pdev->dev, "No IRQ assigned to device: " - "no support for interrupts?\n"); - return -ENODEV; - } - err = pci_enable_device(pdev); if (err) { dev_err(&pdev->dev, "%s: pci_enable_device failed: %d\n", @@ -141,6 +135,13 @@ static int __devinit probe(struct pci_dev *pdev, return err; } + if (!pdev->irq) { + dev_warn(&pdev->dev, "No IRQ assigned to device: " + "no support for interrupts?\n"); + pci_disable_device(pdev); + return -ENODEV; + } + err = verify_pci_2_3(pdev); if (err) goto err_verify; -- cgit v1.2.3-59-g8ed1b From 3d4f9d76b0641b7984f95982e390927fc5998ad6 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 14 Sep 2010 11:36:27 -0700 Subject: uio: Fix lack of locking in init_uio_class There is no locking in init_uio_class so multiple drivers can race and create multiple uio classes. Fix this by simplifying the code. In particular always register the uio class during module_init and make things simpler. Signed-off-by: Eric W. Biederman Reviewed-by: Thomas Gleixner Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio.c | 58 ++++++++++++++----------------------------------------- 1 file changed, 15 insertions(+), 43 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index bff1afbde5a4..bc774cce0a4d 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -45,10 +45,7 @@ static DEFINE_IDR(uio_idr); static const struct file_operations uio_fops; /* UIO class infrastructure */ -static struct uio_class { - struct kref kref; - struct class *class; -} *uio_class; +static struct class *uio_class; /* Protect idr accesses */ static DEFINE_MUTEX(minor_lock); @@ -757,55 +754,35 @@ static void uio_major_cleanup(void) static int init_uio_class(void) { - int ret = 0; - - if (uio_class != NULL) { - kref_get(&uio_class->kref); - goto exit; - } + struct class *class; + int ret; /* This is the first time in here, set everything up properly */ ret = uio_major_init(); if (ret) goto exit; - uio_class = kzalloc(sizeof(*uio_class), GFP_KERNEL); - if (!uio_class) { - ret = -ENOMEM; - goto err_kzalloc; - } - - kref_init(&uio_class->kref); - uio_class->class = class_create(THIS_MODULE, "uio"); - if (IS_ERR(uio_class->class)) { - ret = IS_ERR(uio_class->class); + class = class_create(THIS_MODULE, "uio"); + if (IS_ERR(class)) { + ret = IS_ERR(class); printk(KERN_ERR "class_create failed for uio\n"); goto err_class_create; } + uio_class = class; return 0; err_class_create: - kfree(uio_class); - uio_class = NULL; -err_kzalloc: uio_major_cleanup(); exit: return ret; } -static void release_uio_class(struct kref *kref) +static void release_uio_class(void) { /* Ok, we cheat as we know we only have one uio_class */ - class_destroy(uio_class->class); - kfree(uio_class); - uio_major_cleanup(); + class_destroy(uio_class); uio_class = NULL; -} - -static void uio_class_destroy(void) -{ - if (uio_class) - kref_put(&uio_class->kref, release_uio_class); + uio_major_cleanup(); } /** @@ -828,10 +805,6 @@ int __uio_register_device(struct module *owner, info->uio_dev = NULL; - ret = init_uio_class(); - if (ret) - return ret; - idev = kzalloc(sizeof(*idev), GFP_KERNEL); if (!idev) { ret = -ENOMEM; @@ -847,7 +820,7 @@ int __uio_register_device(struct module *owner, if (ret) goto err_get_minor; - idev->dev = device_create(uio_class->class, parent, + idev->dev = device_create(uio_class, parent, MKDEV(uio_major, idev->minor), idev, "uio%d", idev->minor); if (IS_ERR(idev->dev)) { @@ -874,13 +847,12 @@ int __uio_register_device(struct module *owner, err_request_irq: uio_dev_del_attributes(idev); err_uio_dev_add_attributes: - device_destroy(uio_class->class, MKDEV(uio_major, idev->minor)); + device_destroy(uio_class, MKDEV(uio_major, idev->minor)); err_device_create: uio_free_minor(idev); err_get_minor: kfree(idev); err_kzalloc: - uio_class_destroy(); return ret; } EXPORT_SYMBOL_GPL(__uio_register_device); @@ -907,9 +879,8 @@ void uio_unregister_device(struct uio_info *info) uio_dev_del_attributes(idev); dev_set_drvdata(idev->dev, NULL); - device_destroy(uio_class->class, MKDEV(uio_major, idev->minor)); + device_destroy(uio_class, MKDEV(uio_major, idev->minor)); kfree(idev); - uio_class_destroy(); return; } @@ -917,11 +888,12 @@ EXPORT_SYMBOL_GPL(uio_unregister_device); static int __init uio_init(void) { - return 0; + return init_uio_class(); } static void __exit uio_exit(void) { + release_uio_class(); } module_init(uio_init) -- cgit v1.2.3-59-g8ed1b From 70a9156bad9d9d1476df35dde582b9f411bf5914 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 14 Sep 2010 11:36:54 -0700 Subject: uio: Don't clear driver data Currently uio sets it's driver data to NULL just as it is unregistering attributes. sysfs maks the guaranatee that it will not call attributes after device_destroy is called so this is unncessary and leads to lots of unnecessary code in uio.c Signed-off-by: Eric W. Biederman Reviewed-by: Thomas Gleixner Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index bc774cce0a4d..8132288920b2 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -229,10 +229,7 @@ static ssize_t show_name(struct device *dev, struct device_attribute *attr, char *buf) { struct uio_device *idev = dev_get_drvdata(dev); - if (idev) - return sprintf(buf, "%s\n", idev->info->name); - else - return -ENODEV; + return sprintf(buf, "%s\n", idev->info->name); } static DEVICE_ATTR(name, S_IRUGO, show_name, NULL); @@ -240,10 +237,7 @@ static ssize_t show_version(struct device *dev, struct device_attribute *attr, char *buf) { struct uio_device *idev = dev_get_drvdata(dev); - if (idev) - return sprintf(buf, "%s\n", idev->info->version); - else - return -ENODEV; + return sprintf(buf, "%s\n", idev->info->version); } static DEVICE_ATTR(version, S_IRUGO, show_version, NULL); @@ -251,11 +245,7 @@ static ssize_t show_event(struct device *dev, struct device_attribute *attr, char *buf) { struct uio_device *idev = dev_get_drvdata(dev); - if (idev) - return sprintf(buf, "%u\n", - (unsigned int)atomic_read(&idev->event)); - else - return -ENODEV; + return sprintf(buf, "%u\n", (unsigned int)atomic_read(&idev->event)); } static DEVICE_ATTR(event, S_IRUGO, show_event, NULL); @@ -878,7 +868,6 @@ void uio_unregister_device(struct uio_info *info) uio_dev_del_attributes(idev); - dev_set_drvdata(idev->dev, NULL); device_destroy(uio_class, MKDEV(uio_major, idev->minor)); kfree(idev); -- cgit v1.2.3-59-g8ed1b From 6427a7655afd7f07dfa83736defd1d94656c83e5 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 14 Sep 2010 11:37:36 -0700 Subject: uio: Cleanup irq handling. Change the value of UIO_IRQ_NONE -2 to 0. 0 is well defined in the rest of the kernel as the value to indicate an irq has not been assigned. Update the calls to request_irq and free_irq to only ignore UIO_IRQ_NONE and UIO_IRQ_CUSTOM allowing the rest of the kernel's possible irq numbers to be used. Signed-off-by: Eric W. Biederman Reviewed-by: Thomas Gleixner Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio.c | 14 +++++++------- include/linux/uio_driver.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 8132288920b2..0fd2cda74244 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -512,7 +512,7 @@ static unsigned int uio_poll(struct file *filep, poll_table *wait) struct uio_listener *listener = filep->private_data; struct uio_device *idev = listener->dev; - if (idev->info->irq == UIO_IRQ_NONE) + if (!idev->info->irq) return -EIO; poll_wait(filep, &idev->wait, wait); @@ -530,7 +530,7 @@ static ssize_t uio_read(struct file *filep, char __user *buf, ssize_t retval; s32 event_count; - if (idev->info->irq == UIO_IRQ_NONE) + if (!idev->info->irq) return -EIO; if (count != sizeof(s32)) @@ -578,7 +578,7 @@ static ssize_t uio_write(struct file *filep, const char __user *buf, ssize_t retval; s32 irq_on; - if (idev->info->irq == UIO_IRQ_NONE) + if (!idev->info->irq) return -EIO; if (count != sizeof(s32)) @@ -825,9 +825,9 @@ int __uio_register_device(struct module *owner, info->uio_dev = idev; - if (idev->info->irq >= 0) { - ret = request_irq(idev->info->irq, uio_interrupt, - idev->info->irq_flags, idev->info->name, idev); + if (info->irq && (info->irq != UIO_IRQ_CUSTOM)) { + ret = request_irq(info->irq, uio_interrupt, + info->irq_flags, info->name, idev); if (ret) goto err_request_irq; } @@ -863,7 +863,7 @@ void uio_unregister_device(struct uio_info *info) uio_free_minor(idev); - if (info->irq >= 0) + if (info->irq && (info->irq != UIO_IRQ_CUSTOM)) free_irq(info->irq, idev); uio_dev_del_attributes(idev); diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index 5dcc9ff72f69..d6188e5a52df 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -108,7 +108,7 @@ extern void uio_event_notify(struct uio_info *info); /* defines for uio_info->irq */ #define UIO_IRQ_CUSTOM -1 -#define UIO_IRQ_NONE -2 +#define UIO_IRQ_NONE 0 /* defines for uio_mem->memtype */ #define UIO_MEM_NONE 0 -- cgit v1.2.3-59-g8ed1b From 91960a46c658b719c03fba80f1c60a96393bbcfd Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 14 Sep 2010 11:38:06 -0700 Subject: uio: Support 2^MINOR_BITS minors register_chrdev limits uio devices to 256 minor numbers which causes problems on one system I have with 384+ uio devices. So instead set UIO_MAX_DEVICES to the maximum number of minors and use alloc_chrdev_region to reserve the uio minors. The final result is that the code works the same but the uio driver now supports any minor the idr allocator comes up with. Signed-off-by: Eric W. Biederman Reviewed-by: Thomas Gleixner Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio.c | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 0fd2cda74244..3d4d65b0626f 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -23,9 +23,10 @@ #include #include #include +#include #include -#define UIO_MAX_DEVICES 255 +#define UIO_MAX_DEVICES (1U << MINORBITS) struct uio_device { struct module *owner; @@ -41,6 +42,7 @@ struct uio_device { }; static int uio_major; +static struct cdev *uio_cdev; static DEFINE_IDR(uio_idr); static const struct file_operations uio_fops; @@ -731,15 +733,44 @@ static const struct file_operations uio_fops = { static int uio_major_init(void) { - uio_major = register_chrdev(0, "uio", &uio_fops); - if (uio_major < 0) - return uio_major; - return 0; + static const char name[] = "uio"; + struct cdev *cdev = NULL; + dev_t uio_dev = 0; + int result; + + result = alloc_chrdev_region(&uio_dev, 0, UIO_MAX_DEVICES, name); + if (result) + goto out; + + result = -ENOMEM; + cdev = cdev_alloc(); + if (!cdev) + goto out_unregister; + + cdev->owner = THIS_MODULE; + cdev->ops = &uio_fops; + kobject_set_name(&cdev->kobj, "%s", name); + + result = cdev_add(cdev, uio_dev, UIO_MAX_DEVICES); + if (result) + goto out_put; + + uio_major = MAJOR(uio_dev); + uio_cdev = cdev; + result = 0; +out: + return result; +out_put: + kobject_put(&cdev->kobj); +out_unregister: + unregister_chrdev_region(uio_dev, UIO_MAX_DEVICES); + goto out; } static void uio_major_cleanup(void) { - unregister_chrdev(uio_major, "uio"); + unregister_chrdev_region(MKDEV(uio_major, 0), UIO_MAX_DEVICES); + cdev_del(uio_cdev); } static int init_uio_class(void) -- cgit v1.2.3-59-g8ed1b From c66fdab64fd791bdd49fed4f5785643251ddf586 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 14 Sep 2010 11:38:36 -0700 Subject: uio: Statically allocate uio_class and use class .dev_attrs. Instead of adding uio class attributes manually after the uio device has been created and we have sent a uevent to userspace, use the class attribute mechanism. This removes races and makes the code simpler. At the same time don't bother to dynamically allocate a struct class for uio, just declare one statically. Less code is needed and it is easier to set the class parameters.tune the class Signed-off-by: Eric W. Biederman Reviewed-by: Thomas Gleixner Signed-off-by: Hans J. Koch Signed-off-by: Greg Kroah-Hartman --- drivers/uio/uio.c | 53 ++++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 35 deletions(-) (limited to 'drivers/uio') diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index 3d4d65b0626f..3fe9a9da42d1 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -46,9 +46,6 @@ static struct cdev *uio_cdev; static DEFINE_IDR(uio_idr); static const struct file_operations uio_fops; -/* UIO class infrastructure */ -static struct class *uio_class; - /* Protect idr accesses */ static DEFINE_MUTEX(minor_lock); @@ -233,7 +230,6 @@ static ssize_t show_name(struct device *dev, struct uio_device *idev = dev_get_drvdata(dev); return sprintf(buf, "%s\n", idev->info->name); } -static DEVICE_ATTR(name, S_IRUGO, show_name, NULL); static ssize_t show_version(struct device *dev, struct device_attribute *attr, char *buf) @@ -241,7 +237,6 @@ static ssize_t show_version(struct device *dev, struct uio_device *idev = dev_get_drvdata(dev); return sprintf(buf, "%s\n", idev->info->version); } -static DEVICE_ATTR(version, S_IRUGO, show_version, NULL); static ssize_t show_event(struct device *dev, struct device_attribute *attr, char *buf) @@ -249,17 +244,18 @@ static ssize_t show_event(struct device *dev, struct uio_device *idev = dev_get_drvdata(dev); return sprintf(buf, "%u\n", (unsigned int)atomic_read(&idev->event)); } -static DEVICE_ATTR(event, S_IRUGO, show_event, NULL); -static struct attribute *uio_attrs[] = { - &dev_attr_name.attr, - &dev_attr_version.attr, - &dev_attr_event.attr, - NULL, +static struct device_attribute uio_class_attributes[] = { + __ATTR(name, S_IRUGO, show_name, NULL), + __ATTR(version, S_IRUGO, show_version, NULL), + __ATTR(event, S_IRUGO, show_event, NULL), + {} }; -static struct attribute_group uio_attr_grp = { - .attrs = uio_attrs, +/* UIO class infrastructure */ +static struct class uio_class = { + .name = "uio", + .dev_attrs = uio_class_attributes, }; /* @@ -276,10 +272,6 @@ static int uio_dev_add_attributes(struct uio_device *idev) struct uio_port *port; struct uio_portio *portio; - ret = sysfs_create_group(&idev->dev->kobj, &uio_attr_grp); - if (ret) - goto err_group; - for (mi = 0; mi < MAX_UIO_MAPS; mi++) { mem = &idev->info->mem[mi]; if (mem->size == 0) @@ -347,8 +339,6 @@ err_map: kobject_put(&map->kobj); } kobject_put(idev->map_dir); - sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp); -err_group: dev_err(idev->dev, "error creating sysfs files (%d)\n", ret); return ret; } @@ -374,8 +364,6 @@ static void uio_dev_del_attributes(struct uio_device *idev) kobject_put(&port->portio->kobj); } kobject_put(idev->portio_dir); - - sysfs_remove_group(&idev->dev->kobj, &uio_attr_grp); } static int uio_get_minor(struct uio_device *idev) @@ -775,7 +763,6 @@ static void uio_major_cleanup(void) static int init_uio_class(void) { - struct class *class; int ret; /* This is the first time in here, set everything up properly */ @@ -783,16 +770,14 @@ static int init_uio_class(void) if (ret) goto exit; - class = class_create(THIS_MODULE, "uio"); - if (IS_ERR(class)) { - ret = IS_ERR(class); - printk(KERN_ERR "class_create failed for uio\n"); - goto err_class_create; + ret = class_register(&uio_class); + if (ret) { + printk(KERN_ERR "class_register failed for uio\n"); + goto err_class_register; } - uio_class = class; return 0; -err_class_create: +err_class_register: uio_major_cleanup(); exit: return ret; @@ -800,9 +785,7 @@ exit: static void release_uio_class(void) { - /* Ok, we cheat as we know we only have one uio_class */ - class_destroy(uio_class); - uio_class = NULL; + class_unregister(&uio_class); uio_major_cleanup(); } @@ -841,7 +824,7 @@ int __uio_register_device(struct module *owner, if (ret) goto err_get_minor; - idev->dev = device_create(uio_class, parent, + idev->dev = device_create(&uio_class, parent, MKDEV(uio_major, idev->minor), idev, "uio%d", idev->minor); if (IS_ERR(idev->dev)) { @@ -868,7 +851,7 @@ int __uio_register_device(struct module *owner, err_request_irq: uio_dev_del_attributes(idev); err_uio_dev_add_attributes: - device_destroy(uio_class, MKDEV(uio_major, idev->minor)); + device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); err_device_create: uio_free_minor(idev); err_get_minor: @@ -899,7 +882,7 @@ void uio_unregister_device(struct uio_info *info) uio_dev_del_attributes(idev); - device_destroy(uio_class, MKDEV(uio_major, idev->minor)); + device_destroy(&uio_class, MKDEV(uio_major, idev->minor)); kfree(idev); return; -- cgit v1.2.3-59-g8ed1b