On Thursday, July 06, 2017 02:57:32 PM Daniel Vetter wrote: > 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 <alan@xxxxxxxxxxxxxxx> > 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 <daniel.vetter@xxxxxxxx> > Date: Sat Sep 22 19:52:11 2012 +0200 > > console: implement lockdep support for console_lock > > On the patch itself: > - 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. > > - At first I thought I could force the build depency with just a dummy > symbol that fbcon.ko exports and fb.ko uses. But that leads to a > module depency cycle (it works fine when built-in). > > Since this tight binding is the entire goal the simplest solution is > to move all the fbcon modules (and there's a bunch of optinal > source-files which are each modules of their own, for no good > reason) into the overall fb.ko core module. That's a bit more than > what I would have liked to do in this patch, but oh well. > > Cc: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> > Cc: Sergey Senozhatsky <sergey.senozhatsky.work@xxxxxxxxx> > Cc: Linux Fbdev development list <linux-fbdev@xxxxxxxxxxxxxxx> > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > --- > v2: Switch to building fbcon code into fb.ko right away because the > cheap trick leads to a module depency loop. > --- Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
