2009-03-22 14:49:09

by Andrei Borzenkov

[permalink] [raw]
Subject: Re: [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency

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 <[email protected]>
> Signed-off-by: Andrea Righi <[email protected]>

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))


Attachments:
(No filename) (13.54 kB)
signature.asc (197.00 B)
This is a digitally signed message part.
Download all attachments

2009-03-22 17:57:41

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency

On Sunday 22 March 2009, Andrey Borzenkov wrote:
> 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 <[email protected]>
> > Signed-off-by: Andrea Righi <[email protected]>
>
> Hmm ... is it in current Linus tree? Because I happened to have this again
> in rc8:

It doesn't seems so, at least I don't see it in there.

Thanks,
Rafael


> [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))

2009-03-22 22:06:49

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH -mmotm] fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency

On Sun, 22 Mar 2009 18:57:19 +0100 "Rafael J. Wysocki" <[email protected]> wrote:

> On Sunday 22 March 2009, Andrey Borzenkov wrote:
> > 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 <[email protected]>
> > > Signed-off-by: Andrea Righi <[email protected]>
> >
> > Hmm ... is it in current Linus tree? Because I happened to have this again
> > in rc8:
>
> It doesn't seems so, at least I don't see it in there.

I didn't realise that this was needed in mainline.

I would hate to mrege it this late and find that it introduces some nasty
new deadlock which everyone hits. Probably it's more prodent to merge it
into 2.6.29.1 now.



From: Andrea Righi <[email protected]>

Fix a circular locking dependency 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 <[email protected]>
Signed-off-by: Andrea Righi <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Krzysztof Helt <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

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 -puN drivers/video/backlight/backlight.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency drivers/video/backlight/backlight.c
--- a/drivers/video/backlight/backlight.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency
+++ a/drivers/video/backlight/backlight.c
@@ -35,6 +35,8 @@ static int fb_notifier_callback(struct n
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 n
backlight_update_status(bd);
}
mutex_unlock(&bd->ops_lock);
+ unlock_fb_info(evdata->info);
return 0;
}

diff -puN drivers/video/backlight/lcd.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency drivers/video/backlight/lcd.c
--- a/drivers/video/backlight/lcd.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency
+++ a/drivers/video/backlight/lcd.c
@@ -40,6 +40,8 @@ static int fb_notifier_callback(struct n
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 n
}
}
mutex_unlock(&ld->ops_lock);
+ unlock_fb_info(evdata->info);
return 0;
}

diff -puN drivers/video/console/fbcon.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency drivers/video/console/fbcon.c
--- a/drivers/video/console/fbcon.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency
+++ a/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
}
}

- 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(

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
}
}

-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 not
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 not

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 not
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 -puN drivers/video/fbmem.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency drivers/video/fbmem.c
--- a/drivers/video/fbmem.c~fbmem-fix-fb_info-lock-and-mm-mmap_sem-circular-locking-dependency
+++ a/drivers/video/fbmem.c
@@ -1086,13 +1086,8 @@ static long do_fb_ioctl(struct fb_info *
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 *
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))
_


2009-04-06 18:46:22

by Krzysztof Helt

[permalink] [raw]
Subject: commit 66c1ca breaks fbdev mode switching

Hi,

The commit 66c1ca breaks fbdev mode switching. The first mode
(selected with mode_option or during boot) is ok but every mode
switch with fbset locks up computer. If the commit is reverted
mode switching works correctly.

I do not understand why. I would be grateful if someone else
can confirm the problem as well.

The commit's details are:

commit 66c1ca019078220dc1bf968f2bb18421100ef147
Author: Andrea Righi <[email protected]>
Date: Tue Mar 31 15:25:18 2009 -0700

fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency


Regards,
Krzysztof

----------------------------------------------------------------------
Kto lepiej radzi sobie w kuchni?
Odpowiedz i wygraj sprzet AGD! >> http://link.interia.pl/f2103

2009-04-06 21:45:46

by Andrea Righi

