On (09/19/18 20:02), Tetsuo Handa wrote:
> I'm inclined to propose a simple one shown below, similar to just having
> several "struct cont" for concurrent printk() users.
Tetsuo, thanks for the patch.
> What Linus has commented is that implicit context is bad, and below one
> uses explicit context.
> After almost all users are converted to use below one, we might be able
> to get rid of KERN_CONT support.
The good thing about cont buffer is that we flush it on panic. E.g.
core/arch early boot stage can do:
pr_cont("going to call early_init_foo()...");
early_init_foo();
pr_cont("OK\n");
should early_init_foo() panic the system we will have
"going to call early_init_foo()" on the serial console. This can
be addressed if you'd iterate printk_buffers[] in flush_on_panic().
> +#define MAX_PRINTK_BUFFERS 16
> +static struct printk_buffer printk_buffers[MAX_PRINTK_BUFFERS];
Well, hmm, maybe. Now can we have a problem of either too-small or too-large
MAX_PRINTK_BUFFERS. 16 buffers on a 4 CPU arm board most probably will just
waste some memory. At the same time we probably don't want to have NR_CPUS
buffers. The fallback to "regular printk" is still a bit troubling - technically
there may be cases when we don't fix anything.
So, overall, I'm not against your patch. There are some pros and cons,
however.
pr_line() patch seems to be simpler [probably] and smaller [definitely].
The only problem, as you have mentioned, is that people may miscalculate
the size of the buffer, which won't crash us or anything; people can overshot
even a LOG_LINE_MAX buffer. So probably I'm not completely sold on having
a fixed size printk_buffers[].
May be all we want at the end is to drop explicit buffer API and have just
two options in pr_line:
DEFINE_PR_LINE() -- 80-bytes (or 256) pr_line // implicit buffer
DEFINE_PR_LINE_HUGE() -- 1024-bytes pr_line // implicit buffer
So, no explicit buffers, just "a normal" pr_line or "a huge" pr_line.
And no "normal printk" fallback; buffered printk line stays buffered.
The 80-bytes limit can be lifted to, say, 256-bytes.
Tetsuo, do you still want to have a fixed size array of printk buffers?
What do others think?
BTW, Tetsuo, I have addressed your pr_line suggestions/corrections.
Couldn't send the patch or reply to emails because I was offline for
a week due to personal reasons; but I can send it now - it does not
have DEFINE_PR_LINE_HUGE() macro. Just a previous version with
corrections which you have pointed out.
-ss
