From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753345AbdFMMyp (ORCPT ); Tue, 13 Jun 2017 08:54:45 -0400 Received: from mx2.suse.de ([195.135.220.15]:53700 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753103AbdFMMyn (ORCPT ); Tue, 13 Jun 2017 08:54:43 -0400 From: Petr Mladek To: Sergey Senozhatsky , Steven Rostedt Cc: Andrew Morton , Peter Zijlstra , Aleksey Makarov , Sabrina Dubroca , Sudeep Holla , linux-kernel@vger.kernel.org, Sergey Senozhatsky , Petr Mladek Subject: [PATCH 2/3] printk/console: Clean up logic around fallback console Date: Tue, 13 Jun 2017 14:54:03 +0200 Message-Id: <1497358444-30736-3-git-send-email-pmladek@suse.com> X-Mailer: git-send-email 1.8.5.6 In-Reply-To: <1497358444-30736-1-git-send-email-pmladek@suse.com> References: <1497358444-30736-1-git-send-email-pmladek@suse.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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