| Message ID | 20090705145203.GA8326@linux-sh.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Sun, 5 Jul 2009, Paul Mundt wrote: > break; > fb_info->node = i; > mutex_init(&fb_info->lock); > - mutex_init(&fb_info->mm_lock); Why not "lock" as well? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jul 05, 2009 at 07:56:56AM -0700, Linus Torvalds wrote: > > > On Sun, 5 Jul 2009, Paul Mundt wrote: > > break; > > fb_info->node = i; > > mutex_init(&fb_info->lock); > > - mutex_init(&fb_info->mm_lock); > > Why not "lock" as well? > I had that initially, but matroxfb will break if we do that, and presently nothing cares about trying to take ->lock that early on. ->mm_lock was a special case as the lock/unlock pairs were sprinkled around well before initialization, while in the ->lock case all of the lock/unlock pairs are handled internally by the fbmem code (at least a quick grep does not show any drivers using it on their own). -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 6 Jul 2009, Paul Mundt wrote: > > > > Why not "lock" as well? > > I had that initially, but matroxfb will break if we do that, and > presently nothing cares about trying to take ->lock that early on. I really would rather have consistency than some odd rules like that. In particular - if matroxfb is different and needs its own lock initialization because it doesn't use the common allocation routine, then please make _that_ consistent too. Rather than have it special-case just one lock that it needs to initialize separately, make it clear that since it does its own allocations it needs to initialize _everything_ separately. Otherwise anybody grepping for things will always be confused, since depending on random factors they'll notice the initializations in one place or the other, and then do the wrong thing - exactly like mm_lock was not correctly initialized this time around. In other words: it's actually _better_ to make the matroxfb pain _more_ obvious rather than less. And maybe we can deprecate the driver, throw it out entirely, or have somebody actually fix it? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 53ea056..27a5271 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -1514,7 +1514,6 @@ register_framebuffer(struct fb_info *fb_info) break; fb_info->node = i; mutex_init(&fb_info->lock); - mutex_init(&fb_info->mm_lock); fb_info->dev = device_create(fb_class, fb_info->device, MKDEV(FB_MAJOR, i), NULL, "fb%d", i); diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c index d4a2c11..dd413ad 100644 --- a/drivers/video/fbsysfs.c +++ b/drivers/video/fbsysfs.c @@ -62,6 +62,8 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev) mutex_init(&info->bl_curve_mutex); #endif + mutex_init(&info->mm_lock); + return info; #undef PADDING #undef BYTES_PER_LONG