Re: [PATCH printk v4 31/39] printk, xen: fbfront: create/use safe function for forcing preferred
From: Petr Mladek <pmladek@suse.com>
Date: 2022-11-15 13:22:22
Also in:
dri-devel, lkml
On Mon 2022-11-14 20:57:18, John Ogness wrote:
Hi, After more detailed runtime testing I discovered that I didn't re-insert the console to the correct place in the list. More below...quoted
--- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c@@ -3461,6 +3462,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_behind_rcu(&con->node, console_list.first);This is adding the console as the 2nd item. It should be the new head. The patch below fixes it. I have done careful runtime testing with this fixup. After the force_preferred, the console is the new head and sending data to /dev/console redirects to that console.
Great catch!
It would be nice if we could fold this in. Sorry.
I have missed it as well :-/
quoted hunk ↗ jump to hunk
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8d635467882f..4b77586cf4cb 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c@@ -3494,7 +3494,7 @@ void console_force_preferred_locked(struct console *con) /* Only the new head can have CON_CONSDEV set. */ console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV); - hlist_add_behind_rcu(&con->node, console_list.first); + hlist_add_head_rcu(&con->node, &console_list); } EXPORT_SYMBOL(console_force_preferred_locked);
With this change: Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr