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: 49+ 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-29 10:03 ` Andrea Righi
2009-01-30 4:15 ` Andrey Borzenkov
2009-01-30 10:23 ` Andrea Righi
2009-01-30 10:23 ` Andrea Righi
2009-01-31 15:53 ` Andrea Righi
2009-01-31 16:05 ` Andrey Borzenkov
2009-01-31 16:05 ` Andrey Borzenkov
2009-01-31 15:53 ` Andrea Righi
2009-01-31 17:09 ` Andrea Righi
2009-01-31 17:09 ` Andrea Righi
2009-01-31 17:44 ` Geert Uytterhoeven
2009-01-31 18:11 ` Andrea Righi
2009-01-31 18:11 ` Andrea Righi
2009-01-31 17:44 ` Geert Uytterhoeven
2009-02-01 7:15 ` Andrey Borzenkov
2009-02-01 22:50 ` Andrea Righi
2009-02-01 22:50 ` Andrea Righi
2009-02-02 17:36 ` Andrey Borzenkov
2009-02-03 10:09 ` Andrea Righi
2009-02-03 10:09 ` Andrea Righi
2009-02-04 8:19 ` Andrey Borzenkov
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-02-05 1:49 ` Rafael J. Wysocki
2009-02-04 23:58 ` Rafael J. Wysocki
2009-03-22 14:48 ` Andrey Borzenkov
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-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
2009-04-07 9:54 ` Andrea Righi
2009-04-07 9:17 ` Geert Uytterhoeven
2009-04-06 21:45 ` Andrea Righi
2009-04-06 18:47 ` Krzysztof Helt
2009-03-22 17:57 ` [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency Rafael J. Wysocki
2009-02-04 21:46 ` Andrea Righi
2009-02-02 17:36 ` [Linux-fbdev-devel] [2.6.29-rc2] fb_mmap: circular locking dependency on hibernation Andrey Borzenkov
2009-02-01 7:15 ` Andrey Borzenkov
2009-01-30 4:15 ` Andrey Borzenkov
2009-01-29 9:10 ` Geert Uytterhoeven
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 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.