two locations: was: Re: [PATCH printk v1 03/13] printk: use percpu flag instead of cpu_online()

From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: two locations: was: Re: [PATCH printk v1 03/13] printk: use percpu flag instead of cpu_online()
Date: Wed, 16 Feb 2022 14:58:17 +0100	[thread overview]
Message-ID: <Yg0C+UtoegnybA4q@alley> (raw)
In-Reply-To: <20220207194323.273637-4-john.ogness@linutronix.de>

On Mon 2022-02-07 20:49:13, John Ogness wrote:
> The CON_ANYTIME console flag is used to label consoles that will
> work correctly before percpu resources are allocated. To check
> the condition, cpu_online(raw_smp_processor_id()) was used.
> However, this is odd because CPUs can go offline at a later point.
> Also, the function is forced to use the raw_ variant because
> migration is not disabled.
> 
> Since commit ab6f762f0f53 ("printk: queue wake_up_klogd irq_work
> only if per-CPU areas are ready") there is a variable to identify
> if percpu resources have been allocated. Use that variable instead
> of cpu_online(raw_smp_processor_id()).

> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index d1b773823d63..b346e60e9e51 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2577,11 +2577,11 @@ static int have_callable_console(void)
>   *
>   * Console drivers may assume that per-cpu resources have been allocated. So
>   * unless they're explicitly marked as being able to cope (CON_ANYTIME) don't
> - * call them until this CPU is officially up.
> + * call them until per-cpu resources have been allocated.
>   */
>  static inline int can_use_console(void)
>  {
> -	return cpu_online(raw_smp_processor_id()) || have_callable_console();
> +	return (printk_percpu_data_ready() || have_callable_console());
>  }

cpu_online(raw_smp_processor_id()) check is used also in
call_console_drivers(). The same logic should be used in both
locations.

I found this when reviewing 6th patch that replaced both checks
with a single one.

Note that I am still not sure if this change is correct at all.
It will allow to always call the console during CPU hotplug
and I am not sure if it is safe. IMHO, it might cause problems when
a console driver uses, for example, CPU-bound workqueues.

Best Regards,
Petr