Messages in this thread | | | Date | Fri, 11 Aug 2006 09:34:44 -0400 | From | "Dmitry Torokhov" <> | Subject | Re: [patch 5/6] Convert to use mutexes instead of semaphores |
| |
On 8/11/06, Richard Purdie <rpurdie@rpsys.net> wrote: > On Fri, 2006-08-11 at 08:58 -0400, Dmitry Torokhov wrote: > > On 8/11/06, Dmitry Torokhov <dtor@insightbb.com> 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 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/
| |