[permalink] [raw]
Subject: Re: commit 66c1ca breaks fbdev mode switching

On Mon, Apr 06, 2009 at 08:47:18PM +0200, Krzysztof Helt wrote:
> Hi,
>
> The commit 66c1ca breaks fbdev mode switching. The first mode
> (selected with mode_option or during boot) is ok but every mode
> switch with fbset locks up computer. If the commit is reverted
> mode switching works correctly.
>
> I do not understand why. I would be grateful if someone else
> can confirm the problem as well.
>
> The commit's details are:
>
> commit 66c1ca019078220dc1bf968f2bb18421100ef147
> Author: Andrea Righi <[email protected]>
> Date: Tue Mar 31 15:25:18 2009 -0700
>
> fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency
>
>
> Regards,
> Krzysztof

Any lockdep info or oops? it just locks up the system?

Thanks,
-Andrea

2009-04-07 06:11:54

by Krzysztof Helt

[permalink] [raw]
Subject: Re: Re: commit 66c1ca breaks fbdev mode switching

Andrea Righi napisa?(a):
> On Mon, Apr 06, 2009 at 08:47:18PM +0200, Krzysztof Helt wrote:
> > Hi,
> >
> > The commit 66c1ca breaks fbdev mode switching. The first mode
> > (selected with mode_option or during boot) is ok but every mode
> > switch with fbset locks up computer. If the commit is reverted
> > mode switching works correctly.
> >
> > I do not understand why. I would be grateful if someone else
> > can confirm the problem as well.
> >
> > The commit&#039;s details are:
> >
> > commit 66c1ca019078220dc1bf968f2bb18421100ef147
> > Author: Andrea Righi >[email protected]>
> > Date: Tue Mar 31 15:25:18 2009 -0700
> >
> > fbmem: fix fb_info->lock and mm->mmap_sem circular locking
> dependency
> >
> >
> > Regards,
> > Krzysztof
>
> Any lockdep info or oops? it just locks up the system?
>

It just locked up. I can try with more debug info enabled.

Regards,
Krzysztof

----------------------------------------------------------------------
10% zysku na lokacie bankowej z gwarancja BFG. Sprawdz!
http://link.interia.pl/f20fd

2009-04-07 09:17:21

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: commit 66c1ca breaks fbdev mode switching

On Mon, 6 Apr 2009, Andrea Righi wrote:
> On Mon, Apr 06, 2009 at 08:47:18PM +0200, Krzysztof Helt wrote:
> > The commit 66c1ca breaks fbdev mode switching. The first mode
> > (selected with mode_option or during boot) is ok but every mode
> > switch with fbset locks up computer. If the commit is reverted
> > mode switching works correctly.
> >
> > I do not understand why. I would be grateful if someone else
> > can confirm the problem as well.
> >
> > The commit's details are:
> >
> > commit 66c1ca019078220dc1bf968f2bb18421100ef147
> > Author: Andrea Righi <[email protected]>
> > Date: Tue Mar 31 15:25:18 2009 -0700
> >
> > fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency
>
> Any lockdep info or oops? it just locks up the system?

# echo d > /proc/sysrq-trigger

[ 187.654947] INFO: lockdep is turned off.

Strange, I do have lockdep enabled:

CONFIG_LOCKDEP_SUPPORT=y
CONFIG_LOCKDEP=y
CONFIG_DEBUG_LOCKDEP=y

# echo w > /proc/sysrq-trigger

