All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 00/33] fbcon notifier begone!
Date: Mon, 20 May 2019 10:21:43 +0200	[thread overview]
Message-ID: <20190520082216.26273-1-daniel.vetter@ffwll.ch> (raw)

Hi all,

This patch series removes the fbcon notifier. It also contains the
beginnings of trying to understand how locking in this area works.

The super short summary of locking rules is that everything in fbcon needs
to be protected by the monolithic console_lock, including the setup code.
So not really an option to pull the kms modeset code out from underneath
that, at least not without rewriting half the console layer. Or I'm not
yet seeing clear enough.

The other side is that fbdev userspace access vs. fbcon locking is totally
broken.

For context of this series I'm just going to quote the commit message that
started this all:

commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Aug 1 17:32:07 2017 +0200

    fbcon: Make fbcon a built-time depency for fbdev
    
    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@linux.intel.com>
    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@ffwll.ch>
    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.
    
    Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
    Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
    Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
    Cc: Steven Rostedt <rostedt@goodmis.org>
    Reviewed-by: Sean Paul <seanpaul@chromium.org>
    Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Comments, review and testing much appreciated, as always.

Wrt long-term plans: I'm not sure where this will lead just yet, or
whether it'll lead anywhere at all. But I think removing the notifier
indirection is definitely a step into the right direction.

Cheers, Daniel

Daniel Vetter (33):
  dummycon: Sprinkle locking checks
  fbdev: locking check for fb_set_suspend
  vt: might_sleep() annotation for do_blank_screen
  vt: More locking checks
  fbdev/sa1100fb: Remove dead code
  fbdev/cyber2000: Remove struct display
  fbdev/aty128fb: Remove dead code
  fbcon: s/struct display/struct fbcon_display/
  fbcon: Remove fbcon_has_exited
  fbcon: call fbcon_fb_(un)registered directly
  fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify
  fbdev/omap: sysfs files can't disappear before the device is gone
  fbdev: sysfs files can't disappear before the device is gone
  staging/olpc: lock_fb_info can't fail
  fbdev/atyfb: lock_fb_info can't fail
  fbdev: lock_fb_info cannot fail
  fbcon: call fbcon_fb_bind directly
  fbdev: make unregister/unlink functions not fail
  fbdev: unify unlink_framebufer paths
  fbdev/sh_mob: Remove fb notifier callback
  fbdev: directly call fbcon_suspended/resumed
  fbcon: Call fbcon_mode_deleted/new_modelist directly
  fbdev: Call fbcon_get_requirement directly
  Revert "backlight/fbcon: Add FB_EVENT_CONBLANK"
  fbcon: directly call fbcon_fb_blanked
  fbmem: pull fbcon_fb_blanked out of fb_blank
  fbdev: remove FBINFO_MISC_USEREVENT around fb_blank
  fb: Flatten control flow in fb_set_var
  fbcon: replace FB_EVENT_MODE_CHANGE/_ALL with direct calls
  vgaswitcheroo: call fbcon_remap_all directly
  fbcon: Call con2fb_map functions directly
  fbcon: Document what I learned about fbcon locking
  staging/olpc_dcon: Add drm conversion to TODO

 drivers/gpu/vga/vga_switcheroo.c              |  11 +-
 drivers/staging/olpc_dcon/TODO                |   7 +
 drivers/staging/olpc_dcon/olpc_dcon.c         |   6 +-
 drivers/tty/vt/vt.c                           |  18 +
 drivers/video/backlight/backlight.c           |   2 +-
 drivers/video/backlight/lcd.c                 |   2 -
 drivers/video/console/dummycon.c              |   6 +
 drivers/video/fbdev/aty/aty128fb.c            |  64 ---
 drivers/video/fbdev/aty/atyfb_base.c          |   3 +-
 drivers/video/fbdev/core/fbcmap.c             |   6 +-
 drivers/video/fbdev/core/fbcon.c              | 303 ++++++--------
 drivers/video/fbdev/core/fbcon.h              |   6 +-
 drivers/video/fbdev/core/fbmem.c              | 374 ++++++------------
 drivers/video/fbdev/core/fbsysfs.c            |  20 +-
 drivers/video/fbdev/cyber2000fb.c             |   1 -
 .../video/fbdev/omap2/omapfb/omapfb-sysfs.c   |  21 +-
 drivers/video/fbdev/sa1100fb.c                |  25 --
 drivers/video/fbdev/sh_mobile_lcdcfb.c        | 112 +-----
 drivers/video/fbdev/sh_mobile_lcdcfb.h        |   5 -
 include/linux/console_struct.h                |   5 +-
 include/linux/fb.h                            |  40 +-
 include/linux/fbcon.h                         |  30 ++
 22 files changed, 342 insertions(+), 725 deletions(-)

-- 
2.20.1


             reply	other threads:[~2019-05-20  8:22 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-20  8:21 Daniel Vetter [this message]
2019-05-20  8:21 ` [PATCH 01/33] dummycon: Sprinkle locking checks Daniel Vetter
2019-05-20  8:21 ` [PATCH 02/33] fbdev: locking check for fb_set_suspend Daniel Vetter
2019-05-20  8:21 ` [PATCH 03/33] vt: might_sleep() annotation for do_blank_screen Daniel Vetter
2019-05-20  8:21 ` [PATCH 04/33] vt: More locking checks Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 05/33] fbdev/sa1100fb: Remove dead code Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 06/33] fbdev/cyber2000: Remove struct display Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 07/33] fbdev/aty128fb: Remove dead code Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 08/33] fbcon: s/struct display/struct fbcon_display/ Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 09/33] fbcon: Remove fbcon_has_exited Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-21 14:23   ` [PATCH] " Daniel Vetter
2019-05-22 10:04     ` Bartlomiej Zolnierkiewicz
2019-05-22 10:04       ` Bartlomiej Zolnierkiewicz
2019-05-22 10:38       ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 10/33] fbcon: call fbcon_fb_(un)registered directly Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:33   ` Thomas Zimmermann
2019-05-20  8:33     ` Thomas Zimmermann
2019-05-20  8:37     ` Thomas Zimmermann
2019-05-20  8:37       ` Thomas Zimmermann
2019-05-21 15:09       ` Daniel Vetter
2019-05-21 15:09         ` Daniel Vetter
2019-05-20 17:08   ` Sam Ravnborg
2019-05-20 17:08     ` Sam Ravnborg
2019-05-20 17:08     ` Sam Ravnborg
2019-05-20 17:25     ` Daniel Vetter
2019-05-20 17:25       ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 11/33] fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  9:04   ` Geert Uytterhoeven
2019-05-20  8:21 ` [PATCH 12/33] fbdev/omap: sysfs files can't disappear before the device is gone Daniel Vetter
2019-05-20  8:21 ` [PATCH 13/33] fbdev: " Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 14/33] staging/olpc: lock_fb_info can't fail Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 15/33] fbdev/atyfb: " Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:21 ` [PATCH 16/33] fbdev: lock_fb_info cannot fail Daniel Vetter
2019-05-20  8:21   ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 17/33] fbcon: call fbcon_fb_bind directly Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 18/33] fbdev: make unregister/unlink functions not fail Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20 19:08   ` [Intel-gfx] " kbuild test robot
2019-05-20 19:08     ` kbuild test robot
2019-05-20 19:08     ` kbuild test robot
2019-05-20 19:25   ` kbuild test robot
2019-05-20 19:25     ` kbuild test robot
2019-05-20 19:25     ` [Intel-gfx] " kbuild test robot
2019-05-20 21:45   ` kbuild test robot
2019-05-20 21:45     ` kbuild test robot
2019-05-20 21:45     ` kbuild test robot
2019-05-20  8:22 ` [PATCH 19/33] fbdev: unify unlink_framebufer paths Daniel Vetter
2019-05-21 10:52   ` Maarten Lankhorst
2019-05-20  8:22 ` [PATCH 20/33] fbdev/sh_mob: Remove fb notifier callback Daniel Vetter
2019-05-20  9:05   ` Geert Uytterhoeven
2019-05-20  9:19     ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 21/33] fbdev: directly call fbcon_suspended/resumed Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20 19:24   ` kbuild test robot
2019-05-20 19:24     ` kbuild test robot
2019-05-20 19:24     ` kbuild test robot
2019-05-20  8:22 ` [PATCH 22/33] fbcon: Call fbcon_mode_deleted/new_modelist directly Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 23/33] fbdev: Call fbcon_get_requirement directly Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 24/33] Revert "backlight/fbcon: Add FB_EVENT_CONBLANK" Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 25/33] fbcon: directly call fbcon_fb_blanked Daniel Vetter
2019-05-20  8:22 ` [PATCH 26/33] fbmem: pull fbcon_fb_blanked out of fb_blank Daniel Vetter
2019-05-20  8:22 ` [PATCH 27/33] fbdev: remove FBINFO_MISC_USEREVENT around fb_blank Daniel Vetter
2019-05-20 17:20   ` Sam Ravnborg
2019-05-20 17:20     ` Sam Ravnborg
2019-05-20 17:29     ` Daniel Vetter
2019-05-20 17:29       ` Daniel Vetter
2019-05-20 17:53       ` Sam Ravnborg
2019-05-20  8:22 ` [PATCH 28/33] fb: Flatten control flow in fb_set_var Daniel Vetter
2019-05-20  8:22 ` [PATCH 29/33] fbcon: replace FB_EVENT_MODE_CHANGE/_ALL with direct calls Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-21 10:56   ` Maarten Lankhorst
2019-05-21 10:56     ` Maarten Lankhorst
2019-05-21 12:42     ` Daniel Vetter
2019-05-21 12:42       ` Daniel Vetter
2019-05-21 12:42       ` Daniel Vetter
2019-05-20  8:22 ` [PATCH 30/33] vgaswitcheroo: call fbcon_remap_all directly Daniel Vetter
2019-05-20  8:22   ` Daniel Vetter
2019-05-20  8:37   ` Lukas Wunner
2019-05-20  8:37     ` Lukas Wunner
2019-05-20  8:22 ` [PATCH 31/33] fbcon: Call con2fb_map functions directly Daniel Vetter
2019-05-20 19:28   ` [Intel-gfx] " kbuild test robot
2019-05-20 19:28     ` kbuild test robot
2019-05-20 19:34   ` [Intel-gfx] " kbuild test robot
2019-05-20 19:34     ` kbuild test robot
2019-05-20  8:22 ` [PATCH 32/33] fbcon: Document what I learned about fbcon locking Daniel Vetter
2019-05-21 11:13   ` Maarten Lankhorst
2019-05-20  8:22 ` [PATCH 33/33] staging/olpc_dcon: Add drm conversion to TODO Daniel Vetter
2019-05-20  8:30 ` ✗ Fi.CI.CHECKPATCH: warning for fbcon notifier begone! Patchwork
2019-05-20  8:41 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-05-20  8:44 ` ✗ Fi.CI.SPARSE: warning " Patchwork
2019-05-21 17:08 ` ✗ Fi.CI.CHECKPATCH: warning for fbcon notifier begone! (rev2) Patchwork
2019-05-21 17:22 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-05-21 17:29 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-05-22  8:15 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-22 18:43 ` ✓ Fi.CI.IGT: " Patchwork
2019-05-24  8:53 [PATCH 00/33] fbcon notifier begone! Daniel Vetter
2019-05-24  8:53 ` Daniel Vetter
2019-05-25 17:19 ` Sam Ravnborg
2019-05-27  7:17   ` Daniel Vetter
2019-05-27 11:56     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190520082216.26273-1-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH 00/33] fbcon notifier begone'\!'' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.