From: Andrey Borzenkov <arvidjaar@gmail.com>
To: Andrea Righi <righi.andrea@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
"Antonino A. Daplas" <adaplas@gmail.com>,
linux-fbdev-devel@lists.sourceforge.net,
linux-pm@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, Dave Jones <davej@redhat.com>,
Harvey Harrison <harvey.harrison@gmail.com>,
"Rafael J. Wysocki" <rjw@sisk.pl>,
Johannes Weiner <hannes@cmpxchg.org>,
Stefan Richter <stefanr@s5r6.in-berlin.de>
Subject: Re: [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency
Date: Sun, 22 Mar 2009 17:48:33 +0300 [thread overview]
Message-ID: <200903221748.42871.arvidjaar@gmail.com> (raw)
In-Reply-To: <1233783983-28802-1-git-send-email-righi.andrea@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 14251 bytes --]
On 5 февраля 2009 00:46:23 Andrea Righi wrote:
> Fix this circular locking dependencies in the frame buffer console
> driver pushing down the mutex fb_info->lock.
>
> Circular locking dependecies occur calling the blocking
> fb_notifier_call_chain() with fb_info->lock held. Notifier callbacks
> can try to acquire mm->mmap_sem, while fb_mmap() acquires the locks
> in the reverse order mm->mmap_sem => fb_info->lock.
>
> Tested-by: Andrey Borzenkov <arvidjaar@mail.ru>
> Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
Hmm ... is it in current Linus tree? Because I happened to have this again
in rc8:
[47800.440873] =======================================================
[47800.440950] [ INFO: possible circular locking dependency detected ]
[47800.440981] 2.6.29-rc8-1avb #35
[47800.440999] -------------------------------------------------------
[47800.441026] s2disk/19740 is trying to acquire lock:
[47800.441057] (&fb_info->lock){--..}, at: [<c02175b7>] fb_mmap+0x97/0x170
[47800.441149]
[47800.441152] but task is already holding lock:
[47800.441183] (&mm->mmap_sem){----}, at: [<c0106e2e>] sys_mmap2+0x8e/0xc0
[47800.441260]
[47800.441262] which lock already depends on the new lock.
[47800.441266]
[47800.441307]
[47800.441309] the existing dependency chain (in reverse order) is:
[47800.441345]
[47800.441347] -> #3 (&mm->mmap_sem){----}:
[47800.441426] [<c014347f>] __lock_acquire+0x129f/0x1930
[47800.441466] [<c0143b6c>] lock_acquire+0x5c/0x80
[47800.441500] [<c0175f97>] might_fault+0x77/0xa0
[47800.441548] [<c01fd746>] copy_to_user+0x36/0x120
[47800.441591] [<c0199eb7>] filldir+0x97/0xe0
[47800.441627] [<c01d2279>] sysfs_readdir+0x129/0x220
[47800.441668] [<c019a066>] vfs_readdir+0x86/0xa0
[47800.441701] [<c019a1a8>] sys_getdents+0x68/0xc0
[47800.441734] [<c010340a>] syscall_call+0x7/0xb
[47800.441770] [<ffffffff>] 0xffffffff
[47800.441839]
[47800.441842] -> #2 (sysfs_mutex){--..}:
[47800.441906] [<c014347f>] __lock_acquire+0x129f/0x1930
[47800.441941] [<c0143b6c>] lock_acquire+0x5c/0x80
[47800.441974] [<c03556ba>] mutex_lock_nested+0xba/0x2f0
[47800.442012] [<c01d260c>] sysfs_addrm_start+0x2c/0xc0
[47800.442012] [<c01d2bc0>] create_dir+0x40/0x90
[47800.442012] [<c01d2c3b>] sysfs_create_dir+0x2b/0x50
[47800.442012] [<c01f74ec>] kobject_add_internal+0xbc/0x1b0
[47800.442012] [<c01f76b1>] kobject_add_varg+0x31/0x50
[47800.442012] [<c01f772c>] kobject_add+0x2c/0x60
[47800.442012] [<c026c598>] device_add+0xa8/0x550
[47800.442012] [<c026ca52>] device_register+0x12/0x20
[47800.442012] [<c026cb0b>] device_create_vargs+0xab/0xc0
[47800.442012] [<c026cb48>] device_create+0x28/0x30
[47800.442012] [<c0265dfd>] register_con_driver+0xed/0x130
[47800.442012] [<c02672eb>] take_over_console+0x1b/0x50
[47800.442012] [<c02244fd>] fbcon_takeover+0x5d/0xb0
[47800.442012] [<c0225170>] fbcon_event_notify+0x820/0x900
[47800.442012] [<c0137183>] notifier_call_chain+0x53/0xa0
[47800.442012] [<c0137444>] __blocking_notifier_call_chain+0x44/0x60
[47800.442012] [<c013747a>] blocking_notifier_call_chain+0x1a/0x20
[47800.442012] [<c0216f51>] fb_notifier_call_chain+0x11/0x20
[47800.442012] [<c0217ed8>] register_framebuffer+0x168/0x220
[47800.442012] [<c047e2f2>] vesafb_probe+0x542/0x783
[47800.442012] [<c026f72f>] platform_drv_probe+0xf/0x20
[47800.442012] [<c026e917>] driver_probe_device+0x87/0x1b0
[47800.442012] [<c026ead8>] __device_attach+0x8/0x10
[47800.442012] [<c026df5b>] bus_for_each_drv+0x5b/0x80
[47800.442012] [<c026eb86>] device_attach+0x76/0x80
[47800.442012] [<c026dd77>] bus_attach_device+0x47/0x70
[47800.442012] [<c026c813>] device_add+0x323/0x550
[47800.442012] [<c0270105>] platform_device_add+0x175/0x1c0
[47800.442012] [<c047e5cd>] vesafb_init+0x9a/0x1ec
[47800.442012] [<c010111a>] do_one_initcall+0x2a/0x160
[47800.442012] [<c04694d5>] kernel_init+0x83/0xd5
[47800.442012] [<c0103a37>] kernel_thread_helper+0x7/0x10
[47800.442012] [<ffffffff>] 0xffffffff
[47800.442012]
[47800.442012] -> #1 ((fb_notifier_list).rwsem){----}:
[47800.442012] [<c014347f>] __lock_acquire+0x129f/0x1930
[47800.442012] [<c0143b6c>] lock_acquire+0x5c/0x80
[47800.442012] [<c0355fb9>] down_read+0x49/0x90
[47800.442012] [<c013742a>] __blocking_notifier_call_chain+0x2a/0x60
[47800.442012] [<c013747a>] blocking_notifier_call_chain+0x1a/0x20
[47800.442012] [<c0216f51>] fb_notifier_call_chain+0x11/0x20
[47800.442012] [<c021856e>] do_fb_ioctl+0x2ae/0x5d0
[47800.442012] [<c02188ad>] fb_ioctl+0x1d/0x20
[47800.442012] [<c0199270>] vfs_ioctl+0x20/0x80
[47800.442012] [<c0199482>] do_vfs_ioctl+0x72/0x570
[47800.442012] [<c01999b9>] sys_ioctl+0x39/0x70
[47800.442012] [<c0103331>] sysenter_do_call+0x12/0x31
[47800.442012] [<ffffffff>] 0xffffffff
[47800.442012]
[47800.442012] -> #0 (&fb_info->lock){--..}:
[47800.442012] [<c01435ec>] __lock_acquire+0x140c/0x1930
[47800.442012] [<c0143b6c>] lock_acquire+0x5c/0x80
[47800.442012] [<c03556ba>] mutex_lock_nested+0xba/0x2f0
[47800.442012] [<c02175b7>] fb_mmap+0x97/0x170
[47800.442012] [<c017cbc0>] mmap_region+0x2e0/0x440
[47800.442012] [<c017ceea>] do_mmap_pgoff+0x1ca/0x2f0
[47800.442012] [<c0106e4d>] sys_mmap2+0xad/0xc0
[47800.442012] [<c0103331>] sysenter_do_call+0x12/0x31
[47800.442012] [<ffffffff>] 0xffffffff
[47800.442012]
[47800.442012] other info that might help us debug this:
[47800.442012]
[47800.442012] 1 lock held by s2disk/19740:
[47800.442012] #0: (&mm->mmap_sem){----}, at: [<c0106e2e>]
sys_mmap2+0x8e/0xc0
[47800.442012]
[47800.442012] stack backtrace:
[47800.442012] Pid: 19740, comm: s2disk Not tainted 2.6.29-rc8-1avb #35
[47800.442012] Call Trace:
[47800.442012] [<c0354324>] ? printk+0x18/0x1c
[47800.442012] [<c0141d8f>] print_circular_bug_tail+0xcf/0xe0
[47800.442012] [<c01435ec>] __lock_acquire+0x140c/0x1930
[47800.442012] [<c0132936>] ? add_wait_queue+0x36/0x50
[47800.442012] [<c013f375>] ? lock_release_holdtime+0x35/0x210
[47800.442012] [<c0143b6c>] lock_acquire+0x5c/0x80
[47800.442012] [<c02175b7>] ? fb_mmap+0x97/0x170
[47800.442012] [<c03556ba>] mutex_lock_nested+0xba/0x2f0
[47800.442012] [<c02175b7>] ? fb_mmap+0x97/0x170
[47800.442012] [<c02175b7>] ? fb_mmap+0x97/0x170
[47800.442012] [<c01892c5>] ? kmem_cache_alloc+0xa5/0x100
[47800.442012] [<c02175b7>] fb_mmap+0x97/0x170
[47800.442012] [<c017cbc0>] mmap_region+0x2e0/0x440
[47800.442012] [<c017ceea>] do_mmap_pgoff+0x1ca/0x2f0
[47800.442012] [<c0106e4d>] sys_mmap2+0xad/0xc0
[47800.442012] [<c0103331>] sysenter_do_call+0x12/0x31
[47803.041426] Syncing filesystems ... done.
[47803.055587] Freezing user space processes ... (elapsed 0.00 seconds)
done.
[47803.059328] Freezing remaining freezable tasks ... (elapsed 0.00 seconds)
done.
> ---
> drivers/video/backlight/backlight.c | 3 +
> drivers/video/backlight/lcd.c | 3 +
> drivers/video/console/fbcon.c | 73
> ++++++++++++++++++++++++++++++----- drivers/video/fbmem.c
> | 11 +-----
> 4 files changed, 70 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c index 157057c..dd37cbc 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -35,6 +35,8 @@ static int fb_notifier_callback(struct
> notifier_block *self, return 0;
>
> bd = container_of(self, struct backlight_device, fb_notif);
> + if (!lock_fb_info(evdata->info))
> + return -ENODEV;
> mutex_lock(&bd->ops_lock);
> if (bd->ops)
> if (!bd->ops->check_fb ||
> @@ -47,6 +49,7 @@ static int fb_notifier_callback(struct
> notifier_block *self, backlight_update_status(bd);
> }
> mutex_unlock(&bd->ops_lock);
> + unlock_fb_info(evdata->info);
> return 0;
> }
>
> diff --git a/drivers/video/backlight/lcd.c
> b/drivers/video/backlight/lcd.c index b644947..0bb13df 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -40,6 +40,8 @@ static int fb_notifier_callback(struct
> notifier_block *self, if (!ld->ops)
> return 0;
>
> + if (!lock_fb_info(evdata->info))
> + return -ENODEV;
> mutex_lock(&ld->ops_lock);
> if (!ld->ops->check_fb || ld->ops->check_fb(ld, evdata->info)) {
> if (event == FB_EVENT_BLANK) {
> @@ -51,6 +53,7 @@ static int fb_notifier_callback(struct
> notifier_block *self, }
> }
> mutex_unlock(&ld->ops_lock);
> + unlock_fb_info(evdata->info);
> return 0;
> }
>
> diff --git a/drivers/video/console/fbcon.c
> b/drivers/video/console/fbcon.c index 1657b96..2cd500a 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -2954,8 +2954,11 @@ static int fbcon_fb_unbind(int idx)
>
> static int fbcon_fb_unregistered(struct fb_info *info)
> {
> - int i, idx = info->node;
> + int i, idx;
>
> + if (!lock_fb_info(info))
> + return -ENODEV;
> + idx = info->node;
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> if (con2fb_map[i] == idx)
> con2fb_map[i] = -1;
> @@ -2979,13 +2982,14 @@ static int fbcon_fb_unregistered(struct
> fb_info *info) }
> }
>
> - if (!num_registered_fb)
> - unregister_con_driver(&fb_con);
> -
> -
> if (primary_device == idx)
> primary_device = -1;
>
> + unlock_fb_info(info);
> +
> + if (!num_registered_fb)
> + unregister_con_driver(&fb_con);
> +
> return 0;
> }
>
> @@ -3021,9 +3025,13 @@ static inline void fbcon_select_primary(struct
> fb_info *info)
>
> static int fbcon_fb_registered(struct fb_info *info)
> {
> - int ret = 0, i, idx = info->node;
> + int ret = 0, i, idx;
>
> + if (!lock_fb_info(info))
> + return -ENODEV;
> + idx = info->node;
> fbcon_select_primary(info);
> + unlock_fb_info(info);
>
> if (info_idx == -1) {
> for (i = first_fb_vc; i <= last_fb_vc; i++) {
> @@ -3124,7 +3132,7 @@ static void fbcon_get_requirement(struct
> fb_info *info, }
> }
>
> -static int fbcon_event_notify(struct notifier_block *self,
> +static int fbcon_event_notify(struct notifier_block *self,
> unsigned long action, void *data)
> {
> struct fb_event *event = data;
> @@ -3132,7 +3140,7 @@ static int fbcon_event_notify(struct
> notifier_block *self, struct fb_videomode *mode;
> struct fb_con2fbmap *con2fb;
> struct fb_blit_caps *caps;
> - int ret = 0;
> + int idx, ret = 0;
>
> /*
> * ignore all events except driver registration and deregistration
> @@ -3144,23 +3152,54 @@ static int fbcon_event_notify(struct
> notifier_block *self,
>
> switch(action) {
> case FB_EVENT_SUSPEND:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_suspended(info);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_RESUME:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_resumed(info);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_MODE_CHANGE:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_modechanged(info);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_MODE_CHANGE_ALL:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_set_all_vcs(info);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_MODE_DELETE:
> mode = event->data;
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> ret = fbcon_mode_deleted(info, mode);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_FB_UNBIND:
> - ret = fbcon_fb_unbind(info->node);
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> + idx = info->node;
> + unlock_fb_info(info);
> + ret = fbcon_fb_unbind(idx);
> break;
> case FB_EVENT_FB_REGISTERED:
> ret = fbcon_fb_registered(info);
> @@ -3178,17 +3217,31 @@ static int fbcon_event_notify(struct
> notifier_block *self, con2fb->framebuffer =
> con2fb_map[con2fb->console - 1];
> break;
> case FB_EVENT_BLANK:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_fb_blanked(info, *(int *)event->data);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_NEW_MODELIST:
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_new_modelist(info);
> + unlock_fb_info(info);
> break;
> case FB_EVENT_GET_REQ:
> caps = event->data;
> + if (!lock_fb_info(info)) {
> + ret = -ENODEV;
> + goto done;
> + }
> fbcon_get_requirement(info, caps);
> + unlock_fb_info(info);
> break;
> }
> -
> done:
> return ret;
> }
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index cfd9dce..b64f061 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1086,13 +1086,8 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd, return -EINVAL;
> con2fb.framebuffer = -1;
> event.data = &con2fb;
> -
> - if (!lock_fb_info(info))
> - return -ENODEV;
> event.info = info;
> fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event);
> - unlock_fb_info(info);
> -
> ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0;
> break;
> case FBIOPUT_CON2FBMAP:
> @@ -1109,12 +1104,8 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd, break;
> }
> event.data = &con2fb;
> - if (!lock_fb_info(info))
> - return -ENODEV;
> event.info = info;
> - ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP,
> - &event);
> - unlock_fb_info(info);
> + ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
> break;
> case FBIOBLANK:
> if (!lock_fb_info(info))
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
next prev parent reply other threads:[~2009-03-22 14:48 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-01-27 18:37 [2.6.29-rc2] fb_mmap: circular locking dependency on hibernation Andrey Borzenkov
2009-01-29 9:10 ` [Linux-fbdev-devel] " Geert Uytterhoeven
2009-01-29 10:03 ` Andrea Righi
2009-01-30 4:15 ` Andrey Borzenkov
2009-01-30 10:23 ` Andrea Righi
2009-01-31 15:53 ` Andrea Righi
2009-01-31 16:05 ` Andrey Borzenkov
2009-01-31 17:09 ` Andrea Righi
2009-01-31 17:44 ` Geert Uytterhoeven
2009-01-31 18:11 ` Andrea Righi
2009-02-01 7:15 ` Andrey Borzenkov
2009-02-01 22:50 ` Andrea Righi
2009-02-02 17:36 ` Andrey Borzenkov
2009-02-03 10:09 ` Andrea Righi
2009-02-04 8:19 ` Andrey Borzenkov
2009-02-04 21:46 ` [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency Andrea Righi
2009-02-04 23:58 ` Rafael J. Wysocki
2009-02-05 1:49 ` Rafael J. Wysocki
2009-03-22 14:48 ` Andrey Borzenkov [this message]
2009-03-22 17:57 ` Rafael J. Wysocki
2009-03-22 21:52 ` Andrew Morton
2009-04-06 18:47 ` commit 66c1ca breaks fbdev mode switching Krzysztof Helt
2009-04-06 21:45 ` Andrea Righi
2009-04-07 9:17 ` Geert Uytterhoeven
2009-04-07 9:54 ` Andrea Righi
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=200903221748.42871.arvidjaar@gmail.com \
--to=arvidjaar@gmail.com \
--cc=adaplas@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=davej@redhat.com \
--cc=geert@linux-m68k.org \
--cc=hannes@cmpxchg.org \
--cc=harvey.harrison@gmail.com \
--cc=linux-fbdev-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@lists.linux-foundation.org \
--cc=righi.andrea@gmail.com \
--cc=rjw@sisk.pl \
--cc=stefanr@s5r6.in-berlin.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).