* [PATCH printk v3 00/40] reduce console_lock scope
@ 2022-11-07 14:15 John Ogness
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: John Ogness @ 2022-11-07 14:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay,
Josh Triplett, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes,
rcu, Jason Wessel, Daniel Thompson, Douglas Anderson,
Greg Kroah-Hartman, Jiri Slaby, kgdb-bugreport, linux-serial,
linux-fsdevel, Miguel Ojeda, Richard Weinberger, Anton Ivanov,
Johannes Berg, linux-um, Luis Chamberlain, Aaron Tomlin,
Andy Shevchenko, Ilpo Järvinen, Geert Uytterhoeven,
Tony Lindgren, Lukas Wunner, Geert Uytterhoeven, linux-m68k,
Ard Biesheuvel, linux-efi, linuxppc-dev, Krzysztof Kozlowski,
Alim Akhtar, linux-arm-kernel, linux-samsung-soc, Michal Simek,
Peter Zijlstra, Mathias Nyman, linux-usb, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, Helge Deller,
Javier Martinez Canillas, Thomas Zimmermann, Juergen Gross,
Boris Ostrovsky, Tom Rix, linux-fbdev, dri-devel
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
(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.
The base commit for this series is from Paul McKenney's RCU tree
and provides an NMI-safe SRCU implementation [1]. Without the
NMI-safe SRCU implementation, this series is not less safe than
mainline. But we will need the NMI-safe SRCU implementation for
atomic consoles anyway, so we might as well get it in
now. Especially since it _does_ increase the reliability for
mainline in the panic path.
Changes since v3:
general:
- introduce a synchronized console_is_registered() to query if
a console is registered, meant to replace CON_ENABLED
(mis)use for this purpose
- directly read console->flags for registered consoles if it is
race-free (and document that it is so)
- replace uart_console_enabled() with a new
uart_console_registered() based on console_is_registered()
- change comments about why console_lock is used to synchronize
console->device() by providing an example
registration check fixups:
- 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
kgdb/kdb:
- in configure_kgdboc(), take console_list_lock to synchronize
tty_find_polling_driver() against register_console()
- add comments explaining why calling console->write() without
locking might work
tty: sh-sci:
- use a setup() callback to setup the early console
fbdev: xen:
- implement a cleaner approach for
console_force_preferred_locked()
rcu:
- implement debug_lockdep_rcu_enabled() for
!CONFIG_DEBUG_LOCK_ALLOC
printk:
- check CONFIG_DEBUG_LOCK_ALLOC for srcu_read_lock_held()
availability
- for console_lock/_trylock/_unlock, replace "lock the console
system" language with "block the console subsystem from
printing"
- use WRITE_ONCE() for updating console->flags of registered
consoles
- expand comments of synchronize_srcu() calls to explain why
they are needed, and also expand comments to explain when it
is not needed
- change CON_BOOT consoles to always begin at earliest message
- for non-BOOT/non-PRINTBUFFER consoles, initialize @seq to the
minimal @seq of any of the enabled boot consoles
- add comments and lockdep assertion to
unregister_console_locked() because it is not clear from the
name which lock is implied
- dropped patches that caused unnecessary churn in the series
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
John Ogness (38):
rcu: implement lockdep_rcu_enabled for !CONFIG_DEBUG_LOCK_ALLOC
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: use console_is_enabled()
um: kmsg_dump: 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
tty: tty_io: document console_lock usage
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: __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
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
Thomas Gleixner (2):
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(-)
base-commit: e29a4915db1480f96e0bc2e928699d086a71f43c
--
2.30.2
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
@ 2022-11-07 14:16 ` John Ogness
2022-11-08 12:41 ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled() John Ogness
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: John Ogness @ 2022-11-07 14:16 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
The initial intention of the UML kmsg_dumper is to dump the kernel
buffers to stdout if there is no console available to perform the
regular crash output.
However, if ttynull was registered as a console, no crash output was
seen. Commit e23fe90dec28 ("um: kmsg_dumper: always dump when not tty
console") tried to fix this by performing the kmsg_dump unless the
stdio console was behind /dev/console or enabled. But this allowed
kmsg dumping to occur even if other non-stdio consoles will output
the crash output. Also, a console being the driver behind
/dev/console has nothing to do with a crash scenario.
Restore the initial intention by dumping the kernel buffers to stdout
only if a non-ttynull console is registered and enabled. Also add
detailed comments so that it is clear why these rules are applied.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/um/kernel/kmsg_dump.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index 0224fcb36e22..40abf1e9ccb1 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -17,13 +17,22 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
unsigned long flags;
size_t len = 0;
- /* only dump kmsg when no console is available */
+ /*
+ * If no consoles are available to output crash information, dump
+ * the kmsg buffer to stdout.
+ */
+
if (!console_trylock())
return;
for_each_console(con) {
- if(strcmp(con->name, "tty") == 0 &&
- (con->flags & (CON_ENABLED | CON_CONSDEV)) != 0) {
+ /*
+ * The ttynull console and disabled consoles are ignored
+ * since they cannot output. All other consoles are
+ * expected to output the crash information.
+ */
+ if (strcmp(con->name, "ttynull") != 0 &&
+ (con->flags & CON_ENABLED)) {
break;
}
}
--
2.30.2
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
@ 2022-11-07 14:16 ` John Ogness
2022-11-09 14:52 ` Petr Mladek
2022-11-09 14:56 ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator John Ogness
2022-11-11 14:43 ` [PATCH printk v3 00/40] reduce console_lock scope Mathieu Desnoyers
3 siblings, 2 replies; 10+ messages in thread
From: John Ogness @ 2022-11-07 14:16 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/um/kernel/kmsg_dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index 40abf1e9ccb1..f0233e2f8de0 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
* expected to output the crash information.
*/
if (strcmp(con->name, "ttynull") != 0 &&
- (con->flags & CON_ENABLED)) {
+ console_is_enabled(con)) {
break;
}
}
--
2.30.2
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled() John Ogness
@ 2022-11-07 14:16 ` John Ogness
2022-11-09 15:05 ` Petr Mladek
2022-11-11 14:43 ` [PATCH printk v3 00/40] reduce console_lock scope Mathieu Desnoyers
3 siblings, 1 reply; 10+ messages in thread
From: John Ogness @ 2022-11-07 14:16 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
Rather than using the console_lock to guarantee safe console list
traversal, use srcu console list iteration.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/um/kernel/kmsg_dump.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index f0233e2f8de0..b6acb3837f1d 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -16,16 +16,15 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
struct console *con;
unsigned long flags;
size_t len = 0;
+ int cookie;
/*
* If no consoles are available to output crash information, dump
* the kmsg buffer to stdout.
*/
- if (!console_trylock())
- return;
-
- for_each_console(con) {
+ cookie = console_srcu_read_lock();
+ for_each_console_srcu(con) {
/*
* The ttynull console and disabled consoles are ignored
* since they cannot output. All other consoles are
@@ -36,9 +35,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
break;
}
}
-
- console_unlock();
-
+ console_srcu_read_unlock(cookie);
if (con)
return;
--
2.30.2
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
@ 2022-11-08 12:41 ` Petr Mladek
0 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2022-11-08 12:41 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
On Mon 2022-11-07 15:22:04, John Ogness wrote:
> The initial intention of the UML kmsg_dumper is to dump the kernel
> buffers to stdout if there is no console available to perform the
> regular crash output.
>
> However, if ttynull was registered as a console, no crash output was
> seen. Commit e23fe90dec28 ("um: kmsg_dumper: always dump when not tty
> console") tried to fix this by performing the kmsg_dump unless the
> stdio console was behind /dev/console or enabled. But this allowed
> kmsg dumping to occur even if other non-stdio consoles will output
> the crash output. Also, a console being the driver behind
> /dev/console has nothing to do with a crash scenario.
>
> Restore the initial intention by dumping the kernel buffers to stdout
> only if a non-ttynull console is registered and enabled. Also add
> detailed comments so that it is clear why these rules are applied.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
The change makes sense to me:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled() John Ogness
@ 2022-11-09 14:52 ` Petr Mladek
2022-11-09 14:56 ` Petr Mladek
1 sibling, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2022-11-09 14:52 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
On Mon 2022-11-07 15:22:07, John Ogness wrote:
> Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> arch/um/kernel/kmsg_dump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> index 40abf1e9ccb1..f0233e2f8de0 100644
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> * expected to output the crash information.
> */
> if (strcmp(con->name, "ttynull") != 0 &&
> - (con->flags & CON_ENABLED)) {
> + console_is_enabled(con)) {
The patch does not explain why the racy check is needed here.
I would prefer to merge this patch with the patch switching to the srcu
console list iterator. It will become more or less self-explaining.
Anyway, I agree that the racy check is acceptable here.
Best Regards,
Petr
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled() John Ogness
2022-11-09 14:52 ` Petr Mladek
@ 2022-11-09 14:56 ` Petr Mladek
2022-11-09 14:57 ` Petr Mladek
1 sibling, 1 reply; 10+ messages in thread
From: Petr Mladek @ 2022-11-09 14:56 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
On Mon 2022-11-07 15:22:07, John Ogness wrote:
> Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> arch/um/kernel/kmsg_dump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> index 40abf1e9ccb1..f0233e2f8de0 100644
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> * expected to output the crash information.
> */
> if (strcmp(con->name, "ttynull") != 0 &&
> - (con->flags & CON_ENABLED)) {
> + console_is_enabled(con)) {
Same as with the 9th patch. I would merge this with the patch
switching to the srcu console list iterator. It will explain
why the racy check is needed here. This change does not make
sense without the other.
Best Regards,
Petr
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()
2022-11-09 14:56 ` Petr Mladek
@ 2022-11-09 14:57 ` Petr Mladek
0 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2022-11-09 14:57 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
On Wed 2022-11-09 15:56:11, Petr Mladek wrote:
> On Mon 2022-11-07 15:22:07, John Ogness wrote:
> > Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
> >
> > Signed-off-by: John Ogness <john.ogness@linutronix.de>
> > ---
> > arch/um/kernel/kmsg_dump.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> > index 40abf1e9ccb1..f0233e2f8de0 100644
> > --- a/arch/um/kernel/kmsg_dump.c
> > +++ b/arch/um/kernel/kmsg_dump.c
> > @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> > * expected to output the crash information.
> > */
> > if (strcmp(con->name, "ttynull") != 0 &&
> > - (con->flags & CON_ENABLED)) {
> > + console_is_enabled(con)) {
>
> Same as with the 9th patch. I would merge this with the patch
> switching to the srcu console list iterator. It will explain
> why the racy check is needed here. This change does not make
> sense without the other.
Ah, this was supposed to be for the 10th patch.
I am sorry for confusion.
Best Regards,
Petr
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator
2022-11-07 14:16 ` [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator John Ogness
@ 2022-11-09 15:05 ` Petr Mladek
0 siblings, 0 replies; 10+ messages in thread
From: Petr Mladek @ 2022-11-09 15:05 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um
On Mon 2022-11-07 15:22:09, John Ogness wrote:
> Rather than using the console_lock to guarantee safe console list
> traversal, use srcu console list iteration.
It might be obvious. But I would add:
"It allows to dump the messages even when console_lock() is blocked
during panic()."
It will help to understand this is a clear win.
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
After merging with 9th patch for using the racy console_is_enabled(con):
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH printk v3 00/40] reduce console_lock scope
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
` (2 preceding siblings ...)
2022-11-07 14:16 ` [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator John Ogness
@ 2022-11-11 14:43 ` Mathieu Desnoyers
3 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2022-11-11 14:43 UTC (permalink / raw)
To: John Ogness, Petr Mladek, Paul E. McKenney, Frederic Weisbecker
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Neeraj Upadhyay, Josh Triplett, Lai Jiangshan, Joel Fernandes,
rcu, Jason Wessel, Daniel Thompson, Douglas Anderson,
Greg Kroah-Hartman, Jiri Slaby, kgdb-bugreport, linux-serial,
linux-fsdevel, Miguel Ojeda, Richard Weinberger, Anton Ivanov,
Johannes Berg, linux-um, Luis Chamberlain, Aaron Tomlin,
Andy Shevchenko, Ilpo Järvinen, Geert Uytterhoeven,
Tony Lindgren, Lukas Wunner, Geert Uytterhoeven, linux-m68k,
Ard Biesheuvel, linux-efi, linuxppc-dev, Krzysztof Kozlowski,
Alim Akhtar, linux-arm-kernel, linux-samsung-soc, Michal Simek,
Peter Zijlstra, Mathias Nyman, linux-usb, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, Helge Deller,
Javier Martinez Canillas, Thomas Zimmermann, Juergen Gross,
Boris Ostrovsky, Tom Rix, linux-fbdev, dri-devel
On 2022-11-07 09:15, John Ogness wrote:
[...]
>
> The base commit for this series is from Paul McKenney's RCU tree
> and provides an NMI-safe SRCU implementation [1]. Without the
> NMI-safe SRCU implementation, this series is not less safe than
> mainline. But we will need the NMI-safe SRCU implementation for
> atomic consoles anyway, so we might as well get it in
> now. Especially since it _does_ increase the reliability for
> mainline in the panic path.
So, your email got me to review the SRCU nmi-safe series:
[1] https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/log/?h=srcunmisafe.2022.10.21a
Especially this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?h=srcunmisafe.2022.10.21a&id=5d0f5953b60f5f7a278085b55ddc73e2932f4c33
I disagree with the overall approach taken there, which is to create
yet another SRCU flavor, this time with explicit "nmi-safe" read-locks.
This adds complexity to the kernel APIs and I think we can be clever
about this and make SRCU nmi-safe without requiring a whole new incompatible
API.
You can find the basic idea needed to achieve this in the libside RCU
user-space implementation. I needed to introduce a split-counter concept
to support rseq vs atomics to keep track of per-cpu grace period counters.
The "rseq" counter is the fast-path, but if rseq fails, the abort handler
uses the atomic counter instead.
https://github.com/compudj/side/blob/main/src/rcu.h#L23
struct side_rcu_percpu_count {
uintptr_t begin;
uintptr_t rseq_begin;
uintptr_t end;
uintptr_t rseq_end;
} __attribute__((__aligned__(SIDE_CACHE_LINE_SIZE)));
The idea is to "split" each percpu counter into two counters, one for rseq,
and the other for atomics. When a grace period wants to observe the value of
a percpu counter, it simply sums the two counters:
https://github.com/compudj/side/blob/main/src/rcu.c#L112
The same idea can be applied to SRCU in the kernel: one counter for percpu ops,
and the other counter for nmi context, so basically:
srcu_read_lock()
if (likely(!in_nmi()))
increment the percpu-ops lock counter
else
increment the atomic lock counter
srcu_read_unlock()
if (likely(!in_nmi()))
increment the percpu-ops unlock counter
else
increment the atomic unlock counter
Then in the grace period sum the percpu-ops and the atomic values whenever
each counter value is read.
This would allow SRCU to be NMI-safe without requiring the callers to
explicitly state whether they need to be nmi-safe or not, and would only
take the overhead of the atomics in the NMI handlers rather than for all
users which happen to use SRCU read locks shared with nmi handlers.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-11-11 14:43 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
2022-11-08 12:41 ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled() John Ogness
2022-11-09 14:52 ` Petr Mladek
2022-11-09 14:56 ` Petr Mladek
2022-11-09 14:57 ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator John Ogness
2022-11-09 15:05 ` Petr Mladek
2022-11-11 14:43 ` [PATCH printk v3 00/40] reduce console_lock scope Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).