lkml.org 
[lkml]   [2008]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RESEND][RFC][PATCH] leds: fix oops race in led trigger registration
    On Thu, 21 Aug 2008 09:06:54 +0100
    Richard Purdie <rpurdie@rpsys.net> wrote:

    > Hi,
    >
    > On Thu, 2008-08-21 at 09:17 +0200, Vegard Nossum wrote:
    > > I resubmit this, which was posted a month ago, with no responses from
    > > the maintainer (and no recent fixes in this area either). I have not
    > > tested it myself, but I am reasonably confident that this is an
    > > obviously correct fix. I guess it would not hurt to exercise it as well.
    > >
    > > Zdenek: Have recent kernels also been showing this behaviour? Did the
    > > patch fix the problem for you? You said something about a new issue
    > > showing up, do you think that was related to this patch?
    > >
    > > Also, just one question (since I'm newbie at this): Is it dangerous to
    > > register the &dev_attr_trigger device file before we put the device on
    > > the list or initialize it completely? If so, the initialization should
    > > probably be moved all to the top before the file is made public.
    >
    > This patch isn't quite right since whilst it fixes one race, it opens up
    > a number of other potential problems. We definitely should have the led
    > listed in leds_list before calling the various functions and the
    > existing init order is correct in that sense.
    >
    > I suspect your problem is caused by something calling a trigger
    > registration between the point its added to the list and the
    > init_rwsem(). The best solution is to move the init_rwsem() to earlier
    > in the function. Yes, that means some ifdef ugliness but so be it. Could
    > you see whether you still see the problem with the patch below?
    >
    > Cheers,
    >
    > Richard
    >
    >
    > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
    > index 559a408..c9d8219 100644
    > --- a/drivers/leds/led-class.c
    > +++ b/drivers/leds/led-class.c
    > @@ -113,6 +113,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
    > if (rc)
    > goto err_out;
    >
    > +#ifdef CONFIG_LEDS_TRIGGERS
    > + init_rwsem(&led_cdev->trigger_lock);
    > +#endif
    > /* add to the list of leds */
    > down_write(&leds_list_lock);
    > list_add_tail(&led_cdev->node, &leds_list);
    > @@ -121,8 +124,6 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
    > led_update_brightness(led_cdev);
    >
    > #ifdef CONFIG_LEDS_TRIGGERS
    > - init_rwsem(&led_cdev->trigger_lock);
    > -
    > rc = device_create_file(led_cdev->dev, &dev_attr_trigger);
    > if (rc)
    > goto err_out_led_list;
    >

    ??



    \
     
     \ /
      Last update: 2008-08-26 23:41    [W:6.296 / U:0.364 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site