Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 10 Apr 2019 10:59:26 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To:     Petr Mladek <pmladek@suse.com>
Cc:     Feng Tang <feng.tang@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>,
        Borislav Petkov <bp@suse.de>
Subject: Re: [PATCH 2/2] panic: Enable to print out all printk msg in buffer
Message-ID: <20190410015926.GC26212@jagdpanzerIV>
References: <1554115684-26846-1-git-send-email-feng.tang@intel.com>
 <1554115684-26846-2-git-send-email-feng.tang@intel.com>
 <20190409141430.w2fulp7jnnthojrc@pathway.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190409141430.w2fulp7jnnthojrc@pathway.suse.cz>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On (04/09/19 16:14), Petr Mladek wrote:
> We should:
> 
>    + Flush the latest messages before we replay the log.

Do you mean the pending messages? When we replay the log we also should
print "header line" and panic-cpu backtrace. So we will print panic-cpu
oops twice

	// from panic-cpu flush_on_panic
	[header]
	backtrace
	[end of header]

        // from console_replay
	then all logbuf messages
	and then the same header/backtrace one more time

	[header]
	backtrace]
	[end of backtrace]

Is there any particular reason to flush pending messages before
we play the buffer? Replaying the logbuf can take some time, so
my guess would be that you want to print panic-cpu backtrace as
soon as possible. Is there something else?

[..]
> Then I would update cosole_unlock() with something like:
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 02ca827b8fac..14ef4e2431e7 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2386,21 +2386,32 @@ void console_unlock(void)
>  
>  	for (;;) {
>  		struct printk_log *msg;
> +		int reset_idx = 0;
>  		size_t ext_len = 0;
> -		size_t len;
> +		size_t len = 0;
>  
>  		printk_safe_enter_irqsave(flags);
>  		raw_spin_lock(&logbuf_lock);
> +
>  		if (console_seq < log_first_seq) {
>  			len = sprintf(text,
>  				      "** %llu printk messages dropped **\n",
>  				      log_first_seq - console_seq);
>  
>  			/* messages are gone, move to first one */
> +			reset_idx = 1;
> +		}
> +
> +		if (console_replay) {
> +			len += sprintf(text + len,
> +				       "Replaying the entire log:\n");
> +			reset_idx = 1;
> +			console_replay = 0;
> +		}
> +
> +		if (reset_idx) {
>  			console_seq = log_first_seq;
>  			console_idx = log_first_idx;
> -		} else {
> -			len = 0;
>  		}

The printing loop becomes a bit complex with one more state, I should
say; but can do. We handle exclusive consoles there, dropped messages,
panic reset. But I see what you are trying to do.

	-ss