[ 243.301997] SysRq : Show Blocked State
[ 243.302007] task PC stack pid father
[ 243.302017] events/0 D 0000000000000000 0 9 2
[ 243.302025] Call Trace:
[ 243.302038] [c0000000060cb680] [c0000000060cb730] 0xc0000000060cb730 (unreliable)
[ 243.302055] [c0000000060cb850] [c00000000001088c] .__switch_to+0xec/0x14c
[ 243.302068] [c0000000060cb8e0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 243.302078] [c0000000060cba10] [c0000000003c459c] .schedule+0x1c/0x3c
[ 243.302087] [c0000000060cba90] [c0000000003c4798] .schedule_timeout+0x40/0x1f8
[ 243.302098] [c0000000060cbb80] [c0000000003c65b4] .__down+0x70/0xc4
[ 243.302110] [c0000000060cbc40] [c000000000073f3c] .down+0x44/0x6c
[ 243.302123] [c0000000060cbcd0] [c0000000000532c0] .acquire_console_sem+0x30/0x64
[ 243.302143] [c0000000060cbd50] [c00000000024511c] .console_callback+0x20/0x19c
[ 243.302156] [c0000000060cbdf0] [c00000000006940c] .worker_thread+0x23c/0x364
[ 243.302165] [c0000000060cbf00] [c00000000006e5d4] .kthread+0x78/0xc4
[ 243.302177] [c0000000060cbf90] [c000000000021450] .kernel_thread+0x54/0x70
[ 243.302184] events/1 D 0000000000000000 0 10 2
[ 243.302192] Call Trace:
[ 243.302199] [c0000000060d3660] [c0000000060d3710] 0xc0000000060d3710 (unreliable)
[ 243.302210] [c0000000060d3830] [c00000000001088c] .__switch_to+0xec/0x14c
[ 243.302220] [c0000000060d38c0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 243.302229] [c0000000060d39f0] [c0000000003c459c] .schedule+0x1c/0x3c
[ 243.302238] [c0000000060d3a70] [c0000000003c4798] .schedule_timeout+0x40/0x1f8
[ 243.302248] [c0000000060d3b60] [c0000000003c65b4] .__down+0x70/0xc4
[ 243.302257] [c0000000060d3c20] [c000000000073f3c] .down+0x44/0x6c
[ 243.302267] [c0000000060d3cb0] [c0000000000532c0] .acquire_console_sem+0x30/0x64
[ 243.302278] [c0000000060d3d30] [c00000000021ac88] .fb_flashcursor+0x44/0x3bc
[ 243.302288] [c0000000060d3df0] [c00000000006940c] .worker_thread+0x23c/0x364
[ 243.302298] [c0000000060d3f00] [c00000000006e5d4] .kthread+0x78/0xc4
[ 243.302307] [c0000000060d3f90] [c000000000021450] .kernel_thread+0x54/0x70
[ 243.302322] ps3fb D 0000000000000000 0 370 2
[ 243.302330] Call Trace:
[ 243.302337] [c0000000061077a0] [c000000006107850] 0xc000000006107850 (unreliable)
[ 243.302348] [c000000006107970] [c00000000001088c] .__switch_to+0xec/0x14c
[ 243.302358] [c000000006107a00] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 243.302367] [c000000006107b30] [c0000000003c459c] .schedule+0x1c/0x3c
[ 243.302376] [c000000006107bb0] [c0000000003c4798] .schedule_timeout+0x40/0x1f8
[ 243.302386] [c000000006107ca0] [c0000000003c65b4] .__down+0x70/0xc4
[ 243.302396] [c000000006107d60] [c000000000073f3c] .down+0x44/0x6c
[ 243.302406] [c000000006107df0] [c0000000000532c0] .acquire_console_sem+0x30/0x64
[ 243.302417] [c000000006107e70] [c000000000228d0c] .ps3fbd+0x4c/0x9c
[ 243.302426] [c000000006107f00] [c00000000006e5d4] .kthread+0x78/0xc4
[ 243.302435] [c000000006107f90] [c000000000021450] .kernel_thread+0x54/0x70
[ 243.302466] fbset D 000000000ff59878 0 2069 2065
[ 243.302474] Call Trace:
[ 243.302481] [c000000006d43040] [c000000006d430f0] 0xc000000006d430f0 (unreliable)
[ 243.302492] [c000000006d43210] [c00000000001088c] .__switch_to+0xec/0x14c
[ 243.302502] [c000000006d432a0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 243.302512] [c000000006d433d0] [c0000000003c587c] .mutex_lock_nested+0x268/0x414
[ 243.302521] [c000000006d434d0] [c000000000214200] .lock_fb_info+0x28/0x64
[ 243.302531] [c000000006d43560] [c0000000002208f8] .fbcon_event_notify+0x144/0x89c
[ 243.302542] [c000000006d436b0] [c000000000074174] .notifier_call_chain+0xac/0x124
[ 243.302552] [c000000006d43760] [c000000000074510] .__blocking_notifier_call_chain+0x70/0xb0
[ 243.302565] [c000000006d43810] [c000000000213bd0] .fb_notifier_call_chain+0x28/0x40
[ 243.302575] [c000000006d43890] [c000000000215dec] .fb_set_var+0x2d4/0x314
[ 243.302583] [c000000006d43a10] [c000000000215f68] .do_fb_ioctl+0x13c/0x4bc
[ 243.302592] [c000000006d43c10] [c0000000002163dc] .fb_compat_ioctl+0xf4/0x834
[ 243.302606] [c000000006d43d50] [c0000000001292c4] .compat_sys_ioctl+0x11c/0x448
[ 243.302616] [c000000006d43e30] [c0000000000074dc] syscall_exit+0x0/0x40

