Message-Id: <1497358444-30736-3-git-send-email-pmladek@suse.com>
Date:   Tue, 13 Jun 2017 14:54:03 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Aleksey Makarov <aleksey.makarov@...aro.org>,
        Sabrina Dubroca <sd@...asysnail.net>,
        Sudeep Holla <sudeep.holla@....com>,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Petr Mladek <pmladek@...e.com>
Subject: [PATCH 2/3] printk/console: Clean up logic around fallback console

register_console() has a fallback code that enables the first
usable console when a preferred one is not set. Though the logic
is really tangled by the variable "has_preferred"

The only purpose of the variable is to decide whether we use the fallback
code or not. It is set when the console setup succeeds and the fallback
console is enabled. It makes some sense. We have enabled a console.
We have a fallback. Therefore the fallback code is not longer needed.

But the value is reset in the next register_console() call by the code:

	if (!has_preferred || bcon || !console_drivers)
		has_preferred = preferred_console >= 0

Now, keep your head firmly in hands and try to understand all the twists
here.

"!has_preferred" is true when register_console() is called for the 1st
time or it was false after the previous call. The second possibility
means that "preferred_console" was not set in the previous call.
Therefore the previous call tried to setup a console as a fallback
and it must have failed.

"bcon" is true when a boot console is first in the console_drivers
list. This is a complicated clue in itself. We need to look on how
the list is manipulated. New consoles are put at the beginning of
the list. But the preferred console (with CON_CONSDEV flag) is kept
at the beginning.

Note that we might add a preferred console when there already
is one. Then the new preferred console obsoletes the old one.
One case is when the early and real console have the same
definition and entry in the preferred_console list. Then they
both are preferred. Rather theoretical case is when the first
one is registered using the fallback mechanism before
any preferred_console is set, e.g. by the command line.

OK, let's go back to "bcon". It is "false" when there are
no console drivers. Or it can become "false" when a real console
is put at the beginning of the list. The are basically two
possibilities.

If the first boot console does _not_ have CON_CONSDEV flag set then
any new console will be put at the beginning of the list. By other
words, "bcon" will become "false" once we register the first real
console.

If the first boot console has CON_CONSDEV flag set then "bcon"
will become "false" only when a real console with CON_CONSDEV
flag set is registered. There are only two possibilities.
Either if "preferred_console" is set then we need to wait
until this one is registered. But then "has_preferred" is
true and the fallback code is not used. Or if "preferred_console"
is not set, it is enough to register any real console using
the fallback code that always sets CON_CONSDEV.

Please note that only one real console can be registered using
the fallback code. It will cause "bcon" to become "false"
and "as_preferred" to be "true". This will be important later.

The only exception is when the real console is unregistered
again. There will be no real console in the list. A boot
console will become the first in the list again or console_drivers
will become NULL.

Finally, "!console_drivers" is true if none driver was registered
before or when all drivers were unregistered, e.g. early drivers
by late_initcall(printk_late_init).

Now, if any of the three checks are true, we (re)set "has_preferred"
value. Let's repeat. The variable is set to "true" when any preferred
console is configured. And this information is used to decide whether
we will try to setup this console as a fallback.

If you still have a head close your neck, you might ask what is the exact
purpose of the code. The answer is in the commit 69331af79cf29e26d1231
("Fixes and cleanups for earlyprintk aka boot console"). In short:

  + The check of "bcon" allows to use the fallback code more times
    until we have a real console registered.

  + The "!console_drivers" check is moved from console_unregister().
    It allows to re-enable the fallback mechanism when the last
    console is unregistered.

OK, we know that the current code is not trivial and we know what
we want to achieve. Could we do it an easier way?

This patch gets rid of the "has_preferred" variable. Instead it
uses the following check:

	if (!have_real_console && preferred_console < 0)
		/* try to setup fallback */

Where "have_real_console" is detected using the same code that prevented
registering boot consoles once a real one was in the list. It basically
replaces the check of "!bcon", "console_drivers", and "has_preferred".

IMHO, it is much more straightforward. It perfectly fits the expected
logic that was described in the above mentioned commit and also
in the commit 4d09161196c9a836eac ("printk: Enable the use of more
than one CON_BOOT (early console)").

In short, we try to make debugging easier. Therefore we try to enable
any boot console and one real console if no console is configured
(preferred one is not selected). The fallback code is not used
if a console is configured.

Note that the fallback code might actually be used to register boot
consoles. They might be registered before the "console=" commandline
parameters are handled.

The patch does not change the existing behavior. If I did not miss
anything, the fallback code is called in exactly the same situations
where it was called before.

Signed-off-by: Petr Mladek <pmladek@...e.com>
---
 kernel/printk/printk.c | 44 +++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8ebc480fdbc6..6e651f68bffd 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2413,51 +2413,45 @@ void register_console(struct console *newcon)
 	unsigned long flags;
 	struct console *bcon = NULL;
 	struct console_cmdline *c;
-	static bool has_preferred;
+	bool have_real_console = false;
 
-	if (console_drivers)
-		for_each_console(bcon)
+	if (console_drivers) {
+		for_each_console(bcon) {
 			if (WARN(bcon == newcon,
 					"console '%s%d' already registered\n",
 					bcon->name, bcon->index))
 				return;
 
-	/*
-	 * before we register a new CON_BOOT console, make sure we don't
-	 * already have a valid console
-	 */
-	if (console_drivers && newcon->flags & CON_BOOT) {
-		/* find the last or real console */
-		for_each_console(bcon) {
-			if (!(bcon->flags & CON_BOOT)) {
-				pr_info("Too late to register bootconsole %s%d\n",
-					newcon->name, newcon->index);
-				return;
-			}
+			if (!(bcon->flags & CON_BOOT))
+				have_real_console = true;
 		}
 	}
 
+	if (have_real_console && newcon->flags & CON_BOOT) {
+		pr_info("Too late to register bootconsole %s%d\n",
+			newcon->name, newcon->index);
+		return;
+	}
+
+	/*
+	 * We have not registered the real preferred console if a boot
+	 * console is still the first one.
+	 */
 	if (console_drivers && console_drivers->flags & CON_BOOT)
 		bcon = console_drivers;
 
-	if (!has_preferred || bcon || !console_drivers)
-		has_preferred = preferred_console >= 0;
-
 	/*
-	 *	See if we want to use this console driver. If we
-	 *	didn't select a console we take the first one
-	 *	that registers here.
+	 * Enable any boot console and the first real one when
+	 * consoles are not configured.
 	 */
-	if (!has_preferred) {
+	if (!have_real_console && preferred_console < 0) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
 		    newcon->setup(newcon, NULL) == 0) {
 			newcon->flags |= CON_ENABLED;
-			if (newcon->device) {
+			if (newcon->device)
 				newcon->flags |= CON_CONSDEV;
-				has_preferred = true;
-			}
 		}
 	}
 
-- 
1.8.5.6

Powered by blists - more mailing lists