Inter-revision diff: cover letter

Comparing v3 (message) to v2 (message)

--- v3
+++ v2
@@ -1,29 +1,19 @@
-This is v3 of a series to prepare for threaded/atomic
-printing. v2 is here [0]. This series focuses on reducing the
-scope of the BKL console_lock. It achieves this by switching to
-SRCU and a dedicated mutex for console list iteration and
-modification, respectively. The console_lock will no longer
-offer this protection and is completely removed from
+This is v2 of a series to prepare for threaded/atomic
+printing. It is a rework of patches 6-12 of the v1 [0]. From
+the v1, patches 1-5 are already mainline and a rework of
+patches >12 will be posted in a later series.
+
+This series focuses on reducing the scope of the BKL
+console_lock. It achieves this by switching to SRCU and a
+dedicated mutex for console list iteration and modification,
+respectively. The console_lock will no longer offer this
+protection and is completely removed from
 (un)register_console() and console_stop/start() code.
-
-Also, during the review of v2 it came to our attention that
-many console drivers are checking CON_ENABLED to see if they
-are registered. Because this flag can change without
-unregistering and because this flag does not represent an
-atomic point when an (un)registration process is complete,
-a new console_is_registered() function is introduced. This
-function uses the console_list_lock to synchronize with the
-(un)registration process to provide a reliable status.
 
 All users of the console_lock for list iteration have been
 modified. For the call sites where the console_lock is still
-needed (because of other reasons), comments are added to
-explain exactly why the console_lock was needed.
-
-All users of CON_ENABLED for registration status have been
-modified to use console_is_registered(). Note that there are
-still users of CON_ENABLED, but this is for legitimate purposes
-about a registered console being able to print.
+needed (because of other reasons), I added comments to explain
+exactly why the console_lock was needed.
 
 The base commit for this series is from Paul McKenney's RCU tree
 and provides an NMI-safe SRCU implementation [1]. Without the
@@ -33,171 +23,176 @@
 now. Especially since it _does_ increase the reliability for
 mainline in the panic path.
 
-Changes since v3:
+Changes since v2:
 
 general:
 
-- introduce a synchronized console_is_registered() to query if
-  a console is registered, meant to replace CON_ENABLED
-  (mis)use for this purpose
+- introduce console_is_enabled() to document safe data race on
+  console->flags
 
-- directly read console->flags for registered consoles if it is
-  race-free (and document that it is so)
+- switch all "console->flags & CON_ENABLED" code sites to
+  console_is_enabled()
 
-- replace uart_console_enabled() with a new
-  uart_console_registered() based on console_is_registered()
+- add "for_each_console_srcu" to .clang-format
 
-- change comments about why console_lock is used to synchronize
-  console->device() by providing an example
+- cleanup/clarify comments relating to console_lock
+  coverage/usage
 
-registration check fixups:
+um:
 
-- the following drivers were modified to use the new
-  console_is_registered() instead of CON_ENABLED checks
-
-   - arch/m68k/emu/nfcon.c
-   - drivers/firmware/efi/earlycon.c
-   - drivers/net/netconsole.c
-   - drivers/tty/hvc/hvc_console.c
-   - drivers/tty/serial/8250/8250_core.c
-   - drivers/tty/serial/earlycon.c
-   - drivers/tty/serial/pic32_uart.c
-   - drivers/tty/serial/samsung_tty.c
-   - drivers/tty/serial/serial_core.c
-   - drivers/tty/serial/xilinx_uartps.c
-   - drivers/usb/early/xhci-dbc.c
-
-um: kmsg_dumper:
-
-- change stdout dump criteria to match original intention
+- kmsg_dumper: use srcu instead of console_lock for list
+  iteration
 
 kgdb/kdb:
 
-- in configure_kgdboc(), take console_list_lock to synchronize
-  tty_find_polling_driver() against register_console()
+- configure_kgdboc: keep console_lock for console->device()
+  synchronization, use srcu for list iteration
 
-- add comments explaining why calling console->write() without
-  locking might work
+- kgdboc_earlycon_pre_exp_handler: use srcu instead of
+  documenting unsafety for list iteration
 
-tty: sh-sci:
+- kgdboc_earlycon_init: use console_list_lock instead of
+  console_lock to lock list
 
-- use a setup() callback to setup the early console
+- kdb_msg_write: use srcu instead of documenting unsafety for
+  list iteration
 
-fbdev: xen:
+tty:
 
-- implement a cleaner approach for
-  console_force_preferred_locked()
+- show_cons_active: keep console_lock for console->device()
+  synchronization
 
-rcu:
+fbdev:
 
