* re:Possible deadlock when suspending framebuffer
@ 2011-06-15 1:09 Wanlong Gao
2011-06-15 5:58 ` Possible " Bruno Prémont
0 siblings, 1 reply; 6+ messages in thread
From: Wanlong Gao @ 2011-06-15 1:09 UTC (permalink / raw)
To: Francis Moreau
Cc: Paul Mundt, linux-fbdev, Linux Kernel Mailing List,
Francis Moreau, Linus Torvalds
[-- Attachment #1: Type: text/plain, Size: 885 bytes --]
<snip>
Hi Francis:
can you test this patch?
Thanks
>From fe026c42af4cbdce053460a428a445e99071586a Mon Sep 17 00:00:00 2001
From: Wanlong Gao <wanlong.gao@gmail.com>
Date: Wed, 15 Jun 2011 09:03:41 +0800
Subject: [PATCH] test
Signed-off-by: Wanlong Gao <wanlong.gao@gmail.com>
---
drivers/video/fbmem.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 5aac00e..6e6cef3 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1642,11 +1642,8 @@ static int do_unregister_framebuffer(struct
fb_info *fb_info)
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- unlock_fb_info(fb_info);
if (ret)
return -EINVAL;
--
1.7.4.1
[-- Attachment #2: 0001-test.patch --]
[-- Type: text/x-patch, Size: 836 bytes --]
>From fe026c42af4cbdce053460a428a445e99071586a Mon Sep 17 00:00:00 2001
From: Wanlong Gao <wanlong.gao@gmail.com>
Date: Wed, 15 Jun 2011 09:03:41 +0800
Subject: [PATCH] test
test
Signed-off-by: Wanlong Gao <wanlong.gao@gmail.com>
---
drivers/video/fbmem.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 5aac00e..6e6cef3 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1642,11 +1642,8 @@ static int do_unregister_framebuffer(struct fb_info *fb_info)
if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- unlock_fb_info(fb_info);
if (ret)
return -EINVAL;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Possible deadlock when suspending framebuffer
2011-06-15 1:09 re:Possible deadlock when suspending framebuffer Wanlong Gao
@ 2011-06-15 5:58 ` Bruno Prémont
2011-06-15 6:22 ` Wanlong Gao
2011-06-15 7:12 ` Francis Moreau
0 siblings, 2 replies; 6+ messages in thread
From: Bruno Prémont @ 2011-06-15 5:58 UTC (permalink / raw)
To: Wanlong Gao
Cc: Francis Moreau, Paul Mundt, linux-fbdev,
Linux Kernel Mailing List, Linus Torvalds
Hi,
On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
> <snip>
> Hi Francis:
> can you test this patch?
Do you have a deadlock trace which you are trying to fix?
It's either the caller of unregister_framebuffer() which must be
changed to not call unregister_framebuffer with info's lock held or
the code reacting on the notification that must not try to acquire the
lock again.
The interesting par is if console semaphore has some relation to this
deadlock as the order for taking both varies... It could be
lock_fb_info(); console_lock() versus console_lock(); lock_fb_info()
Bruno
> Thanks
>
> From fe026c42af4cbdce053460a428a445e99071586a Mon Sep 17 00:00:00 2001
> From: Wanlong Gao <wanlong.gao@gmail.com>
> Date: Wed, 15 Jun 2011 09:03:41 +0800
> Subject: [PATCH] test
>
>
>
> Signed-off-by: Wanlong Gao <wanlong.gao@gmail.com>
> ---
> drivers/video/fbmem.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 5aac00e..6e6cef3 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1642,11 +1642,8 @@ static int do_unregister_framebuffer(struct
> fb_info *fb_info)
> if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
> return -EINVAL;
>
> - if (!lock_fb_info(fb_info))
> - return -ENODEV;
> event.info = fb_info;
> ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
> - unlock_fb_info(fb_info);
Not a good idea to stop taking fb_lock here.
Pretty all calls of fb_notifier_call_chain are protected by info's
lock, except the one for FB_EVENT_FB_UNREGISTERED a few lines further.
IMHO it wou make sense to add the lock around that last one so all
notifier chain calls are handled the same.
> if (ret)
> return -EINVAL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible deadlock when suspending framebuffer
2011-06-15 5:58 ` Possible " Bruno Prémont
@ 2011-06-15 6:22 ` Wanlong Gao
2011-06-15 7:04 ` Américo Wang
2011-06-15 7:12 ` Francis Moreau
1 sibling, 1 reply; 6+ messages in thread
From: Wanlong Gao @ 2011-06-15 6:22 UTC (permalink / raw)
To: Bruno Prémont
Cc: Francis Moreau, Paul Mundt, linux-fbdev,
Linux Kernel Mailing List, Linus Torvalds
On Wed, Jun 15, 2011 at 1:58 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
>
> Hi,
>
> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
> > <snip>
> > Hi Francis:
> > can you test this patch?
>
> Do you have a deadlock trace which you are trying to fix?
No, I just look at the code and try to fix this but I'm not sure.
Can you teach me how to have a deadlock trace here?
Thanks
>
> It's either the caller of unregister_framebuffer() which must be
> changed to not call unregister_framebuffer with info's lock held or
> the code reacting on the notification that must not try to acquire the
> lock again.
>
> The interesting par is if console semaphore has some relation to this
> deadlock as the order for taking both varies... It could be
> lock_fb_info(); console_lock() versus console_lock(); lock_fb_info()
>
I see, thanks
> Bruno
>
>
> > Thanks
> >
><snip>
>
> Not a good idea to stop taking fb_lock here.
> Pretty all calls of fb_notifier_call_chain are protected by info's
> lock, except the one for FB_EVENT_FB_UNREGISTERED a few lines further.
Yup, thanks
>
> IMHO it wou make sense to add the lock around that last one so all
> notifier chain calls are handled the same.
>
> <snip>
>
--
Best regards
Wanlong Gao
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible deadlock when suspending framebuffer
2011-06-15 6:22 ` Wanlong Gao
@ 2011-06-15 7:04 ` Américo Wang
0 siblings, 0 replies; 6+ messages in thread
From: Américo Wang @ 2011-06-15 7:04 UTC (permalink / raw)
To: wanlong.gao
Cc: Bruno Prémont, Francis Moreau, Paul Mundt, linux-fbdev,
Linux Kernel Mailing List, Linus Torvalds
On Wed, Jun 15, 2011 at 2:22 PM, Wanlong Gao <wanlong.gao@gmail.com> wrote:
> On Wed, Jun 15, 2011 at 1:58 PM, Bruno Prémont
> <bonbons@linux-vserver.org> wrote:
>>
>> Hi,
>>
>> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
>> > <snip>
>> > Hi Francis:
>> > can you test this patch?
>>
>> Do you have a deadlock trace which you are trying to fix?
> No, I just look at the code and try to fix this but I'm not sure.
> Can you teach me how to have a deadlock trace here?
CONFIG_LOCKDEP=y
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible deadlock when suspending framebuffer
2011-06-15 5:58 ` Possible " Bruno Prémont
2011-06-15 6:22 ` Wanlong Gao
@ 2011-06-15 7:12 ` Francis Moreau
2011-06-15 10:20 ` Bruno Prémont
1 sibling, 1 reply; 6+ messages in thread
From: Francis Moreau @ 2011-06-15 7:12 UTC (permalink / raw)
To: Bruno Prémont
Cc: Wanlong Gao, Paul Mundt, linux-fbdev, Linux Kernel Mailing List,
Linus Torvalds
On Wed, Jun 15, 2011 at 7:58 AM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> Hi,
>
> On Wed, 15 Jun 2011 09:09:24 Wanlong Gao <wanlong.gao@gmail.com> wrote:
>> <snip>
>> Hi Francis:
>> can you test this patch?
>
> Do you have a deadlock trace which you are trying to fix?
>
> It's either the caller of unregister_framebuffer() which must be
> changed to not call unregister_framebuffer with info's lock held or
> the code reacting on the notification that must not try to acquire the
> lock again.
>
> The interesting par is if console semaphore has some relation to this
> deadlock as the order for taking both varies... It could be
> lock_fb_info(); console_lock() versus console_lock(); lock_fb_info()
>
> Bruno
>
>
>> Thanks
>>
>> From fe026c42af4cbdce053460a428a445e99071586a Mon Sep 17 00:00:00 2001
>> From: Wanlong Gao <wanlong.gao@gmail.com>
>> Date: Wed, 15 Jun 2011 09:03:41 +0800
>> Subject: [PATCH] test
>>
>>
>>
>> Signed-off-by: Wanlong Gao <wanlong.gao@gmail.com>
>> ---
>> drivers/video/fbmem.c | 3 ---
>> 1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>> index 5aac00e..6e6cef3 100644
>> --- a/drivers/video/fbmem.c
>> +++ b/drivers/video/fbmem.c
>> @@ -1642,11 +1642,8 @@ static int do_unregister_framebuffer(struct
>> fb_info *fb_info)
>> if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)
>> return -EINVAL;
>>
>> - if (!lock_fb_info(fb_info))
>> - return -ENODEV;
>> event.info = fb_info;
>> ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
>> - unlock_fb_info(fb_info);
>
> Not a good idea to stop taking fb_lock here.
> Pretty all calls of fb_notifier_call_chain are protected by info's
> lock, except the one for FB_EVENT_FB_UNREGISTERED a few lines further.
>
> IMHO it wou make sense to add the lock around that last one so all
> notifier chain calls are handled the same.
>
>> if (ret)
>> return -EINVAL;
>
>
Well, sorry for the dumb question but the fb/fbcon code is pretty hard
to follow for me.
Why does store_fbstate() and any fb driver's suspsend methods acquire
the console lock at all ?
Thanks
--
Francis
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Possible deadlock when suspending framebuffer
2011-06-15 7:12 ` Francis Moreau
@ 2011-06-15 10:20 ` Bruno Prémont
0 siblings, 0 replies; 6+ messages in thread
From: Bruno Prémont @ 2011-06-15 10:20 UTC (permalink / raw)
To: Francis Moreau
Cc: Wanlong Gao, Paul Mundt, linux-fbdev, Linux Kernel Mailing List,
Linus Torvalds
On Wed, 15 Jun 2011 09:12:46 Francis Moreau wrote:
> On Wed, Jun 15, 2011 at 7:58 AM, Bruno Prémont wrote:
> Well, sorry for the dumb question but the fb/fbcon code is pretty hard
> to follow for me.
Certainly not just for you
> Why does store_fbstate() and any fb driver's suspsend methods acquire
> the console lock at all ?
>From my understanding, fbcon currently has very loose binding with
framebuffers in general instead of just with those few framebuffers
it is effectively mapped to (and active on!).
James Simmons started a complete rework of fbcon/tty code which is
expected to get things more fine-grained, until then console semaphore
(console_lock) remains a kind of big kernel lock in the
console/framebuffer area.
As such any state change of framebuffer may influence/race against
fbcon.
e.g. for the suspend state you want to avoid fbcon to fiddle with
your framebuffer while it is suspending (and have fbcon know about the
suspended state). This way fbcon can unsuspend framebuffer if needed
but also stop accessing it when it should not.
Bruno
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-15 10:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-15 1:09 re:Possible deadlock when suspending framebuffer Wanlong Gao
2011-06-15 5:58 ` Possible " Bruno Prémont
2011-06-15 6:22 ` Wanlong Gao
2011-06-15 7:04 ` Américo Wang
2011-06-15 7:12 ` Francis Moreau
2011-06-15 10:20 ` Bruno Prémont
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox