lkml.org 
[lkml]   [2010]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] LEDS: fix race between LED device uevent and actual attributes creation
Date
Hi Richard,

For some reason, this version appears to work, while it actually should not
because device_add_attributes has no mean to stop iterating over
leds_class_attrs members.

Also, a LED device removal would have made the device attributes be really
removed in led_classdev_unregister, and then tried to be removed a second time
by device_remove_attr. Since we never check the return value of
device_remove_file and the underlying calls which are made to the syfs
functions, no error is produced, but this is actually wrong.

I just resent a new version which really fix the issue and is cleaner. Sorry
for the noise.

On Wednesday 10 March 2010 15:11:44 Fainelli wrote:
> If we were to dynamically register/unregister leds and have udev or other
> daemons handle the leds class uevents, we would be notified of the adding
> of a new LED and if the daemon immediately tries to open one of the
> attributes of the led device, it would fail with a "no such file or
> directory" error since the attributes are not yet created. Fix this by
> switching attributes to be class-wide, such that the driver core will
> register these attributes with device_add_attrs and then emit the
> kobject_uevent ADD signal.
>
> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
> CC: stable@kernel.org
> ---
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 782f958..0be0310 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -78,6 +78,14 @@ static DEVICE_ATTR(max_brightness, 0444,
> led_max_brightness_show, NULL); static DEVICE_ATTR(trigger, 0644,
> led_trigger_show, led_trigger_store); #endif
>
> +static struct device_attribute *leds_class_attrs[] = {
> + &dev_attr_brightness,
> + &dev_attr_max_brightness,
> +#ifdef CONFIG_LEDS_TRIGGERS
> + &dev_attr_trigger,
> +#endif
> +};
> +
> /**
> * led_classdev_suspend - suspend an led_classdev.
> * @led_cdev: the led_classdev to suspend.
> @@ -127,18 +135,11 @@ static int led_resume(struct device *dev)
> */
> int led_classdev_register(struct device *parent, struct led_classdev
> *led_cdev) {
> - int rc;
> -
> led_cdev->dev = device_create(leds_class, parent, 0, led_cdev,
> "%s", led_cdev->name);
> if (IS_ERR(led_cdev->dev))
> return PTR_ERR(led_cdev->dev);
>
> - /* register the attributes */
> - rc = device_create_file(led_cdev->dev, &dev_attr_brightness);
> - if (rc)
> - goto err_out;
> -
> #ifdef CONFIG_LEDS_TRIGGERS
> init_rwsem(&led_cdev->trigger_lock);
> #endif
> @@ -150,17 +151,9 @@ int led_classdev_register(struct device *parent,
> struct led_classdev *led_cdev) if (!led_cdev->max_brightness)
> led_cdev->max_brightness = LED_FULL;
>
> - rc = device_create_file(led_cdev->dev, &dev_attr_max_brightness);
> - if (rc)
> - goto err_out_attr_max;
> -
> led_update_brightness(led_cdev);
>
> #ifdef CONFIG_LEDS_TRIGGERS
> - rc = device_create_file(led_cdev->dev, &dev_attr_trigger);
> - if (rc)
> - goto err_out_led_list;
> -
> led_trigger_set_default(led_cdev);
> #endif
>
> @@ -168,18 +161,8 @@ int led_classdev_register(struct device *parent,
> struct led_classdev *led_cdev) led_cdev->name);
>
> return 0;
> -
> -#ifdef CONFIG_LEDS_TRIGGERS
> -err_out_led_list:
> - device_remove_file(led_cdev->dev, &dev_attr_max_brightness);
> -#endif
> -err_out_attr_max:
> - device_remove_file(led_cdev->dev, &dev_attr_brightness);
> - list_del(&led_cdev->node);
> -err_out:
> - device_unregister(led_cdev->dev);
> - return rc;
> }
> +
> EXPORT_SYMBOL_GPL(led_classdev_register);
>
> /**
> @@ -215,6 +198,7 @@ static int __init leds_init(void)
> return PTR_ERR(leds_class);
> leds_class->suspend = led_suspend;
> leds_class->resume = led_resume;
> + leds_class->dev_attrs = *leds_class_attrs;
> return 0;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>



\
 
 \ /
  Last update: 2010-03-10 18:37    [W:0.078 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site