From: Paul Mundt <lethal@linux-sh.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Wu Zhangjin <wuzhangjin@gmail.com>,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mips@linux-mips.org, Krzysztof Helt <krzysztof.h1@wp.pl>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
Andrew Morton <akpm@linux-foundation.org>,
Ralf Baechle <ralf@linux-mips.org>, ???? <yanh@lemote.com>,
zhangfx <zhangfx@lemote.com>
Subject: Re: [BUG] drivers/video/sis: deadlock introduced by "fbdev: add mutex for fb_mmap locking"
Date: Sun, 5 Jul 2009 23:52:03 +0900 [thread overview]
Message-ID: <20090705145203.GA8326@linux-sh.org> (raw)
In-Reply-To: <alpine.LFD.2.01.0907050715490.3210@localhost.localdomain>
On Sun, Jul 05, 2009 at 07:19:33AM -0700, Linus Torvalds wrote:
>
>
> On Sun, 5 Jul 2009, Wu Zhangjin wrote:
> >
> > then it works! so, I guess there is a deadlock introduced by the above
> > commit.
>
> Hmm. Perhaps more likely, the 'mm_lock' mutex hasn't even been initialized
> yet. We appear to have had that problem with matroxfb and sm501fb, and it
> may be more common than that. See commit f50bf2b2.
>
> That said, I do agree that the mm_lock seems to be causing more problems
> than it actually fixes, and maybe we should revert it. Krzysztof?
>
Looking at this a bit closer, just moving the mutex initialization in to
framebuffer_alloc() should basically fix most of these, at least it
certainly does for sm501fb and for this sis case as well. So, here's a
patch to do that.
As an aside note, matroxfb is the odd one out, as it doesn't use
framebuffer_alloc() directly for whatever reason. This actually raises an
additional issue, in that framebuffer_alloc() is already where other
mutexes are initialized, which will simply never happen on matroxfb
(suggesting that at least the FB_BACKLIGHT and matroxfb combination will
blow up, although perhaps that's not a valid combination).
Signed-off-by: Paul Mundt <lethal@linux-sh.org>
---
drivers/video/fbmem.c | 1 -
drivers/video/fbsysfs.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
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
next prev parent reply other threads:[~2009-07-05 14:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-05 9:11 [BUG] drivers/video/sis: deadlock introduced by "fbdev: add mutex for fb_mmap locking" Wu Zhangjin
2009-07-05 14:19 ` Linus Torvalds
2009-07-05 14:52 ` Paul Mundt [this message]
2009-07-05 14:56 ` Linus Torvalds
2009-07-05 15:01 ` Paul Mundt
2009-07-05 15:19 ` Linus Torvalds
2009-07-05 15:25 ` Paul Mundt
2009-07-05 15:32 ` Linus Torvalds
2009-07-05 16:18 ` Krzysztof Helt
2009-07-06 1:13 ` Wu Zhangjin
2009-07-06 14:50 ` Krzysztof Helt
2009-07-06 16:29 ` Linus Torvalds
2009-07-05 16:43 ` Krzysztof Helt
2009-07-05 15:05 ` Krzysztof Helt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090705145203.GA8326@linux-sh.org \
--to=lethal@linux-sh.org \
--cc=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=krzysztof.h1@wp.pl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=ralf@linux-mips.org \
--cc=rjw@sisk.pl \
--cc=torvalds@linux-foundation.org \
--cc=wuzhangjin@gmail.com \
--cc=yanh@lemote.com \
--cc=zhangfx@lemote.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.