diff mbox series

[printk,v2,31/38] printk: register_console: use srcu console list iterator

Message ID 20221019145600.1282823-32-john.ogness@linutronix.de
State New
Headers
Series reduce console_lock scope |

Commit Message

John Ogness Oct. 19, 2022, 2:55 p.m. UTC 
  Use srcu console list iteration for console list traversal. Now
the traversal at the beginning of register_console() is safe.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Petr Mladek Oct. 26, 2022, 8:20 a.m. UTC | #1 
On Wed 2022-10-19 17:01:53, John Ogness wrote:
> Use srcu console list iteration for console list traversal. Now
> the traversal at the beginning of register_console() is safe.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Reviewed-by: Petr Mladek <pmladek@suse.com>

That said, I think that we could omit this patch. It does not harm.
It prevents a crash when iterating the list but it is only small part
of the races here.

For example, it sets "realcon_enabled" but the list might change before the
value is used. Also the later hlist_empty(&console_list) and
!console_first->device checks are not secure against each other.

The real solution is the introduction of console_list_lock in 33th
patch. That patch removes the rcu read lock. From this POV, this patch
is just unnecessary churn.

Best Regards,
Petr
diff mbox series

Patch

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 410ad9d5649c..809c43e596d8 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3188,20 +3188,23 @@  void register_console(struct console *newcon)
 	struct console *con;
 	bool bootcon_enabled = false;
 	bool realcon_enabled = false;
+	int cookie;
 	int err;
 
-	for_each_console(con) {
+	cookie = console_srcu_read_lock();
+	for_each_console_srcu(con) {
 		if (WARN(con == newcon, "console '%s%d' already registered\n",
-					 con->name, con->index))
+					 con->name, con->index)) {
+			console_srcu_read_unlock(cookie);
 			return;
-	}
+		}
 
-	for_each_console(con) {
 		if (con->flags & CON_BOOT)
 			bootcon_enabled = true;
 		else
 			realcon_enabled = true;
 	}
+	console_srcu_read_unlock(cookie);
 
 	/* Do not register boot consoles when there already is a real one. */
 	if (newcon->flags & CON_BOOT && realcon_enabled) {