Bug#704933: linux-image-3.2.0-4-amd64:amd64: system hangs when initializing primary video card (3.2.39-2 -> 3.2.41-2 regression)
Control: tag -1 patch
07.04.2013 в 22:31:00 +0100 Ben Hutchings написал:
> On Mon, 2013-04-08 at 00:37 +0400, Stepan Golosunov wrote:
> > After upgrading linux-image-3.2.0-4-amd64:amd64 to 3.2.41-2 system
> > hangs when initializing primary video card.
> >
> > Normally during boot bios and later linux console messages are
> > printed on primary monitor (connected to 05:00.0 card), then the
> > second video card (04:00.0) is initialized, some console messages
> > are printed on the second monitor and the first monitor is blanked.
> > Then console messages are again printed on the primary monitor.
> >
> > After upgrading to 3.2.41-2 system hangs (no disk activity, no visible
> > reaction to keyboard, though NumLock and Alt-SysRq-B work) with first
> > monitor blanked. Photo of the second monitor attached; note the
> > absence of the
> > [ 9.145482] fbcon: radeondrmfb (fb1) is primary device
> > [ 9.148103] fbcon: Remapping primary device, fb1, to tty 1-63
> > lines, which are present on the monitor when booting with 3.2.39-2.
> [...]
>
> There seems to be only one change that could have caused this,
The relevant changes in 3.2.40 (
fb: rework locking to fix lock ordering on takeover,
fb: Yet another band-aid for fixing lockdep mess;
described in
http://lists.freedesktop.org/archives/dri-devel/2013-January/033976.html)
appear to be missing from debian/changelog.
And the missing fix (fbcon: fix locking harder) was discussed in
http://lists.freedesktop.org/archives/dri-devel/2013-January/033980.html.
Any of the two attached patches makes 3.2.41-2 bootable here.
>From 054430e773c9a1e26f38e30156eff02dedfffc17 Mon Sep 17 00:00:00 2001
From: Dave Airlie <airlied@gmail.com>
Date: Fri, 25 Jan 2013 01:38:56 +0000
Subject: fbcon: fix locking harder
Okay so Alan's patch handled the case where there was no registered fbcon,
however the other path entered in set_con2fb_map pit.
In there we called fbcon_takeover, but we also took the console lock in a couple
of places. So push the console lock out to the callers of set_con2fb_map,
this means fbmem and switcheroo needed to take the lock around the fb notifier
entry points that lead to this.
This should fix the efifb regression seen by Maarten.
Tested-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Tested-by: Lu Hua <huax.lu@intel.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
Index: linux-3.2.41/drivers/gpu/vga/vga_switcheroo.c
===================================================================
--- linux-3.2.41.orig/drivers/gpu/vga/vga_switcheroo.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/gpu/vga/vga_switcheroo.c 2013-04-12 09:15:38.138119524 +0400
@@ -26,6 +26,7 @@
#include <linux/fb.h>
#include <linux/pci.h>
+#include <linux/console.h>
#include <linux/vga_switcheroo.h>
struct vga_switcheroo_client {
@@ -256,8 +257,10 @@
if (new_client->fb_info) {
struct fb_event event;
+ console_lock();
event.info = new_client->fb_info;
fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
+ console_unlock();
}
ret = vgasr_priv.handler->switchto(new_client->id);
Index: linux-3.2.41/drivers/video/console/fbcon.c
===================================================================
--- linux-3.2.41.orig/drivers/video/console/fbcon.c 2013-04-12 09:13:12.555551883 +0400
+++ linux-3.2.41/drivers/video/console/fbcon.c 2013-04-12 09:15:38.142119486 +0400
@@ -843,6 +843,8 @@
*
* Maps a virtual console @unit to a frame buffer device
* @newidx.
+ *
+ * This should be called with the console lock held.
*/
static int set_con2fb_map(int unit, int newidx, int user)
{
@@ -860,7 +862,7 @@
if (!search_for_mapped_con() || !con_is_bound(&fb_con)) {
info_idx = newidx;
- return fbcon_takeover(0);
+ return do_fbcon_takeover(0);
}
if (oldidx != -1)
@@ -868,7 +870,6 @@
found = search_fb_in_map(newidx);
- console_lock();
con2fb_map[unit] = newidx;
if (!err && !found)
err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
@@ -895,7 +896,6 @@
if (!search_fb_in_map(info_idx))
info_idx = newidx;
- console_unlock();
return err;
}
@@ -3026,6 +3026,7 @@
}
#endif /* CONFIG_VT_HW_CONSOLE_BINDING */
+/* called with console_lock held */
static int fbcon_fb_unbind(int idx)
{
int i, new_idx = -1, ret = 0;
@@ -3052,6 +3053,7 @@
return ret;
}
+/* called with console_lock held */
static int fbcon_fb_unregistered(struct fb_info *info)
{
int i, idx;
@@ -3089,6 +3091,7 @@
return 0;
}
+/* called with console_lock held */
static void fbcon_remap_all(int idx)
{
int i;
@@ -3133,6 +3136,7 @@
}
#endif /* CONFIG_FRAMEBUFFER_DETECT_PRIMARY */
+/* called with console_lock held */
static int fbcon_fb_registered(struct fb_info *info)
{
int ret = 0, i, idx;
@@ -3285,6 +3289,7 @@
ret = fbcon_fb_unregistered(info);
break;
case FB_EVENT_SET_CONSOLE_MAP:
+ /* called with console lock held */
con2fb = event->data;
ret = set_con2fb_map(con2fb->console - 1,
con2fb->framebuffer, 1);
Index: linux-3.2.41/drivers/video/fbmem.c
===================================================================
--- linux-3.2.41.orig/drivers/video/fbmem.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/video/fbmem.c 2013-04-12 09:15:38.142119486 +0400
@@ -1154,8 +1154,10 @@
event.data = &con2fb;
if (!lock_fb_info(info))
return -ENODEV;
+ console_lock();
event.info = info;
ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event);
+ console_unlock();
unlock_fb_info(info);
break;
case FBIOBLANK:
Index: linux-3.2.41/drivers/tty/vt/vt.c
===================================================================
--- linux-3.2.41.orig/drivers/tty/vt/vt.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/tty/vt/vt.c 2013-04-12 09:39:46.239517112 +0400
@@ -3016,7 +3016,7 @@
static struct class *vtconsole_class;
-static int do_bind_con_driver(const struct consw *csw, int first, int last,
+static int bind_con_driver(const struct consw *csw, int first, int last,
int deflt)
{
struct module *owner = csw->owner;
@@ -3027,7 +3027,7 @@
if (!try_module_get(owner))
return -ENODEV;
- WARN_CONSOLE_UNLOCKED();
+ console_lock();
/* check if driver is registered */
for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
@@ -3112,22 +3112,11 @@
retval = 0;
err:
+ console_unlock();
module_put(owner);
return retval;
};
-
-static int bind_con_driver(const struct consw *csw, int first, int last,
- int deflt)
-{
- int ret;
-
- console_lock();
- ret = do_bind_con_driver(csw, first, last, deflt);
- console_unlock();
- return ret;
-}
-
#ifdef CONFIG_VT_HW_CONSOLE_BINDING
static int con_is_graphics(const struct consw *csw, int first, int last)
{
@@ -3164,18 +3153,6 @@
*/
int unbind_con_driver(const struct consw *csw, int first, int last, int deflt)
{
- int retval;
-
- console_lock();
- retval = do_unbind_con_driver(csw, first, last, deflt);
- console_unlock();
- return retval;
-}
-EXPORT_SYMBOL(unbind_con_driver);
-
-/* unlocked version of unbind_con_driver() */
-int do_unbind_con_driver(const struct consw *csw, int first, int last, int deflt)
-{
struct module *owner = csw->owner;
const struct consw *defcsw = NULL;
struct con_driver *con_driver = NULL, *con_back = NULL;
@@ -3184,7 +3161,7 @@
if (!try_module_get(owner))
return -ENODEV;
- WARN_CONSOLE_UNLOCKED();
+ console_lock();
/* check if driver is registered and if it is unbindable */
for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
@@ -3197,8 +3174,10 @@
}
}
- if (retval)
+ if (retval) {
+ console_unlock();
goto err;
+ }
retval = -ENODEV;
@@ -3214,11 +3193,15 @@
}
}
- if (retval)
+ if (retval) {
+ console_unlock();
goto err;
+ }
- if (!con_is_bound(csw))
+ if (!con_is_bound(csw)) {
+ console_unlock();
goto err;
+ }
first = max(first, con_driver->first);
last = min(last, con_driver->last);
@@ -3245,14 +3228,15 @@
if (!con_is_bound(csw))
con_driver->flag &= ~CON_DRIVER_FLAG_INIT;
+ console_unlock();
/* ignore return value, binding should not fail */
- do_bind_con_driver(defcsw, first, last, deflt);
+ bind_con_driver(defcsw, first, last, deflt);
err:
module_put(owner);
return retval;
}
-EXPORT_SYMBOL_GPL(do_unbind_con_driver);
+EXPORT_SYMBOL(unbind_con_driver);
static int vt_bind(struct con_driver *con)
{
@@ -3524,18 +3508,28 @@
}
EXPORT_SYMBOL_GPL(con_debug_leave);
-static int do_register_con_driver(const struct consw *csw, int first, int last)
+/**
+ * register_con_driver - register console driver to console layer
+ * @csw: console driver
+ * @first: the first console to take over, minimum value is 0
+ * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
+ *
+ * DESCRIPTION: This function registers a console driver which can later
+ * bind to a range of consoles specified by @first and @last. It will
+ * also initialize the console driver by calling con_startup().
+ */
+int register_con_driver(const struct consw *csw, int first, int last)
{
struct module *owner = csw->owner;
struct con_driver *con_driver;
const char *desc;
int i, retval = 0;
- WARN_CONSOLE_UNLOCKED();
-
if (!try_module_get(owner))
return -ENODEV;
+ console_lock();
+
for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
con_driver = ®istered_con_driver[i];
@@ -3588,27 +3582,8 @@
}
err:
- module_put(owner);
- return retval;
-}
-
-/**
- * register_con_driver - register console driver to console layer
- * @csw: console driver
- * @first: the first console to take over, minimum value is 0
- * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1
- *
- * DESCRIPTION: This function registers a console driver which can later
- * bind to a range of consoles specified by @first and @last. It will
- * also initialize the console driver by calling con_startup().
- */
-int register_con_driver(const struct consw *csw, int first, int last)
-{
- int retval;
-
- console_lock();
- retval = do_register_con_driver(csw, first, last);
console_unlock();
+ module_put(owner);
return retval;
}
EXPORT_SYMBOL(register_con_driver);
@@ -3626,18 +3601,9 @@
*/
int unregister_con_driver(const struct consw *csw)
{
- int retval;
+ int i, retval = -ENODEV;
console_lock();
- retval = do_unregister_con_driver(csw);
- console_unlock();
- return retval;
-}
-EXPORT_SYMBOL(unregister_con_driver);
-
-int do_unregister_con_driver(const struct consw *csw)
-{
- int i, retval = -ENODEV;
/* cannot unregister a bound driver */
if (con_is_bound(csw))
@@ -3663,53 +3629,27 @@
}
}
err:
+ console_unlock();
return retval;
}
-EXPORT_SYMBOL_GPL(do_unregister_con_driver);
-
-/*
- * If we support more console drivers, this function is used
- * when a driver wants to take over some existing consoles
- * and become default driver for newly opened ones.
- *
- * take_over_console is basically a register followed by unbind
- */
-int do_take_over_console(const struct consw *csw, int first, int last, int deflt)
-{
- int err;
-
- err = do_register_con_driver(csw, first, last);
- /*
- * If we get an busy error we still want to bind the console driver
- * and return success, as we may have unbound the console driver
- * but not unregistered it.
- */
- if (err == -EBUSY)
- err = 0;
- if (!err)
- do_bind_con_driver(csw, first, last, deflt);
-
- return err;
-}
-EXPORT_SYMBOL_GPL(do_take_over_console);
+EXPORT_SYMBOL(unregister_con_driver);
/*
* If we support more console drivers, this function is used
* when a driver wants to take over some existing consoles
* and become default driver for newly opened ones.
*
- * take_over_console is basically a register followed by unbind
+ * take_over_console is basically a register followed by unbind
*/
int take_over_console(const struct consw *csw, int first, int last, int deflt)
{
int err;
err = register_con_driver(csw, first, last);
- /*
- * If we get an busy error we still want to bind the console driver
+ /* if we get an busy error we still want to bind the console driver
* and return success, as we may have unbound the console driver
- * but not unregistered it.
- */
+ * but not unregistered it.
+ */
if (err == -EBUSY)
err = 0;
if (!err)
Index: linux-3.2.41/drivers/video/console/fbcon.c
===================================================================
--- linux-3.2.41.orig/drivers/video/console/fbcon.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/video/console/fbcon.c 2013-04-12 09:39:46.239517112 +0400
@@ -530,33 +530,6 @@
return retval;
}
-static int do_fbcon_takeover(int show_logo)
-{
- int err, i;
-
- if (!num_registered_fb)
- return -ENODEV;
-
- if (!show_logo)
- logo_shown = FBCON_LOGO_DONTSHOW;
-
- for (i = first_fb_vc; i <= last_fb_vc; i++)
- con2fb_map[i] = info_idx;
-
- err = do_take_over_console(&fb_con, first_fb_vc, last_fb_vc,
- fbcon_is_default);
-
- if (err) {
- for (i = first_fb_vc; i <= last_fb_vc; i++)
- con2fb_map[i] = -1;
- info_idx = -1;
- } else {
- fbcon_has_console_bind = 1;
- }
-
- return err;
-}
-
static int fbcon_takeover(int show_logo)
{
int err, i;
@@ -3011,7 +2984,7 @@
{
int ret;
- ret = do_unbind_con_driver(&fb_con, first_fb_vc, last_fb_vc,
+ ret = unbind_con_driver(&fb_con, first_fb_vc, last_fb_vc,
fbcon_is_default);
if (!ret)
@@ -3084,7 +3057,7 @@
primary_device = -1;
if (!num_registered_fb)
- do_unregister_con_driver(&fb_con);
+ unregister_con_driver(&fb_con);
return 0;
}
@@ -3149,7 +3122,7 @@
}
if (info_idx != -1)
- ret = do_fbcon_takeover(1);
+ ret = fbcon_takeover(1);
} else {
for (i = first_fb_vc; i <= last_fb_vc; i++) {
if (con2fb_map_boot[i] == idx)
Index: linux-3.2.41/drivers/video/fbmem.c
===================================================================
--- linux-3.2.41.orig/drivers/video/fbmem.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/video/fbmem.c 2013-04-12 09:39:46.239517112 +0400
@@ -1628,9 +1628,7 @@
event.info = fb_info;
if (!lock_fb_info(fb_info))
return -ENODEV;
- console_lock();
fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
- console_unlock();
unlock_fb_info(fb_info);
return 0;
}
@@ -1646,10 +1644,8 @@
if (!lock_fb_info(fb_info))
return -ENODEV;
- console_lock();
event.info = fb_info;
ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event);
- console_unlock();
unlock_fb_info(fb_info);
if (ret)
@@ -1664,9 +1660,7 @@
num_registered_fb--;
fb_cleanup_device(fb_info);
event.info = fb_info;
- console_lock();
fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
- console_unlock();
/* this may free fb info */
put_fb_info(fb_info);
@@ -1837,8 +1831,11 @@
err = 1;
if (!list_empty(&info->modelist)) {
+ if (!lock_fb_info(info))
+ return -ENODEV;
event.info = info;
err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event);
+ unlock_fb_info(info);
}
return err;
Index: linux-3.2.41/include/linux/console.h
===================================================================
--- linux-3.2.41.orig/include/linux/console.h 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/include/linux/console.h 2013-04-12 09:39:46.239517112 +0400
@@ -77,9 +77,7 @@
int con_is_bound(const struct consw *csw);
int register_con_driver(const struct consw *csw, int first, int last);
int unregister_con_driver(const struct consw *csw);
-int do_unregister_con_driver(const struct consw *csw);
int take_over_console(const struct consw *sw, int first, int last, int deflt);
-int do_take_over_console(const struct consw *sw, int first, int last, int deflt);
void give_up_console(const struct consw *sw);
#ifdef CONFIG_HW_CONSOLE
int con_debug_enter(struct vc_data *vc);
Index: linux-3.2.41/include/linux/vt_kern.h
===================================================================
--- linux-3.2.41.orig/include/linux/vt_kern.h 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/include/linux/vt_kern.h 2013-04-12 09:36:57.000000000 +0400
@@ -132,8 +132,6 @@
int vt_waitactive(int n);
void change_console(struct vc_data *new_vc);
void reset_vc(struct vc_data *vc);
-extern int do_unbind_con_driver(const struct consw *csw, int first, int last,
- int deflt);
extern int unbind_con_driver(const struct consw *csw, int first, int last,
int deflt);
int vty_init(const struct file_operations *console_fops);
Index: linux-3.2.41/drivers/video/fbsysfs.c
===================================================================
--- linux-3.2.41.orig/drivers/video/fbsysfs.c 2013-03-20 19:03:42.000000000 +0400
+++ linux-3.2.41/drivers/video/fbsysfs.c 2013-04-12 09:39:46.239517112 +0400
@@ -175,8 +175,6 @@
if (i * sizeof(struct fb_videomode) != count)
return -EINVAL;
- if (!lock_fb_info(fb_info))
- return -ENODEV;
console_lock();
list_splice(&fb_info->modelist, &old_list);
fb_videomode_to_modelist((const struct fb_videomode *)buf, i,
@@ -188,7 +186,6 @@
fb_destroy_modelist(&old_list);
console_unlock();
- unlock_fb_info(fb_info);
return 0;
}
Reply to: