* [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred
2022-11-16 16:21 [PATCH printk v5 00/40] reduce console_lock scope John Ogness
@ 2022-11-16 16:21 ` John Ogness
2022-11-18 11:22 ` [PATCH printk v5 00/40] reduce console_lock scope Petr Mladek
2022-11-22 16:43 ` Greg Kroah-Hartman
2 siblings, 0 replies; 5+ messages in thread
From: John Ogness @ 2022-11-16 16:21 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
Helge Deller, Greg Kroah-Hartman, Thomas Zimmermann,
Javier Martinez Canillas, Juergen Gross, Boris Ostrovsky, Tom Rix,
linux-fbdev, dri-devel
With commit 9e124fe16ff2("xen: Enable console tty by default in domU
if it's not a dummy") a hack was implemented to make sure that the
tty console remains the console behind the /dev/console device. The
main problem with the hack is that, after getting the console pointer
to the tty console, it is assumed the pointer is still valid after
releasing the console_sem. This assumption is incorrect and unsafe.
Make the hack safe by introducing a new function
console_force_preferred_locked() and perform the full operation
under the console_list_lock.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
drivers/video/fbdev/xen-fbfront.c | 12 +++-----
include/linux/console.h | 1 +
kernel/printk/printk.c | 49 +++++++++++++++++++++++++++++--
3 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 4d2694d904aa..8752d389e382 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -504,18 +504,14 @@ static void xenfb_make_preferred_console(void)
if (console_set_on_cmdline)
return;
- console_lock();
+ console_list_lock();
for_each_console(c) {
if (!strcmp(c->name, "tty") && c->index == 0)
break;
}
- console_unlock();
- if (c) {
- unregister_console(c);
- c->flags |= CON_CONSDEV;
- c->flags &= ~CON_PRINTBUFFER; /* don't print again */
- register_console(c);
- }
+ if (c)
+ console_force_preferred_locked(c);
+ console_list_unlock();
}
static int xenfb_resume(struct xenbus_device *dev)
diff --git a/include/linux/console.h b/include/linux/console.h
index f716e1dd9eaf..9cea254b34b8 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -291,6 +291,7 @@ enum con_flush_mode {
};
extern int add_preferred_console(char *name, int idx, char *options);
+extern void console_force_preferred_locked(struct console *con);
extern void register_console(struct console *);
extern int unregister_console(struct console *);
extern void console_lock(void);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 410d3f2cdeb3..ece34abbc9cc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -247,9 +247,10 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
void console_list_lock(void)
{
/*
- * In unregister_console(), synchronize_srcu() is called with the
- * console_list_lock held. Therefore it is not allowed that the
- * console_list_lock is taken with the srcu_lock held.
+ * In unregister_console() and console_force_preferred_locked(),
+ * synchronize_srcu() is called with the console_list_lock held.
+ * Therefore it is not allowed that the console_list_lock is taken
+ * with the srcu_lock held.
*
* Detecting if this context is really in the read-side critical
* section is only possible if the appropriate debug options are
@@ -3489,6 +3490,48 @@ int unregister_console(struct console *console)
}
EXPORT_SYMBOL(unregister_console);
+/**
+ * console_force_preferred_locked - force a registered console preferred
+ * @con: The registered console to force preferred.
+ *
+ * Must be called under console_list_lock().
+ */
+void console_force_preferred_locked(struct console *con)
+{
+ struct console *cur_pref_con;
+
+ if (!console_is_registered_locked(con))
+ return;
+
+ cur_pref_con = console_first();
+
+ /* Already preferred? */
+ if (cur_pref_con == con)
+ return;
+
+ /*
+ * Delete, but do not re-initialize the entry. This allows the console
+ * to continue to appear registered (via any hlist_unhashed_lockless()
+ * checks), even though it was briefly removed from the console list.
+ */
+ hlist_del_rcu(&con->node);
+
+ /*
+ * Ensure that all SRCU list walks have completed so that the console
+ * can be added to the beginning of the console list and its forward
+ * list pointer can be re-initialized.
+ */
+ synchronize_srcu(&console_srcu);
+
+ con->flags |= CON_CONSDEV;
+ WARN_ON(!con->device);
+
+ /* Only the new head can have CON_CONSDEV set. */
+ console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV);
+ hlist_add_head_rcu(&con->node, &console_list);
+}
+EXPORT_SYMBOL(console_force_preferred_locked);
+
/*
* Initialize the console device. This is called *early*, so
* we can't necessarily depend on lots of kernel help here.
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH printk v5 00/40] reduce console_lock scope
2022-11-16 16:21 [PATCH printk v5 00/40] reduce console_lock scope John Ogness
2022-11-16 16:21 ` [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred John Ogness
@ 2022-11-18 11:22 ` Petr Mladek
2022-11-18 14:55 ` Petr Mladek
2022-11-22 16:43 ` Greg Kroah-Hartman
2 siblings, 1 reply; 5+ messages in thread
From: Petr Mladek @ 2022-11-18 11:22 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
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, Aaron Tomlin, Luis Chamberlain,
Andy Shevchenko, Ilpo Järvinen, Lukas Wunner,
Geert Uytterhoeven, 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,
Thomas Zimmermann, Javier Martinez Canillas, Juergen Gross,
Boris Ostrovsky, Tom Rix, linux-fbdev, dri-devel
On Wed 2022-11-16 17:27:12, John Ogness wrote:
> This is v5 of a series to prepare for threaded/atomic
> printing. v4 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.
The patchset looks ready for linux-next from my POV.
I am going to push it there right now to get as much testing
as possible before the merge window.
Any review and comments are still appreciate. We could always
take it back if some critical problems are discovered and
can't be solved easily.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH printk v5 00/40] reduce console_lock scope
2022-11-18 11:22 ` [PATCH printk v5 00/40] reduce console_lock scope Petr Mladek
@ 2022-11-18 14:55 ` Petr Mladek
0 siblings, 0 replies; 5+ messages in thread
From: Petr Mladek @ 2022-11-18 14:55 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner, linux-kernel,
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, Aaron Tomlin, Luis Chamberlain,
Andy Shevchenko, Ilpo Järvinen, Lukas Wunner,
Geert Uytterhoeven, 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,
Thomas Zimmermann, Javier Martinez Canillas, Juergen Gross,
Boris Ostrovsky, Tom Rix, linux-fbdev, dri-devel
On Fri 2022-11-18 12:22:58, Petr Mladek wrote:
> On Wed 2022-11-16 17:27:12, John Ogness wrote:
> > This is v5 of a series to prepare for threaded/atomic
> > printing. v4 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.
>
> The patchset looks ready for linux-next from my POV.
>
> I am going to push it there right now to get as much testing
> as possible before the merge window.
JFYI, the patchset is committed in printk/linux.git,
branch rework/console-list-lock.
I'll eventually merge it into rework/kthreads. But I wanted to have
it separated until it gets some more testing in linux-next and
eventually some more review.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH printk v5 00/40] reduce console_lock scope
2022-11-16 16:21 [PATCH printk v5 00/40] reduce console_lock scope John Ogness
2022-11-16 16:21 ` [PATCH printk v5 32/40] printk, xen: fbfront: create/use safe function for forcing preferred John Ogness
2022-11-18 11:22 ` [PATCH printk v5 00/40] reduce console_lock scope Petr Mladek
@ 2022-11-22 16:43 ` Greg Kroah-Hartman
2 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-22 16:43 UTC (permalink / raw)
To: John Ogness
Cc: Petr Mladek, Sergey Senozhatsky, Steven Rostedt, Thomas Gleixner,
linux-kernel, Jason Wessel, Daniel Thompson, Douglas Anderson,
Jiri Slaby, kgdb-bugreport, linux-serial, linux-fsdevel,
Miguel Ojeda, Richard Weinberger, Anton Ivanov, Johannes Berg,
linux-um, Aaron Tomlin, Luis Chamberlain, Andy Shevchenko,
Ilpo Järvinen, Lukas Wunner, Geert Uytterhoeven,
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, Thomas Zimmermann,
Javier Martinez Canillas, Juergen Gross, Boris Ostrovsky, Tom Rix,
linux-fbdev, dri-devel
On Wed, Nov 16, 2022 at 05:27:12PM +0106, John Ogness wrote:
> This is v5 of a series to prepare for threaded/atomic
> printing. v4 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.
>
> 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 (for other reasons), comments are added to explain
> exactly why the console_lock is 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 v4:
>
> printk:
>
> - Introduce console_init_seq() to handle the now rather complex
> procedure to find an appropriate start sequence number for a
> new console upon registration.
>
> - When registering a non-boot console and boot consoles are
> registered, try to flush all the consoles to get the next @seq
> value before falling back to use the @seq of the enabled boot
> console that is furthest behind.
>
> - For console_force_preferred_locked(), make the console the
> head of the console list.
>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 5+ messages in thread