and a bit later:

[ 608.812517] INFO: task events/0:9 blocked for more than 480 seconds.
[ 608.812523] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 608.812529] events/0 D 0000000000000000 0 9 2
[ 608.812538] Call Trace:
[ 608.812550] [c0000000060cb680] [c0000000060cb730] 0xc0000000060cb730 (unreliable)
[ 608.812567] [c0000000060cb850] [c00000000001088c] .__switch_to+0xec/0x14c
[ 608.812580] [c0000000060cb8e0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 608.812590] [c0000000060cba10] [c0000000003c459c] .schedule+0x1c/0x3c
[ 608.812599] [c0000000060cba90] [c0000000003c4798] .schedule_timeout+0x40/0x1f8
[ 608.812610] [c0000000060cbb80] [c0000000003c65b4] .__down+0x70/0xc4
[ 608.812622] [c0000000060cbc40] [c000000000073f3c] .down+0x44/0x6c
[ 608.812635] [c0000000060cbcd0] [c0000000000532c0] .acquire_console_sem+0x30/0x64
[ 608.812647] [c0000000060cbd50] [c00000000024511c] .console_callback+0x20/0x19c
[ 608.812659] [c0000000060cbdf0] [c00000000006940c] .worker_thread+0x23c/0x364
[ 608.812669] [c0000000060cbf00] [c00000000006e5d4] .kthread+0x78/0xc4
[ 608.812681] [c0000000060cbf90] [c000000000021450] .kernel_thread+0x54/0x70
[ 608.812687] INFO: lockdep is turned off.
[ 608.812693] INFO: task events/1:10 blocked for more than 480 seconds.
[ 608.812698] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 608.812703] events/1 D 0000000000000000 0 10 2
[ 608.812711] Call Trace:
[ 608.812718] [c0000000060d3660] [c0000000060d3710] 0xc0000000060d3710 (unreliable)
[ 608.812729] [c0000000060d3830] [c00000000001088c] .__switch_to+0xec/0x14c
[ 608.812738] [c0000000060d38c0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 608.812748] [c0000000060d39f0] [c0000000003c459c] .schedule+0x1c/0x3c
[ 608.812757] [c0000000060d3a70] [c0000000003c4798] .schedule_timeout+0x40/0x1f8
[ 608.812767] [c0000000060d3b60] [c0000000003c65b4] .__down+0x70/0xc4
[ 608.812776] [c0000000060d3c20] [c000000000073f3c] .down+0x44/0x6c
[ 608.812786] [c0000000060d3cb0] [c0000000000532c0] .acquire_console_sem+0x30/0x64
[ 608.812797] [c0000000060d3d30] [c00000000021ac88] .fb_flashcursor+0x44/0x3bc
[ 608.812807] [c0000000060d3df0] [c00000000006940c] .worker_thread+0x23c/0x364
[ 608.812816] [c0000000060d3f00] [c00000000006e5d4] .kthread+0x78/0xc4
[ 608.812826] [c0000000060d3f90] [c000000000021450] .kernel_thread+0x54/0x70
[ 608.812832] INFO: lockdep is turned off.
[ 608.812868] INFO: task fbset:2069 blocked for more than 480 seconds.
[ 608.812872] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 608.812878] fbset D 000000000ff59878 0 2069 2065
[ 608.812886] Call Trace:
[ 608.812893] [c000000006d43040] [c000000006d430f0] 0xc000000006d430f0 (unreliable)
[ 608.812904] [c000000006d43210] [c00000000001088c] .__switch_to+0xec/0x14c
[ 608.812914] [c000000006d432a0] [c0000000003c449c] .__schedule+0x9b8/0xa9c
[ 608.812924] [c000000006d433d0] [c0000000003c587c] .mutex_lock_nested+0x268/0x414
[ 608.812933] [c000000006d434d0] [c000000000214200] .lock_fb_info+0x28/0x64
[ 608.812943] [c000000006d43560] [c0000000002208f8] .fbcon_event_notify+0x144/0x89c
[ 608.812954] [c000000006d436b0] [c000000000074174] .notifier_call_chain+0xac/0x124
[ 608.812964] [c000000006d43760] [c000000000074510] .__blocking_notifier_call_chain+0x70/0xb0
[ 608.812978] [c000000006d43810] [c000000000213bd0] .fb_notifier_call_chain+0x28/0x40
[ 608.812987] [c000000006d43890] [c000000000215dec] .fb_set_var+0x2d4/0x314
[ 608.812996] [c000000006d43a10] [c000000000215f68] .do_fb_ioctl+0x13c/0x4bc
[ 608.813005] [c000000006d43c10] [c0000000002163dc] .fb_compat_ioctl+0xf4/0x834
[ 608.813018] [c000000006d43d50] [c0000000001292c4] .compat_sys_ioctl+0x11c/0x448
[ 608.813028] [c000000006d43e30] [c0000000000074dc] syscall_exit+0x0/0x40
[ 608.813034] INFO: lockdep is turned off.

