Re: [PATCH] printk: Add best-effort printk() buffering.

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

Hello,

sorry for the delay.

On (04/30/17 22:54), Tetsuo Handa wrote:
> Sometimes we want to printk() multiple lines in a group without being
> disturbed by concurrent printk() from interrupts and/or other threads.
> For example, mixed printk() output of multiple thread's dump makes it
> hard to interpret.

hm, it's very close to what printk-safe does [and printk-nmi, of course].
the difference is that buffered-printk does not disable local IRQs,
unlike printk-safe, which has to do it by design. so the question is,
can buffered-printk impose atomicity requirements? it seems that it can
(am I wrong?). and, if so, then can we use printk-safe instead? we can
add a new printk_buffered_begin/printk_buffered_end API, for example,
(or enter/exit) for that purpose, that would set a buffered-printk
`printk_context' bit so we can flush buffers in a "special way", not via IRQ
work, and may be avoid message loss (printk-safe buffers are bigger in size
than proposed PAGE_SIZE buffers).


> This patch introduces fixed-sized statically allocated buffers for
> buffering printk() output for each thread/context in best effort
> (i.e. up to PAGE_SIZE bytes, up to 16 concurrent printk() users).

hm, 16 is rather random, it's too much for UP and probably not enough for
a 240 CPUs system. for the time being there are 3 buffered-printk users
(as far as I can see), but who knows how more will be added in the future.
each CPU can have overlapping printks from process, IRQ and NMI contexts.
for NMI we use printk-nmi buffers, so it's out of the list; but, in general,
*it seems* that we better depend on the number of CPUs the system has.
which, once again, returns us back to printk-safe...

thoughts?


[..]

> +/* Must not be called from NMI context. */
> +static void __flush_printk_buffer(struct printk_buffer *ptr, bool all)
> +{
> +	unsigned long flags;
> +
> +	if (!ptr->used)
> +		return;
> +
> +	/*
> +	 * Since printk_deferred() directly calls vprintk_emit(LOGLEVEL_SCHED),
> +	 * this function does not need to care about LOGLEVEL_SCHED case.
> +	 * Therefore, it is safe to call console_trylock() + console_unlock().
> +	 *
> +	 * We don't call boot_delay_msec(level) here because level is unknown.
> +	 */
> +	printk_delay();
> +
> +	/* This stops the holder of console_sem just where we want him */
> +	logbuf_lock_irqsave(flags);
> +	while (1) {
> +		char *text = ptr->buf;
> +		unsigned int text_len = ptr->used;
> +		char *cp = memchr(text, '\n', text_len);
> +		char c;

what guarantees that there'll always be a terminating newline?

	-ss