-- implement debug_lockdep_rcu_enabled() for
-  !CONFIG_DEBUG_LOCK_ALLOC
+- xen-fbfront: xenfb_probe: use srcu instead of console_lock
+  for list iteration, introduce console_force_preferred() to
+  safely implement hack
+
+proc/consoles:
+
+- show_console_dev: keep console_lock for console->device()
+  synchronization
+
+- c_next: use hlist_entry_safe() instead of
+  hlist_for_each_entry_continue()
 
 printk:
 
-- check CONFIG_DEBUG_LOCK_ALLOC for srcu_read_lock_held()
-  availability
+- remove console_lock from console_stop/start() and
+  (un)register_console()
 
-- for console_lock/_trylock/_unlock, replace "lock the console
-  system" language with "block the console subsystem from
-  printing"
+- introduce console_srcu_read_(un)lock() to wrap scru read
+  (un)lock
 
-- use WRITE_ONCE() for updating console->flags of registered
-  consoles
+- rename cons_first() macro to console_first()
 
-- expand comments of synchronize_srcu() calls to explain why
-  they are needed, and also expand comments to explain when it
-  is not needed
+- for_each_console: add lockdep check instead of introducing
+  new for_each_registered_console()
 
-- change CON_BOOT consoles to always begin at earliest message
+- console_list_lock: add warning if in read-side critical
+  section
 
-- for non-BOOT/non-PRINTBUFFER consoles, initialize @seq to the
-  minimal @seq of any of the enabled boot consoles
+- release srcu read lock on handover
 
-- add comments and lockdep assertion to
-  unregister_console_locked() because it is not clear from the
-  name which lock is implied
+- console_flush_all: use srcu instead of relying on console
+  lock for list iteration
 
-- dropped patches that caused unnecessary churn in the series
+- console_unblank: use srcu instead of relying on console_lock
+  for list iteration
+
+- console_flush_on_panic: use srcu for list iteration and
+  document console->seq race
+
+- device: keep console_lock for console->device()
+  synchronization, usr srcu for list iteration
+
+- register_console: split list adding logic into the 3 distinct
+  scenarios
+
+- register_console: set initial sequence number before adding
+  to list
+
+- unregister_console: fix ENODEV return value if the console is
+  not registered
+
+- console_stop: synchronize srcu
+
+- printk_late_init: use _safe variant of iteration
+
+- __pr_flush: use srcu instead of relying on console_lock for
+  list iteration
 
 John Ogness
 