As evidenced by the compat_sys_ioctl, this is a 32-bit userland on ppc64.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village ? Da Vincilaan 7-D1 ? B-1935 Zaventem ? Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 ? RPR Brussels
Fortis ? BIC GEBABEBB ? IBAN BE41293037680010

2009-04-07 09:55:00

by Andrea Righi

[permalink] [raw]
Subject: Re: commit 66c1ca breaks fbdev mode switching

On Tue, Apr 07, 2009 at 11:17:00AM +0200, Geert Uytterhoeven wrote:
> On Mon, 6 Apr 2009, Andrea Righi wrote:
> > On Mon, Apr 06, 2009 at 08:47:18PM +0200, Krzysztof Helt wrote:
> > > The commit 66c1ca breaks fbdev mode switching. The first mode
> > > (selected with mode_option or during boot) is ok but every mode
> > > switch with fbset locks up computer. If the commit is reverted
> > > mode switching works correctly.
> > >
> > > I do not understand why. I would be grateful if someone else
> > > can confirm the problem as well.
> > >
> > > The commit's details are:
> > >
> > > commit 66c1ca019078220dc1bf968f2bb18421100ef147
> > > Author: Andrea Righi <[email protected]>
> > > Date: Tue Mar 31 15:25:18 2009 -0700
> > >
> > > fbmem: fix fb_info->lock and mm->mmap_sem circular locking dependency
> >
> > Any lockdep info or oops? it just locks up the system?
>
> # echo d > /proc/sysrq-trigger
>
> [ 187.654947] INFO: lockdep is turned off.
>
> Strange, I do have lockdep enabled:
>
> CONFIG_LOCKDEP_SUPPORT=y
> CONFIG_LOCKDEP=y
> CONFIG_DEBUG_LOCKDEP=y

Probably because lockdep reported the first lock bug and disabled the
checking for the next lock bugs, to avoid log flooding of follow-up
bugs. Search the first lockdep occurrence in dmesg.

-Andrea