Re: [PATCH v2 9/9] printk: Try enable preferred consoles only when there are any

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-04-23 15:00:14, Petr Mladek wrote:
> try_enable_preferred_console() used to be always called because it
> had several hidden effects, namely:
> 
> - enabled Braille consoles which were ignored by "preferred_dev_console"
>   because they were not associated with /dev/console.
> 
> - returned success when a console was pre-enabled using CON_ENABLED
>   flag.
> 
> - returned success when a console was enabled by default because
>   try_enable_default_console() did not return success.
> 
> The first two hidden effects were removed in previous patches. Remove
> the last one so that try_enable_preferred_console() can be called only
> when any non-Braille console is preferred.
> 
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -4055,18 +4055,23 @@ static int try_enable_braille_console(struct console *newcon)
>  }
>  
>  /* Try to enable the console unconditionally */
> -static void try_enable_default_console(struct console *newcon)
> +static int try_enable_default_console(struct console *newcon)
>  {
> +	int err;
> +
>  	if (newcon->index < 0)
>  		newcon->index = 0;
>  
> -	if (console_call_setup(newcon, NULL) != 0)
> -		return;
> +	err = console_call_setup(newcon, NULL);
> +	if (err)
> +		return err;
>  
>  	newcon->flags |= CON_ENABLED;
>  
>  	if (newcon->device)
>  		newcon->flags |= CON_CONSDEV;
> +
> +	return 0;
>  }
>  
>  #define console_first()				\
> @@ -4109,7 +4114,9 @@ static int try_enable_console(struct console *newcon)
>  	if (preferred_dev_console < 0) {
>  		if (hlist_empty(&console_list) || !console_first()->device ||
>  		    console_first()->flags & CON_BOOT) {
> -			try_enable_default_console(newcon);
> +			err = try_enable_default_console(newcon);
> +			if (err != -ENOENT)
> +				return err;
>  		}
>  	}

I am working on v3 and I realized that this is not enough.
try_enable_preferred_console() is still called when
there already was default console (the if-condition above failed).
But it makes sense only when preferred_dev_console >= 0.

I am going to go further and put it into "else" part.
I'll actually invert the check and do:

static int try_enable_console(struct console *newcon)
{
	int err;

	/*
	 * First, try to enable the console driver as a Braille console.
	 * It would have metadata in the preferred_consoles[] array.
	 * But it won't be counted as @preferred_console because
	 * it does not get printk() messages and is not associated
	 * with /dev/console.
	 *
	 * Note that it might succeed also when the driver has a match()
	 * callback and it took over a boot console. In this case,
	 * the driver will continue working as a classic non-Braille
	 * console.
	 */
	if (want_braille_console) {
		err = try_enable_braille_console(newcon);
		if (err != -ENOENT)
			return err;
	}

	if (preferred_dev_console >= 0) {
		/* See if this console matches one we selected on the command line */
		err = try_enable_preferred_console(newcon, true);
		if (err != -ENOENT)
			return err;

		/* If not, try to match against the platform default(s) */
		err = try_enable_preferred_console(newcon, false);
		if (err != -ENOENT)
			return err;
	} else {
		/*
		 * See if we want to enable this console driver by default.
		 *
		 * Nope when a console is preferred by the command line, device
		 * tree, or SPCR.
		 *
		 * The first real console with tty binding (driver) wins. More
		 * consoles might get enabled before the right one is found.
		 *
		 * Note that a console with tty binding will have CON_CONSDEV
		 * flag set and will be first in the list.
		 */
		if (hlist_empty(&console_list) || !console_first()->device ||
		    console_first()->flags & CON_BOOT) {
			err = try_enable_default_console(newcon);
			if (err != -ENOENT)
				return err;
		}
	}

	/*
	 * Some consoles, such as pstore and netconsole, can be enabled even
	 * without matching. Accept them at this stage when they had a chance
	 * to match() and call setup().
	 */
	if (newcon->flags & CON_ENABLED)
		err = 0;

	return err;
}

I am going to split this into two patches:

   1. Invert the logic and add the "else" part. This should
      not change the behavior at this stage.

   2. Add the return value for try_enabled_default_console()
      and return immediately on success or error. It might
      actually change the behavior because pre-enabled
      console won't be registered when newcon->setup()
      failed.

Best Regards,
Petr