Re: [PATCH] printk: inject caller information into the body of message

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

On (09/28/18 20:01), Tetsuo Handa wrote:
> > Yes, this makes sense. At the same time we can keep pr_line buffer
> > in .bss
> > 
> > 	static char buffer[1024];
> > 	static DEFINE_PR_LINE_BUF(..., buffer);
> > 
> > just like you have already mentioned. But that's going to require a
> > case-by-case handling; so a big list of printk buffers is a simpler
> > option. Fallback, tho, can be painful. On a system with 1024 CPUs can
> > one have more than 16 concurrent cont printks? If the answer is yes,
> > then we are looking at the same broken cont output as before.
> 
> I'm OK with making "16" configurable (at kernel configuration and/or
> at kernel boot like log_buf_len= kernel command line parameter).

Do we really want this? Why .bss placement doesn't work for you?

	void oom(...)
	{
		static DEFINE_PR_LINE(KERN_ERR, pr);

		pr_line(&pr, ....);
		pr_line(&pr, "\n");
	}

the underlying buffer will be static; the pr_line will get re-init
(offset = 0) every time we call the function, which is OK. And we can
pass &pr to any function oom() invokes. What am I missing?

> We could even allow each "struct task_struct" to have corresponding
> "struct printk_buffer".

Tetsuo, realistically, we can't. Sorry. No one will let us to have a printk
buffer on per-task_struct basis. Even if someone will let us to do this,
a miracle, a single per-task_struct buffer won't work. Because, then
someone will discover that a very simple API

	buffered_printk(current->printk_buffer, "......");

does not work if buffered_printk() gets interrupted by IRQ, etc. in case
if that new context also does

	buffered_printk(current->printk_buffer, "......");

So then we will have per-context per-task_struct printk buffer: for task,
for exceptions, for softirq, for hardirq, for NMI, etc. This is not worth
it.

Let's just have a very simple seq_buf based pr_line API. No config options,
no command line arguments - heap, bss or stack for buffer placement. Or even
simpler.

	-ss