From patchwork Wed Jun 28 10:36:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 9813911 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 154AF603F3 for ; Wed, 28 Jun 2017 10:36:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 08A771FF60 for ; Wed, 28 Jun 2017 10:36:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F182C2040D; Wed, 28 Jun 2017 10:36:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9A7121FF60 for ; Wed, 28 Jun 2017 10:36:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751501AbdF1Kgo (ORCPT ); Wed, 28 Jun 2017 06:36:44 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35312 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751469AbdF1Kgm (ORCPT ); Wed, 28 Jun 2017 06:36:42 -0400 Received: by mail-wm0-f66.google.com with SMTP id 131so10642067wmq.2 for ; Wed, 28 Jun 2017 03:36:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id; bh=J6Q1oMs2sX0+kDgJ+vOIgANEdI8VA3/isxAeeTpovGg=; b=IRe1gUYTwgoZJhgbpDaLcvhO3rEq4OLclEEugGGdgbQshW+2doYbD4qKaRQHAh/Q98 5xNJrKllyp1gIPteG8blSR9u8PsmODq51aqrUYHNeSlLSRIcLMM0YwtQHOvwPEnMLPIw j9n/s4T0oy8p4/oNO98lo8vkM/U1k+J9bqEsQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=J6Q1oMs2sX0+kDgJ+vOIgANEdI8VA3/isxAeeTpovGg=; b=lHln2FXXyRQp/Jx8eq2Grc+00juzQFKJy+0lb3+ov+IniF1JABdzswISjxXcaWMOZM upKPyJlz3D0or0j7CZIOR0Pu+lKI2zNi49Kuu0MuE2HtlCwItOhc5w1ZsQ6ONQ2/6G2o FOtjfCk9pcXaWMco7iR1MH5pNBiQpYr2o5gfL98N1lNnIlTbA6tPmJ8EKxtr6J7Afulk 5F7S8Y8Qq/bR7ZzEqM7/6QpAut1UagN2zV+V33w37qqXNTNNpGukuRfWFxBF+pEh97o3 o3/j/5UKQqhKWT+AYcjVnfoRQjY8g1skjsyBBFBSqHdedsFK2mXGyiyqFTPOhyMMgBsE 9ZSQ== X-Gm-Message-State: AKS2vOw5fD8t4WTLtXGViCOkJ5hGETQxrhM+RHiRkFmRFcGyW8uK+23E GszV+8HtsnQJbvs1 X-Received: by 10.80.210.197 with SMTP id q5mr2268717edg.147.1498646200761; Wed, 28 Jun 2017 03:36:40 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:5640:0:960b:2678:e223:c1c6]) by smtp.gmail.com with ESMTPSA id e50sm985304edb.43.2017.06.28.03.36.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Jun 2017 03:36:40 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Cc: LKML , Daniel Vetter , Alan Cox , Sergey Senozhatsky , Linux Fbdev development list , Steven Rostedt , Bartlomiej Zolnierkiewicz , dri-devel@lists.freedesktop.org, Daniel Vetter Subject: [PATCH] fbcon: Make fbcon a built-time depency for fbdev Date: Wed, 28 Jun 2017 12:36:35 +0200 Message-Id: <20170628103635.24651-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.11.0 Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP There's a bunch of folks who're trying to make printk less contended and faster, but there's a problem: printk uses the console_lock, and the console lock has become the BKL for all things fbdev/fbcon, which in turn pulled in half the drm subsystem under that lock. That's awkward. There reasons for that is probably just a historical accident: - fbcon is a runtime option of fbdev, i.e. at runtime you can pick whether your fbdev driver instances are used as kernel consoles. Unfortunately this wasn't implemented with some module option, but through some module loading magic: As long as you don't load fbcon.ko, there's no fbdev console support, but loading it (in any order wrt fbdev drivers) will create console instances for all fbdev drivers. - This was implemented through a notifier chain. fbcon.ko enumerates all fbdev instances at load time and also registers itself as listener in the fbdev notifier. The fbdev core tries to register new fbdev instances with fbcon using the notifier. - On top of that the modifier chain is also used at runtime by the fbdev subsystem to e.g. control backlights for panels. - The problem is that the notifier puts a mutex locking context between fbdev and fbcon, which mixes up the locking contexts for both the runtime usage and the register time usage to notify fbcon. And at runtime fbcon (through the fbdev core) might call into the notifier from a printk critical section while console_lock is held. - This means console_lock must be an outer lock for the entire fbdev subsystem, which also means it must be acquired when registering a new framebuffer driver as the outermost lock since we might call into fbcon (through the notifier) which would result in a locking inversion if fbcon would acquire the console_lock from its notifier callback (which it needs to register the console). - console_lock can be held anywhere, since printk can be called anywhere, and through the above story, plus drm/kms being an fbdev driver, we pull in a shocking amount of locking hiercharchy underneath the console_lock. Which makes cleaning up printk really hard (not even splitting console_lock into an rwsem is all that useful due to this). There's various ways to address this, but the cleanest would be to make fbcon a compile-time option, where fbdev directly calls the fbcon register functions from register_framebuffer, or dummy static inline versions if fbcon is disabled. Maybe augmented with a runtime knob to disable fbcon, if that's needed (for debugging perhaps). But this could break some users who rely on the magic "loading fbcon.ko enables/disables fbdev framebuffers at runtime" thing, even if that's unlikely. Hence we must be careful: 1. Create a compile-time dependency between fbcon and fbdev in the least minimal way. This is what this patch does. 2. Wait at least 1 year to give possible users time to scream about how we broke their setup. Unlikely, since all distros make fbcon compile-in, and embedded platforms only compile stuff they know they need anyway. But still. 3. Convert the notifier to direct functions calls, with dummy static inlines if fbcon is disabled. We'll still need the fb notifier for the other uses (like backlights), but we can probably move it into the fb core (atm it must be built-into vmlinux). 4. Push console_lock down the call-chain, until it is down in console_register again. 5. Finally start to clean up and rework the printk/console locking. For context of this saga see commit 50e244cc793d511b86adea24972f3a7264cae114 Author: Alan Cox Date: Fri Jan 25 10:28:15 2013 +1000 fb: rework locking to fix lock ordering on takeover plus the pile of commits on top that tried to make this all work without terminally upsetting lockdep. We've uncovered all this when console_lock lockdep annotations where added in commit daee779718a319ff9f83e1ba3339334ac650bb22 Author: Daniel Vetter Date: Sat Sep 22 19:52:11 2012 +0200 console: implement lockdep support for console_lock On the patch itself: - Add fbcon.h for the dummy symbol to force the module load ordering between fbcon.ko and fb.ko. In step 3 that's hopefully going to be the place for the fbcon register/unregister functions. - Switch CONFIG_FRAMEBUFFER_CONSOLE to be a boolean, using the overall CONFIG_FB tristate to decided whether it should be a module or built-in. Cc: Alan Cox Cc: Sergey Senozhatsky Cc: Linux Fbdev development list Cc: Steven Rostedt Cc: Bartlomiej Zolnierkiewicz Cc: dri-devel@lists.freedesktop.org Signed-off-by: Daniel Vetter Reviewed-by: Alan Cox --- drivers/video/console/Kconfig | 2 +- drivers/video/console/Makefile | 8 +++++--- drivers/video/console/fbcon.c | 8 ++++++++ drivers/video/fbdev/core/fbmem.c | 4 ++++ include/linux/fbcon.h | 6 ++++++ 5 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 include/linux/fbcon.h diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index 2111d06f8c81..7f1f1fbcef9e 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -117,7 +117,7 @@ config DUMMY_CONSOLE_ROWS Select 25 if you use a 640x480 resolution by default. config FRAMEBUFFER_CONSOLE - tristate "Framebuffer Console support" + bool "Framebuffer Console support" depends on FB && !UML select VT_HW_CONSOLE_BINDING select CRC32 diff --git a/drivers/video/console/Makefile b/drivers/video/console/Makefile index 43bfa485db96..8db6f712585b 100644 --- a/drivers/video/console/Makefile +++ b/drivers/video/console/Makefile @@ -7,13 +7,15 @@ obj-$(CONFIG_SGI_NEWPORT_CONSOLE) += newport_con.o obj-$(CONFIG_STI_CONSOLE) += sticon.o sticore.o obj-$(CONFIG_VGA_CONSOLE) += vgacon.o obj-$(CONFIG_MDA_CONSOLE) += mdacon.o -obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon.o bitblit.o softcursor.o +ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE),y) +obj-$(CONFIG_FB) += fbcon.o bitblit.o softcursor.o ifeq ($(CONFIG_FB_TILEBLITTING),y) -obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += tileblit.o +obj-$(CONFIG_FB) += tileblit.o endif ifeq ($(CONFIG_FRAMEBUFFER_CONSOLE_ROTATION),y) -obj-$(CONFIG_FRAMEBUFFER_CONSOLE) += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \ +obj-$(CONFIG_FB) += fbcon_rotate.o fbcon_cw.o fbcon_ud.o \ fbcon_ccw.o endif +endif obj-$(CONFIG_FB_STI) += sticore.o diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c index 12ded23f1aaf..f97277299b7c 100644 --- a/drivers/video/console/fbcon.c +++ b/drivers/video/console/fbcon.c @@ -68,6 +68,7 @@ #include #include #include +#include #include #include #include @@ -3631,6 +3632,13 @@ static int __init fb_console_init(void) return 0; } +/* + * Dummy export to force a hard depency between fbcon and fbdev core if fbcon is + * enabled. + */ +int fbcon_is_available = 1; +EXPORT_SYMBOL(fbcon_is_available); + fs_initcall(fb_console_init); #ifdef MODULE diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 069fe7960df1..e5556726bc1c 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -1796,6 +1797,9 @@ int register_framebuffer(struct fb_info *fb_info) { int ret; +#ifdef CONFIG_FRAMEBUFFER_CONSOLE + WARN_ON(!fbcon_is_available); +#endif mutex_lock(®istration_lock); ret = do_register_framebuffer(fb_info); diff --git a/include/linux/fbcon.h b/include/linux/fbcon.h new file mode 100644 index 000000000000..b23d011214c0 --- /dev/null +++ b/include/linux/fbcon.h @@ -0,0 +1,6 @@ +#ifndef _LINUX_FBCON_H +#define _LINUX_FBCON_H + +extern int fbcon_is_available; + +#endif /* _LINUX_FBCON_H */