Re: [PATCH v2 3/9] printk: Separate code for adding/updating preferred console metadata

Re: [PATCH v2 3/9] printk: Separate code for adding/updating preferred console metadata

[Marcos Paulo de Souza]

On Thu, 2026-04-23 at 15:00 +0200, Petr Mladek wrote:
> The logic for adding or updating a preferred console is currently
> duplicated within __add_preferred_console(), making the code
> difficult
> to follow and prone to consistency issues.
> 
> Introduce update_preferred_console() to centralize the initialization
> and updating of struct preferred_console entries. This refactoring
> explicitly defines and enforces the following rules:
> 
> 1. Console names and/or indexes are not set when a console is
> preferred
>    via devname; these are resolved later during device matching.
> 2. Console names are only added alongside a valid index.
> 3. Only matching entries are updated.
> 4. Console and Braille options are never cleared. They are updated
>    only via the command line.
> 5. The global 'preferred_dev_console' index and
> 'console_set_on_cmdline'
>    flag are updated consistently.
> 
> Additionally, rename braille_set_options() to
> braille_update_options()
> to better reflect its conditional behavior.
> 
> Behavior change:
> 
> The original code never updated the preferred console options
> when it was preferred more times, e.g. via the command line
> and/or some platform specific code, e.g. SPCR or device tree.
> 
> The new code explicitly allows to update the console options
> when they are preferred over the command line.
> 
> It mostly worked even before but only because the command line was
> processed early enough before handling SPCR, device tree, or other
> platform specific init code.
> 
> The main behavior change is when the same console is preferred more
> times on the command line. Newly, the later or Braille variant
> wins. It is a more common and expected behavior.

Sorry for the late review. I just wanted to say that the improvement is
really great: documentation and code reorganization really improve the
code. It was too confusing and rather problematic to follow up.

Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>

> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/printk/braille.h |   7 +-
>  kernel/printk/printk.c  | 149 ++++++++++++++++++++++++++++++--------
> --
>  2 files changed, 118 insertions(+), 38 deletions(-)
> 
> diff --git a/kernel/printk/braille.h b/kernel/printk/braille.h
> index 55cd3178a17a..0bdac303f8b1 100644
> --- a/kernel/printk/braille.h
> +++ b/kernel/printk/braille.h
> @@ -5,9 +5,10 @@
>  #ifdef CONFIG_A11Y_BRAILLE_CONSOLE
>  
>  static inline void
> -braille_set_options(struct preferred_console *pc, char *brl_options)
> +braille_update_options(struct preferred_console *pc, char
> *brl_options)
>  {
> -	pc->brl_options = brl_options;
> +	if (brl_options)
> +		pc->brl_options = brl_options;
>  }
>  
>  /*
> @@ -29,7 +30,7 @@ _braille_unregister_console(struct console
> *console);
>  #else
>  
>  static inline void
> -braille_set_options(struct preferred_console *pc, char *brl_options)
> +braille_update_options(struct preferred_console *pc, char
> *brl_options)
>  {
>  }
>  
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 13c98285892b..d251bf8e104f 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2544,18 +2544,119 @@ asmlinkage __visible void early_printk(const
> char *fmt, ...)
>  }
>  #endif
>  
> -static void set_user_specified(struct preferred_console *pc, bool
> user_specified)
> +/** update_preferred_console - Update a given entry in the
> preferred_consoles[]
> + *	table.
> + * @i: index of the entry in @preferred_consoles table which should
> get updated.
> + * @name: The name of the preferred console driver.
> + * @idx: Preferred console index, e.g. port number.
> + * @devname: The name of the preferred physical device.
> + * @options: Options used when setting up the console driver.
> + * @brl_options: Options used when setting up the console driver
> + *	as a braille console.
> + * @user_specified: True if preferred via the kernel command line.
> + *
> + * The function ensures that the given values are consistent. Also
> + * it updates some global variables which are used to make the right
> + * decisions in register_console().
> + *
> + * Rules:
> + *
> + *   1. Either @name and valid @idx OR @devname and @idx=-1 are
> allowed.
> + *	Note that a valid @name and @idx will get assigned later
> when
> + *	@devname matches during the device initialization.
> + *   2. Specify @brl_options if the console should be enabled as
> + *	a Braille console [*]
> + *   3. Only matching entries can be updated.
> + *   4. @options passed via the command line are used when the same
> + *	console is preferred also by some platform-specific code.
> + *
> + * [*] Braille console is using the mechanism for registering
> consoles
> + *     but it is very special. It is primarily used for user
> interaction
> + *     with the system. It neither gets printk() messages nor is
> associated
> + *     with /dev/console.
> + */
> +static int update_preferred_console(unsigned int i,
> +				    const char *name, const short
> idx,
> +				    const char *devname, char
> *options,
> +				    char *brl_options, bool
> user_specified)
>  {
> -	if (!user_specified)
> -		return;
> +	struct preferred_console *pc;
> +
> +	if (i >= MAX_PREFERRED_CONSOLES)
> +		return -E2BIG;
> +
> +	pc = &preferred_consoles[i];
> +
> +	if (!name && !devname)
> +		return -EINVAL;
> +
> +	if (devname) {
> +		/*
> +		 * A valid console name and index will get assigned
> when
> +		 * a matching device gets registered.
> +		 */
> +		if (name) {
> +			pr_err("Adding a preferred console devname
> with a hard-coded console name: %s, %s\n",
> +			       devname, name);
> +			return -EINVAL;
> +		}
> +		if (pc->name[0]) {
> +			pr_err("Updating a preferred console entry
> with an already assigned console name via devname: %s, %s\n",
> +			       devname, pc->name);
> +			return -EINVAL;
> +		}
> +		if (idx != -1) {
> +			pr_err("Adding a preferred console devname
> with a hard-coded index: %s, %d\n",
> +			       devname, idx);
> +			return -EINVAL;
> +		}
> +
> +		if (!pc->devname[0]) {
> +			strscpy(pc->devname, devname);
> +			pc->index = idx;
> +		} else if (strcmp(pc->devname, devname) != 0) {
> +			pr_err("Updating a preferred console with an
> invalid devname: %s vs. %s\n",
> +			       pc->devname, devname);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (name) {
> +		/* A console name must be defined with a valid
> index. */
> +		if (idx < 0) {
> +			pr_err("Adding a preferred console with an
> invalid index: %s, %d\n",
> +			       name, idx);
> +			return -EINVAL;
> +		}
> +
> +		if (!pc->name[0]) {
> +			strscpy(pc->name, name);
> +			pc->index = idx;
> +		} else if (strcmp(pc->name, name) != 0 || pc->index
> != idx) {
> +			pr_err("Updating a preferred console with an
> invalid name or index: %s%d vs. %s%d\n",
> +			       pc->name, pc->index, name, idx);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (!pc->options || (user_specified && options))
> +		pc->options = options;
> +
> +	braille_update_options(pc, brl_options);
> +
> +	if (!brl_options)
> +		preferred_dev_console = i;
>  
>  	/*
>  	 * @pc console was defined by the user on the command line.
>  	 * Do not clear when added twice also by SPCR or the device
> tree.
>  	 */
> -	pc->user_specified = true;
> -	/* At least one console defined by the user on the command
> line. */
> -	console_set_on_cmdline = 1;
> +	if (user_specified) {
> +		pc->user_specified = true;
> +		console_set_on_cmdline = 1;
> +	}
> +
> +	return 0;
>  }
>  
>  static int __add_preferred_console(const char *name, const short
> idx,
> @@ -2563,19 +2664,11 @@ static int __add_preferred_console(const char
> *name, const short idx,
>  				   char *brl_options, bool
> user_specified)
>  {
>  	struct preferred_console *pc;
> -	int i;
> +	unsigned int i;
>  
>  	if (!name && !devname)
>  		return -EINVAL;
>  
> -	/*
> -	 * We use a signed short index for struct console for device
> drivers to
> -	 * indicate a not yet assigned index or port. However, a
> negative index
> -	 * value is not valid when the console name and index are
> defined on
> -	 * the command line.
> -	 */
> -	if (name && idx < 0)
> -		return -EINVAL;
>  
>  	/*
>  	 *	See if this tty is not yet registered, and
> @@ -2584,28 +2677,14 @@ static int __add_preferred_console(const char
> *name, const short idx,
>  	for (i = 0, pc = preferred_consoles;
>  	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc-
> >devname[0]);
>  	     i++, pc++) {
> -		if ((name && strcmp(pc->name, name) == 0 && pc-
> >index == idx) ||
> -		    (devname && strcmp(pc->devname, devname) == 0))
> {
> -			if (!brl_options)
> -				preferred_dev_console = i;
> -			set_user_specified(pc, user_specified);
> -			return 0;
> -		}
> +		if (name && strcmp(pc->name, name) == 0 && pc->index
> == idx)
> +			break;
> +		if (devname && strcmp(pc->devname, devname) == 0)
> +			break;
>  	}
> -	if (i == MAX_PREFERRED_CONSOLES)
> -		return -E2BIG;
> -	if (!brl_options)
> -		preferred_dev_console = i;
> -	if (name)
> -		strscpy(pc->name, name);
> -	if (devname)
> -		strscpy(pc->devname, devname);
> -	pc->options = options;
> -	set_user_specified(pc, user_specified);
> -	braille_set_options(pc, brl_options);
>  
> -	pc->index = idx;
> -	return 0;
> +	return update_preferred_console(i, name, idx, devname,
> options,
> +					brl_options,
> user_specified);
>  }
>  
>  static int __init console_msg_format_setup(char *str)

Re: [PATCH v2 5/9] printk: Separate code for enabling console

[Marcos Paulo de Souza]

On Thu, 2026-04-23 at 15:00 +0200, Petr Mladek wrote:
> There are several code paths which try to enable a newly registered
> console. Move the logic into a separate try_enable_console()
> function.
> 
> It simplifies a bit the long register_console() function definition.
> 
> Also followup patches are going to add even more code paths. And it
> will
> be easier to use "return" when it does not make sense to try other
> variants.
> 
> The patch does not change the existing behavior.
> 
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>

> ---
>  kernel/printk/printk.c | 64 ++++++++++++++++++++++++----------------
> --
>  1 file changed, 36 insertions(+), 28 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 7a3bbb0cb794..2543c810efcb 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -4040,6 +4040,41 @@ static void try_enable_default_console(struct
> console *newcon)
>  		newcon->flags |= CON_CONSDEV;
>  }
>  
> +#define console_first()				\
> +	hlist_entry(console_list.first, struct console, node)
> +
> +static int try_enable_console(struct console *newcon)
> +{
> +	int err;
> +
> +	/*
> +	 * 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 (preferred_dev_console < 0) {
> +		if (hlist_empty(&console_list) || !console_first()-
> >device ||
> +		    console_first()->flags & CON_BOOT) {
> +			try_enable_default_console(newcon);
> +		}
> +	}
> +
> +	/* 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) */
> +	return try_enable_preferred_console(newcon, false);
> +}
> +
>  /* Return the starting sequence number for a newly registered
> console. */
>  static u64 get_init_console_seq(struct console *newcon, bool
> bootcon_registered)
>  {
> @@ -4114,9 +4149,6 @@ static u64 get_init_console_seq(struct console
> *newcon, bool bootcon_registered)
>  	return init_seq;
>  }
>  
> -#define console_first()				\
> -	hlist_entry(console_list.first, struct console, node)
> -
>  static int unregister_console_locked(struct console *console);
>  
>  /*
> @@ -4178,31 +4210,7 @@ void register_console(struct console *newcon)
>  			goto unlock;
>  	}
>  
> -	/*
> -	 * 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 (preferred_dev_console < 0) {
> -		if (hlist_empty(&console_list) || !console_first()-
> >device ||
> -		    console_first()->flags & CON_BOOT) {
> -			try_enable_default_console(newcon);
> -		}
> -	}
> -
> -	/* See if this console matches one we selected on the
> command line */
> -	err = try_enable_preferred_console(newcon, true);
> -
> -	/* If not, try to match against the platform default(s) */
> -	if (err == -ENOENT)
> -		err = try_enable_preferred_console(newcon, false);
> +	err = try_enable_console(newcon);
>  
>  	/* printk() messages are not printed to the Braille console.
> */
>  	if (err || newcon->flags & CON_BRL) {

Re: [PATCH v2 1/9] printk: Rename struct console_cmdline to preferred_console

[John Ogness]

On 2026-04-23, Petr Mladek <pmladek@suse.com> wrote:
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 0323149548f6..ca523acbf58d 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2624,8 +2624,10 @@ __setup("console_msg_format=", console_msg_format_setup);
>   */
>  static int __init console_setup(char *str)
>  {
> -	static_assert(sizeof(console_cmdline[0].devname) >= sizeof(console_cmdline[0].name) + 4);
> -	char buf[sizeof(console_cmdline[0].devname)];
> +	static_assert(sizeof(preferred_consoles[0].devname) >=
> +		      sizeof(preferred_consoles[0].name) + 4);
> +
> +	char buf[sizeof(preferred_consoles[0].devname)];

I agree with Steven's suggestion to move the static_assert() below the
local variable definitions.

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Re: [PATCH v2 2/9] printk: Rename preferred_console to preferred_dev_console

[John Ogness]

On 2026-04-23, Petr Mladek <pmladek@suse.com> wrote:
> The preferred_consoles[] array stores information about consoles requested
> via the command line, SPCR, Device Tree, or platform-specific code.
> Within this array, the 'preferred_console' variable tracks the specific
> index that should be associated with /dev/console (typically the last
> non-braille console defined).
>
> The current name "preferred_console" is ambiguous and leads to confusion.
> It does not clearly communicate why one console is "more preferred" than
> others in the array. Furthermore, entries for Braille consoles can exist
> within the preferred_consoles[] array, yet they are never associated with
> /dev/console and do not receive standard printk() output. Consequently,
> the 'preferred_console' index must skip these entries, which is not
> immediately obvious from the name.
>
> Rename the variable to 'preferred_dev_console' to explicitly clarify its
> role in identifying which entry is linked to /dev/console.
>
> The patch should not change the existing behavior.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Acked-by: Chris Down <chris@chrisdown.name>
> Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Re: [PATCH v2 3/9] printk: Separate code for adding/updating preferred console metadata

[John Ogness]

On 2026-04-23, Petr Mladek <pmladek@suse.com> wrote:
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 13c98285892b..d251bf8e104f 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2544,18 +2544,119 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
>  }
>  #endif
>  
> -static void set_user_specified(struct preferred_console *pc, bool user_specified)
> +/** update_preferred_console - Update a given entry in the preferred_consoles[]
> + *	table.

Perhaps the description should be:

Update or add a given entry in the preferred_consoles[] table.

If the caller didn't match an existing preferred_console, this function
is called to add a new entry. This is obvious if you are looking at the
caller, but it is not obvious at all if you are only looking at this
function, i.e. strscpy'ing in @name or @devname is the only clue that
you are dealing with a new entry.

> + * @i: index of the entry in @preferred_consoles table which should get updated.
> + * @name: The name of the preferred console driver.
> + * @idx: Preferred console index, e.g. port number.
> + * @devname: The name of the preferred physical device.
> + * @options: Options used when setting up the console driver.
> + * @brl_options: Options used when setting up the console driver
> + *	as a braille console.
> + * @user_specified: True if preferred via the kernel command line.
> + *
> + * The function ensures that the given values are consistent. Also
> + * it updates some global variables which are used to make the right
> + * decisions in register_console().
> + *
> + * Rules:
> + *
> + *   1. Either @name and valid @idx OR @devname and @idx=-1 are allowed.
> + *	Note that a valid @name and @idx will get assigned later when
> + *	@devname matches during the device initialization.
> + *   2. Specify @brl_options if the console should be enabled as
> + *	a Braille console [*]
> + *   3. Only matching entries can be updated.
> + *   4. @options passed via the command line are used when the same
> + *	console is preferred also by some platform-specific code.
> + *
> + * [*] Braille console is using the mechanism for registering consoles
> + *     but it is very special. It is primarily used for user interaction
> + *     with the system. It neither gets printk() messages nor is associated
> + *     with /dev/console.
> + */
> +static int update_preferred_console(unsigned int i,
> +				    const char *name, const short idx,
> +				    const char *devname, char *options,
> +				    char *brl_options, bool user_specified)
>  {
> -	if (!user_specified)
> -		return;
> +	struct preferred_console *pc;
> +
> +	if (i >= MAX_PREFERRED_CONSOLES)
> +		return -E2BIG;
> +
> +	pc = &preferred_consoles[i];
> +
> +	if (!name && !devname)
> +		return -EINVAL;

You are adding detailed error messages for all possible wrong calls
except for this on. Maybe this should be covered as well with:

	if (WARN_ON(!name && !devname))
		return -EINVAL;

> +	if (devname) {
> +		/*
> +		 * A valid console name and index will get assigned when
> +		 * a matching device gets registered.
> +		 */
> +		if (name) {
> +			pr_err("Adding a preferred console devname with a hard-coded console name: %s, %s\n",
> +			       devname, name);
> +			return -EINVAL;
> +		}
> +		if (pc->name[0]) {
> +			pr_err("Updating a preferred console entry with an already assigned console name via devname: %s, %s\n",
> +			       devname, pc->name);
> +			return -EINVAL;
> +		}
> +		if (idx != -1) {
> +			pr_err("Adding a preferred console devname with a hard-coded index: %s, %d\n",
> +			       devname, idx);
> +			return -EINVAL;
> +		}
> +
> +		if (!pc->devname[0]) {
> +			strscpy(pc->devname, devname);
> +			pc->index = idx;

This block is about setting the devname. Setting the index here as well
is copied code and well hidden. I would keep the index setting outside
the devname and name blocks (as it was previously).

> +		} else if (strcmp(pc->devname, devname) != 0) {
> +			pr_err("Updating a preferred console with an invalid devname: %s vs. %s\n",
> +			       pc->devname, devname);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (name) {
> +		/* A console name must be defined with a valid index. */
> +		if (idx < 0) {
> +			pr_err("Adding a preferred console with an invalid index: %s, %d\n",
> +			       name, idx);
> +			return -EINVAL;
> +		}
> +
> +		if (!pc->name[0]) {
> +			strscpy(pc->name, name);
> +			pc->index = idx;

Here is the copied code. This block is about setting the name.

> +		} else if (strcmp(pc->name, name) != 0 || pc->index != idx) {
> +			pr_err("Updating a preferred console with an invalid name or index: %s%d vs. %s%d\n",
> +			       pc->name, pc->index, name, idx);
> +			return -EINVAL;
> +		}
> +	}

I would put the index setting here as it is relevant regardless of how
the devname or name was updated.

	pc->index = idx;

With or without implementing my suggestions:

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Re: [PATCH v2 3/9] printk: Separate code for adding/updating preferred console metadata

[John Ogness]

On 2026-05-15, John Ogness <john.ogness@linutronix.de> wrote:
>> +		if (!pc->name[0]) {
>> +			strscpy(pc->name, name);
>> +			pc->index = idx;
>
> Here is the copied code. This block is about setting the name.
>
>> +		} else if (strcmp(pc->name, name) != 0 || pc->index != idx) {
>> +			pr_err("Updating a preferred console with an invalid name or index: %s%d vs. %s%d\n",
>> +			       pc->name, pc->index, name, idx);
>> +			return -EINVAL;
>> +		}
>> +	}
>
> I would put the index setting here as it is relevant regardless of how
> the devname or name was updated.
>
> 	pc->index = idx;

Ah, I guess you only wanted to update @index if it is a new entry
(because otherwise it will already have the value of @idx).

If you want to keep the index setting in the devname and name blocks,
maybe just adding a comment to clarify it is a new entry:

	if (!pc->devname[0]) {
		/* This is a new entry */
		strscpy(pc->devname, devname);
		pc->index = idx;

...

	if (!pc->name[0]) {
		/* This is a new entry */
		strscpy(pc->name, name);
		pc->index = idx;

John

Re: [PATCH v2 4/9] printk: Cleanup _braille_(un)register_console() wrappers

[John Ogness]

On 2026-04-23, Sashiko wrote:
>> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
>> index 06b43b678d6e..7b324329882f 100644
>> --- a/drivers/accessibility/braille/braille_console.c
>> +++ b/drivers/accessibility/braille/braille_console.c
>> @@ -360,12 +360,12 @@ int braille_register_console(struct console *console, int index,
>>  		if (ret != 0)
>>  			return ret;
>>  	}
>> -	console->flags |= CON_ENABLED;
>> +	console->flags |= CON_ENABLED | CON_BRL;
>>  	console->index = index;
>>  	braille_co = console;
>>  	register_keyboard_notifier(&keyboard_notifier_block);
>>  	register_vt_notifier(&vt_notifier_block);
>> -	return 1;
>> +	return 0;
>>  }
>
> While reviewing this function, I noticed a pre-existing
> regression. The printk subsystem relies on console_lock() to serialize
> console setup against boot consoles that might be actively printing to
> the same hardware port.  For normal consoles, this synchronization is
> handled explicitly:
>
> kernel/printk/printk.c:console_call_setup() {
>     ...
>     console_lock();
>     err = newcon->setup(newcon, options);
>     console_unlock();
>     ...
> }
>
> However, it looks like braille_register_console() bypasses this
> protection and directly invokes console->setup() just above this block
> without acquiring console_lock().
>
> Could this unprotected setup call cause a data race or hardware state
> corruption if the underlying driver shares the hardware with an active
> boot console?

I do not know enough about braille consoles if they can also be early
consoles, but it makes sense to synchronize the ->setup()
callback. Something like this:

--- a/drivers/accessibility/braille/braille_console.c
+++ b/drivers/accessibility/braille/braille_console.c
@@ -356,7 +356,9 @@ int braille_register_console(struct console *console, int index,
 	if (braille_co)
 		return -ENODEV;
 	if (console->setup) {
+		console_lock();
 		ret = console->setup(console, console_options);
+		console_unlock();
 		if (ret != 0)
 			return ret;
 	}

>>  int braille_unregister_console(struct console *console)
>> @@ -375,5 +375,6 @@ int braille_unregister_console(struct console *console)
>>  	unregister_keyboard_notifier(&keyboard_notifier_block);
>>  	unregister_vt_notifier(&vt_notifier_block);
>>  	braille_co = NULL;
>> -	return 1;
>> +	console->flags &= ~(CON_ENABLED | CON_BRL);
>> +	return 0;
>>  }
>
> This also appears to be a pre-existing regression. The
> unregister_keyboard_notifier() and unregister_vt_notifier() functions
> remove callbacks from atomic notifier chains. Since atomic notifiers
> execute their callbacks under RCU read lock, unregistering them only
> unlinks the node without blocking for concurrent callbacks to finish.
>
> By immediately clearing the global braille_co pointer and returning,
> could a concurrently executing keyboard_notifier_call that already
> passed the braille_co check on another CPU cause a use-after-free or
> null pointer dereference?
>
> Does this function need a synchronize_rcu() before returning to ensure
> all callbacks have completed?

atomic_notifier_chain_unregister() already calls synchronize_rcu()
before returning, so no action is needed here.

For this patch as it is:

Reviewed-by: John Ogness <john.ogness@linutronix.de>

(The console lock/unlock should be a separate patch.)

John

Re: [PATCH v2 5/9] printk: Separate code for enabling console

[John Ogness]

On 2026-04-23, Petr Mladek <pmladek@suse.com> wrote:
> There are several code paths which try to enable a newly registered
> console. Move the logic into a separate try_enable_console() function.
>
> It simplifies a bit the long register_console() function definition.
>
> Also followup patches are going to add even more code paths. And it will
> be easier to use "return" when it does not make sense to try other
> variants.
>
> The patch does not change the existing behavior.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: John Ogness <john.ogness@linutronix.de>