-[0] https://lore.kernel.org/lkml/20221019145600.1282823-1-john.ogness@linutronix.de
-[1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/log/?h=srcunmisafe.2022.10.21a
+[0] https://lore.kernel.org/r/20220924000454.3319186-1-john.ogness@linutronix.de
+[1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/log/?h=srcunmisafe.2022.10.18b
 
-John Ogness (38):
-  rcu: implement lockdep_rcu_enabled for !CONFIG_DEBUG_LOCK_ALLOC
+John Ogness (37):
+  printk: Convert console_drivers list to hlist
   printk: Prepare for SRCU console list protection
-  printk: fix setting first seq for consoles
-  um: kmsg_dump: only dump when no output console available
-  console: introduce console_is_enabled() wrapper
+  printk: introduce console_is_enabled() wrapper
   printk: use console_is_enabled()
+  tty: nfcon: use console_is_enabled()
   um: kmsg_dump: use console_is_enabled()
+  efi: earlycon: use console_is_enabled()
+  netconsole: use console_is_enabled()
+  tty: hvc: use console_is_enabled()
+  tty: serial: earlycon: use console_is_enabled()
+  tty: serial: kgdboc: use console_is_enabled()
+  tty: serial: pic32_uart: use console_is_enabled()
+  tty: serial: samsung_tty: use console_is_enabled()
+  tty: serial: serial_core: use console_is_enabled()
+  tty: serial: xilinx_uartps: use console_is_enabled()
+  tty: tty_io: use console_is_enabled()
+  usb: early: xhci-dbc: use console_is_enabled()
   kdb: kdb_io: use console_is_enabled()
   um: kmsg_dumper: use srcu console list iterator
-  tty: serial: kgdboc: document console_lock usage
+  serial: kgdboc: use srcu console list iterator
+  serial: kgdboc: document console_lock usage
   tty: tty_io: document console_lock usage
+  xen: fbfront: use srcu console list iterator
   proc: consoles: document console_lock usage
   kdb: use srcu console list iterator
   printk: console_flush_all: use srcu console list iterator
   printk: console_unblank: use srcu console list iterator
   printk: console_flush_on_panic: use srcu console list iterator
   printk: console_device: use srcu console list iterator
+  printk: register_console: use srcu console list iterator
   printk: __pr_flush: use srcu console list iterator
   printk: introduce console_list_lock
-  console: introduce console_is_registered()
-  serial_core: replace uart_console_enabled() with
-    uart_console_registered()
-  tty: nfcon: use console_is_registered()
-  efi: earlycon: use console_is_registered()
-  tty: hvc: use console_is_registered()
-  tty: serial: earlycon: use console_is_registered()
-  tty: serial: pic32_uart: use console_is_registered()
-  tty: serial: samsung_tty: use console_is_registered()
-  tty: serial: xilinx_uartps: use console_is_registered()
-  usb: early: xhci-dbc: use console_is_registered()
-  netconsole: avoid CON_ENABLED misuse to track registration
-  printk, xen: fbfront: create/use safe function for forcing preferred
+  serial: kgdboc: use console_list_lock instead of console_lock
   tty: tty_io: use console_list_lock for list synchronization
   proc: consoles: use console_list_lock for list iteration
-  tty: serial: kgdboc: use console_list_lock for list traversal
-  tty: serial: kgdboc: synchronize tty_find_polling_driver() and
-    register_console()
-  tty: serial: kgdboc: use console_list_lock to trap exit
   printk: relieve console_lock of list synchronization duties
-  tty: serial: sh-sci: use setup() callback for early console
+  printk, xen: fbfront: create/use safe function for forcing preferred
 
-Thomas Gleixner (2):
+Thomas Gleixner (1):
   serial: kgdboc: Lock console list in probe function
-  printk: Convert console_drivers list to hlist
 
- .clang-format                       |   1 +
- arch/m68k/emu/nfcon.c               |  10 +-
- arch/um/kernel/kmsg_dump.c          |  24 +-
- drivers/firmware/efi/earlycon.c     |   8 +-
- drivers/net/netconsole.c            |  21 +-
- drivers/tty/hvc/hvc_console.c       |   4 +-
- drivers/tty/serial/8250/8250_core.c |   2 +-
- drivers/tty/serial/earlycon.c       |   4 +-
- drivers/tty/serial/kgdboc.c         |  46 ++-
- drivers/tty/serial/pic32_uart.c     |   4 +-
- drivers/tty/serial/samsung_tty.c    |   2 +-
- drivers/tty/serial/serial_core.c    |  14 +-
- drivers/tty/serial/sh-sci.c         |  17 +-
- drivers/tty/serial/xilinx_uartps.c  |   2 +-
- drivers/tty/tty_io.c                |  18 +-
- drivers/usb/early/xhci-dbc.c        |   2 +-
- drivers/video/fbdev/xen-fbfront.c   |  12 +-
- fs/proc/consoles.c                  |  21 +-
- include/linux/console.h             | 111 +++++++-
- include/linux/rcupdate.h            |   5 +
- include/linux/serial_core.h         |  15 +-
- kernel/debug/kdb/kdb_io.c           |  14 +-
- kernel/printk/printk.c              | 424 +++++++++++++++++++++-------
- 23 files changed, 605 insertions(+), 176 deletions(-)
+ .clang-format                      |   1 +
+ arch/m68k/emu/nfcon.c              |   4 +-
+ arch/um/kernel/kmsg_dump.c         |  15 +-
+ drivers/firmware/efi/earlycon.c    |   4 +-
+ drivers/net/netconsole.c           |   4 +-
+ drivers/tty/hvc/hvc_console.c      |   2 +-
+ drivers/tty/serial/earlycon.c      |   4 +-
+ drivers/tty/serial/kgdboc.c        |  37 ++-
+ drivers/tty/serial/pic32_uart.c    |   2 +-
+ drivers/tty/serial/samsung_tty.c   |   2 +-
+ drivers/tty/serial/serial_core.c   |   2 +-
+ drivers/tty/serial/xilinx_uartps.c |   2 +-
+ drivers/tty/tty_io.c               |  18 +-
+ drivers/usb/early/xhci-dbc.c       |   2 +-
+ drivers/video/fbdev/xen-fbfront.c  |  16 +-
+ fs/proc/consoles.c                 |  20 +-
+ include/linux/console.h            |  75 +++++-
+ include/linux/serial_core.h        |   2 +-
+ kernel/debug/kdb/kdb_io.c          |   7 +-
+ kernel/printk/printk.c             | 373 +++++++++++++++++++++--------
+ 20 files changed, 438 insertions(+), 154 deletions(-)
 
 
-base-commit: e29a4915db1480f96e0bc2e928699d086a71f43c
+base-commit: c2d158a284abd63d727dad7402a2eed650dd4233
 -- 
 2.30.2
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help