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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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