Re: [PATCH v11 3/5] x86/earlyprintk: Allocate early log_buf as early as possible

Andy Shevchenko
2015-06-09T16:11:01Z

On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
> This patch moves call of the early setup_log_buf from the
> arch/x86/kernel/setup.c to the arch/x86/kernel/head{32,64}.c
> and updates log_buf with the earlyprintk messages.
>

Didn't mention lockdep_init() change. Regarding to what Borislav told
you how do you handle double call of lockdep_init()?

> We need to do it because without it we will not see earlyprintk
> messages in the kernel log.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> arch/x86/kernel/head32.c | 6 ++++++
> arch/x86/kernel/head64.c | 5 +++++
> arch/x86/kernel/setup.c | 3 ---
> kernel/printk/printk.c | 5 +++++
> 4 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index 92e8b5f..f28d10f 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -32,2 +32,2 @@ static void __init i386_default_early_setup(void)
> asmlinkage __visible void __init i386_start_kernel(void)
> {
> setup_builtin_cmdline();
> +
> + lockdep_init();
> +
> + /* Allocate early log buffer */
> + setup_log_buf(1);
> +
> cr4_init_shadow();
> sanitize_boot_params(&boot_params);
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 1e5f064..53662d2 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -174,0 +174,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>
> setup_builtin_cmdline();
>
> + lockdep_init();
> +
> + /* Allocate early log buffer */
> + setup_log_buf(1);
> +
> /*
> * Load microcode early on BSP.
> */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 0aeee0a..edfdb6a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1146,9 +1146,6 @@ void __init setup_arch(char **cmdline_p)
> if (init_ohci1394_dma_early)
> init_ohci1394_dma_on_all_controllers();
> #endif
> - /* Allocate bigger log buffer */
> - setup_log_buf(1);
> -
> reserve_initrd();
>
> #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index c099b08..d76a032 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1913,6 +1913,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> va_list ap;
> char buf[512];
> int n;
> + printk_func_t vprintk_func;

Rearrange this to be upper a bit, for example before char buf[].

>
> if (!early_console)
> return;
> @@ -1922,7 +1923,10 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> va_end(ap);
>
> early_console->write(early_console, buf, n);
> +
> + /* Store earlyprintk messages in the log_buf */
> + vprintk_func = this_cpu_read(printk_func);
> + vprintk_func(fmt, ap);

Shouldn't be

va_start(ap, fmt);
…
vprintk_func(…);
va_end(ap);

?

Also, when you print the same message to the serial and to kernel
buffer, do you avoid duplication? Your early_printk messages needs flag
LOG_NOCONS if I understand correctly.

> }
> #endif
>

--
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy