Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fb: rework locking to fix lock ordering on takeover

Adjust the console layer to allow a take over call where the caller
already holds the locks. Make the fb layer lock in order.

This is partly a band aid, the fb layer is terminally confused about the
locking rules it uses for its notifiers it seems.

[akpm@linux-foundation.org: remove stray non-ascii char, tidy comment]
[akpm@linux-foundation.org: export do_take_over_console()]
[airlied: cleanup another non-ascii char]
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: stable <stable@vger.kernel.org>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Dave Airlie <airlied@redhat.com>

authored by

Alan Cox and committed by
Dave Airlie
50e244cc 84b603ab

+104 -27
+70 -23
drivers/tty/vt/vt.c
··· 2987 2987 2988 2988 static struct class *vtconsole_class; 2989 2989 2990 - static int bind_con_driver(const struct consw *csw, int first, int last, 2990 + static int do_bind_con_driver(const struct consw *csw, int first, int last, 2991 2991 int deflt) 2992 2992 { 2993 2993 struct module *owner = csw->owner; ··· 2998 2998 if (!try_module_get(owner)) 2999 2999 return -ENODEV; 3000 3000 3001 - console_lock(); 3001 + WARN_CONSOLE_UNLOCKED(); 3002 3002 3003 3003 /* check if driver is registered */ 3004 3004 for (i = 0; i < MAX_NR_CON_DRIVER; i++) { ··· 3083 3083 3084 3084 retval = 0; 3085 3085 err: 3086 - console_unlock(); 3087 3086 module_put(owner); 3088 3087 return retval; 3089 3088 }; 3089 + 3090 + 3091 + static int bind_con_driver(const struct consw *csw, int first, int last, 3092 + int deflt) 3093 + { 3094 + int ret; 3095 + 3096 + console_lock(); 3097 + ret = do_bind_con_driver(csw, first, last, deflt); 3098 + console_unlock(); 3099 + return ret; 3100 + } 3090 3101 3091 3102 #ifdef CONFIG_VT_HW_CONSOLE_BINDING 3092 3103 static int con_is_graphics(const struct consw *csw, int first, int last) ··· 3210 3199 if (!con_is_bound(csw)) 3211 3200 con_driver->flag &= ~CON_DRIVER_FLAG_INIT; 3212 3201 3213 - console_unlock(); 3214 3202 /* ignore return value, binding should not fail */ 3215 - bind_con_driver(defcsw, first, last, deflt); 3203 + do_bind_con_driver(defcsw, first, last, deflt); 3204 + console_unlock(); 3216 3205 err: 3217 3206 module_put(owner); 3218 3207 return retval; ··· 3503 3492 } 3504 3493 EXPORT_SYMBOL_GPL(con_debug_leave); 3505 3494 3506 - /** 3507 - * register_con_driver - register console driver to console layer 3508 - * @csw: console driver 3509 - * @first: the first console to take over, minimum value is 0 3510 - * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1 3511 - * 3512 - * DESCRIPTION: This function registers a console driver which can later 3513 - * bind to a range of consoles specified by @first and @last. It will 3514 - * also initialize the console driver by calling con_startup(). 3515 - */ 3516 - int register_con_driver(const struct consw *csw, int first, int last) 3495 + static int do_register_con_driver(const struct consw *csw, int first, int last) 3517 3496 { 3518 3497 struct module *owner = csw->owner; 3519 3498 struct con_driver *con_driver; 3520 3499 const char *desc; 3521 3500 int i, retval = 0; 3522 3501 3502 + WARN_CONSOLE_UNLOCKED(); 3503 + 3523 3504 if (!try_module_get(owner)) 3524 3505 return -ENODEV; 3525 - 3526 - console_lock(); 3527 3506 3528 3507 for (i = 0; i < MAX_NR_CON_DRIVER; i++) { 3529 3508 con_driver = &registered_con_driver[i]; ··· 3567 3566 } 3568 3567 3569 3568 err: 3570 - console_unlock(); 3571 3569 module_put(owner); 3570 + return retval; 3571 + } 3572 + 3573 + /** 3574 + * register_con_driver - register console driver to console layer 3575 + * @csw: console driver 3576 + * @first: the first console to take over, minimum value is 0 3577 + * @last: the last console to take over, maximum value is MAX_NR_CONSOLES -1 3578 + * 3579 + * DESCRIPTION: This function registers a console driver which can later 3580 + * bind to a range of consoles specified by @first and @last. It will 3581 + * also initialize the console driver by calling con_startup(). 3582 + */ 3583 + int register_con_driver(const struct consw *csw, int first, int last) 3584 + { 3585 + int retval; 3586 + 3587 + console_lock(); 3588 + retval = do_register_con_driver(csw, first, last); 3589 + console_unlock(); 3572 3590 return retval; 3573 3591 } 3574 3592 EXPORT_SYMBOL(register_con_driver); ··· 3643 3623 * when a driver wants to take over some existing consoles 3644 3624 * and become default driver for newly opened ones. 3645 3625 * 3646 - * take_over_console is basically a register followed by unbind 3626 + * take_over_console is basically a register followed by unbind 3627 + */ 3628 + int do_take_over_console(const struct consw *csw, int first, int last, int deflt) 3629 + { 3630 + int err; 3631 + 3632 + err = do_register_con_driver(csw, first, last); 3633 + /* 3634 + * If we get an busy error we still want to bind the console driver 3635 + * and return success, as we may have unbound the console driver 3636 + * but not unregistered it. 3637 + */ 3638 + if (err == -EBUSY) 3639 + err = 0; 3640 + if (!err) 3641 + do_bind_con_driver(csw, first, last, deflt); 3642 + 3643 + return err; 3644 + } 3645 + EXPORT_SYMBOL_GPL(do_take_over_console); 3646 + 3647 + /* 3648 + * If we support more console drivers, this function is used 3649 + * when a driver wants to take over some existing consoles 3650 + * and become default driver for newly opened ones. 3651 + * 3652 + * take_over_console is basically a register followed by unbind 3647 3653 */ 3648 3654 int take_over_console(const struct consw *csw, int first, int last, int deflt) 3649 3655 { 3650 3656 int err; 3651 3657 3652 3658 err = register_con_driver(csw, first, last); 3653 - /* if we get an busy error we still want to bind the console driver 3659 + /* 3660 + * If we get an busy error we still want to bind the console driver 3654 3661 * and return success, as we may have unbound the console driver 3655 -  * but not unregistered it. 3656 - */ 3662 + * but not unregistered it. 3663 + */ 3657 3664 if (err == -EBUSY) 3658 3665 err = 0; 3659 3666 if (!err)
+28 -1
drivers/video/console/fbcon.c
··· 529 529 return retval; 530 530 } 531 531 532 + static int do_fbcon_takeover(int show_logo) 533 + { 534 + int err, i; 535 + 536 + if (!num_registered_fb) 537 + return -ENODEV; 538 + 539 + if (!show_logo) 540 + logo_shown = FBCON_LOGO_DONTSHOW; 541 + 542 + for (i = first_fb_vc; i <= last_fb_vc; i++) 543 + con2fb_map[i] = info_idx; 544 + 545 + err = do_take_over_console(&fb_con, first_fb_vc, last_fb_vc, 546 + fbcon_is_default); 547 + 548 + if (err) { 549 + for (i = first_fb_vc; i <= last_fb_vc; i++) 550 + con2fb_map[i] = -1; 551 + info_idx = -1; 552 + } else { 553 + fbcon_has_console_bind = 1; 554 + } 555 + 556 + return err; 557 + } 558 + 532 559 static int fbcon_takeover(int show_logo) 533 560 { 534 561 int err, i; ··· 3142 3115 } 3143 3116 3144 3117 if (info_idx != -1) 3145 - ret = fbcon_takeover(1); 3118 + ret = do_fbcon_takeover(1); 3146 3119 } else { 3147 3120 for (i = first_fb_vc; i <= last_fb_vc; i++) { 3148 3121 if (con2fb_map_boot[i] == idx)
+2 -3
drivers/video/fbmem.c
··· 1650 1650 event.info = fb_info; 1651 1651 if (!lock_fb_info(fb_info)) 1652 1652 return -ENODEV; 1653 + console_lock(); 1653 1654 fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event); 1655 + console_unlock(); 1654 1656 unlock_fb_info(fb_info); 1655 1657 return 0; 1656 1658 } ··· 1855 1853 err = 1; 1856 1854 1857 1855 if (!list_empty(&info->modelist)) { 1858 - if (!lock_fb_info(info)) 1859 - return -ENODEV; 1860 1856 event.info = info; 1861 1857 err = fb_notifier_call_chain(FB_EVENT_NEW_MODELIST, &event); 1862 - unlock_fb_info(info); 1863 1858 } 1864 1859 1865 1860 return err;
+3
drivers/video/fbsysfs.c
··· 177 177 if (i * sizeof(struct fb_videomode) != count) 178 178 return -EINVAL; 179 179 180 + if (!lock_fb_info(fb_info)) 181 + return -ENODEV; 180 182 console_lock(); 181 183 list_splice(&fb_info->modelist, &old_list); 182 184 fb_videomode_to_modelist((const struct fb_videomode *)buf, i, ··· 190 188 fb_destroy_modelist(&old_list); 191 189 192 190 console_unlock(); 191 + unlock_fb_info(fb_info); 193 192 194 193 return 0; 195 194 }
+1
include/linux/console.h
··· 78 78 int register_con_driver(const struct consw *csw, int first, int last); 79 79 int unregister_con_driver(const struct consw *csw); 80 80 int take_over_console(const struct consw *sw, int first, int last, int deflt); 81 + int do_take_over_console(const struct consw *sw, int first, int last, int deflt); 81 82 void give_up_console(const struct consw *sw); 82 83 #ifdef CONFIG_HW_CONSOLE 83 84 int con_debug_enter(struct vc_data *vc);