Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   John Ogness <john.ogness@linutronix.de>
To:     Petr Mladek <pmladek@suse.com>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk v3 39/40] printk: relieve console_lock of list
 synchronization duties
In-Reply-To: <20221107141638.3790965-40-john.ogness@linutronix.de>
References: <20221107141638.3790965-1-john.ogness@linutronix.de>
 <20221107141638.3790965-40-john.ogness@linutronix.de>
Date:   Mon, 07 Nov 2022 17:36:48 +0106
Message-ID: <87r0ye916v.fsf@jogness.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On 2022-11-07, John Ogness <john.ogness@linutronix.de> wrote:
> @@ -3344,7 +3340,6 @@ void register_console(struct console *newcon)
>  	 * Put this console in the list - keep the
>  	 * preferred driver at the head of the list.
>  	 */
> -	console_lock();
>  	if (hlist_empty(&console_list)) {
>  		/* Ensure CON_CONSDEV is always set for the head. */
>  		newcon->flags |= CON_CONSDEV;
> @@ -3358,7 +3353,6 @@ void register_console(struct console *newcon)
>  	} else {
>  		hlist_add_behind_rcu(&newcon->node, console_list.first);
>  	}
> -	console_unlock();
>  
>  	/*
>  	 * No need to synchronize SRCU here! The caller does not rely

I just realized that because of the new @seq initialization (patch 5/40)
that we cannot completely remove the console_lock from
register_console(). It will still be needed for @seq synchronization
when registering non-boot/non-printbuffer consoles. So something like
the patch below will need to be folded into this one.

I am not happy with this. If an enabled boot console is behind, the
console_unlock() will probably catch it up and we will end up with some
repeat messages. But maybe this is "good enough" until we implement some
real coordination between boot console and normal console takeovers.

John Ogness

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 17765166ac42..bb119001df56 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3328,12 +3328,21 @@ void register_console(struct console *newcon)
 		 * that message instead. That boot console will be
 		 * unregistered shortly and may be the same device.
 		 */
+
+		/*
+		 * Hold the console_lock to guarantee safe access to
+		 * console->seq.
+		 */
+		console_lock();
+
 		for_each_console(con) {
 			if ((con->flags & (CON_BOOT | CON_ENABLED)) == (CON_BOOT | CON_ENABLED) &&
 			    con->seq < newcon->seq) {
 				newcon->seq = con->seq;
 			}
 		}
+
+		console_unlock();
 	}
 
 	/*