On 8/11/06, Richard Purdie <[email protected]> wrote:
On Fri, 2006-08-11 at 08:58 -0400, Dmitry Torokhov wrote:
> On 8/11/06, Dmitry Torokhov <[email protected]> wrote:
> > Backlight: convert to use mutexes instead of semaphores
> >
>
> Apparently I missed that several drivers also use bd->sem so they need
> to be converted too... But what is it with the drivers:
>
> static void aty128_bl_set_power(struct fb_info *info, int power)
> {
> mutex_lock(&info->bl_mutex);
> up(&info->bl_dev->sem);
> info->bl_dev->props->power = power;
> __aty128_bl_update_status(info->bl_dev);
> down(&info->bl_dev->sem);
> mutex_unlock(&info->bl_mutex);
> }
>
> Why we are doing up() before down()??? And it is in almost every
> driver that uses backlight... Do I need more coffee? [CC-ing bunch of
> people trying to get an answer...]
It looks totally wrong.
Ok, so that is not only me seeing things ;)
In the archives, there are a number of comments from me questioning
whether that driver needs to touch bl_dev->sem anyway (esp. given the
mutex as well). I never did find out what it was trying to protect
against...
I think it is prudent to protect assess to these data structures. For
example, it could possibly race with setting power through sysfs
attribute. Now for this particular driver the race window is
non-existent for all practical purposes, but it looks like atyfb_base
might be needig it (of course currentimplementation not only have
up/down mixed but also has AB-BA deadlock).
How about we add backlight_set_power(&bd, power) to the backlight core
to take care of proper locking for drivers?
--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
