pr_cat() + CATSTR(name, size)?

pr_cat() + CATSTR(name, size)?

[Kay Sievers]

Hey Joe,

what do you think this?

It would make composing continuation lines at the caller side entirely
race-free, and it might fit into the usual pattern.

The more interesting thing, this would allow us to completely race-free
use the dev_printk() calls with continuation content, which we should
avoid otherwise for integrity reasons.

The patch below is just hacked it into the printk.c file to illustrate
the idea. It prints:
  [    0.000000] Kernel command line: root=/dev/sda2 ...
  [    0.000000] 12 34 56 78
  [    0.000000] PID hash table entries: 2048 (order: 2, 16384 bytes)

  5,96,0;Kernel command line: root=/dev/sda2 ...
  4,97,0;12 34 56 78
  6,98,0;PID hash table entries: 2048 (order: 2, 16384 bytes)

Thanks,
Kay


diff --git a/kernel/printk.c b/kernel/printk.c
index dba1821..1fd00b0 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -48,6 +48,32 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/printk.h>
 
+#define CATSTR(name, max)		\
+	char name[max];			\
+	size_t _##name_len = 0;		\
+	size_t _##name_max = max;
+
+#define pr_cat(name, fmt, ...)		\
+	_catstr(name, &_##name_len, _##name_max, fmt, ##__VA_ARGS__)
+
+ssize_t _catstr(char *s, size_t *len, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	size_t l = *len;
+	size_t r;
+
+	va_start(args, fmt);
+	r = vsnprintf(s + l, size - l, fmt, args);
+	va_end(args);
+
+	if (r >= size - l)
+		return -EINVAL;
+
+	*len += r;
+	s[*len] = '\0';
+	return r;
+}
+
 /*
  * Architectures can override it:
  */
@@ -668,6 +694,12 @@ void __init setup_log_buf(int early)
 	char *new_log_buf;
 	int free;
 
+	CATSTR(line, 80);
+	pr_cat(line, "%i ", 12);
+	pr_cat(line, "%i ", 34);
+	pr_cat(line, "%i ", 56);
+	pr_warn("%s%i\n", line, 78);
+
 	if (!new_log_buf_len)
 		return;
 

Re: pr_cat() + CATSTR(name, size)?

[Kay Sievers]

On Wed, 2012-07-11 at 12:33 +0200, Kay Sievers wrote:
> Hey Joe,
> 
> what do you think of this?
> 
> It would make composing continuation lines at the caller side entirely
> race-free, and it might fit into the usual pattern.
> 
> The more interesting thing, this would allow us to completely race-free
> use the dev_printk() calls with continuation content, which we should
> avoid otherwise for integrity reasons.

Better version with better range checking and _INIT() to reset the
string for re-use. It prints:
  [    0.000000] Kernel command line: root=/dev/sda2 ...
  [    0.000000] 1:-12 -34 -56 -78 -90
  [    0.000000] 2:-12 -34 -56 --90
  [    0.000000] 3:-12 -34 --90
  [    0.000000] 4:+12 +34 +-90
  [    0.000000] 5:~12 ~34 ~-90
  [    0.000000] PID hash table entries: 2048 (order: 2, 16384 bytes)

Thanks,
Kay


diff --git a/kernel/printk.c b/kernel/printk.c
index dba1821..1490153 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -48,6 +48,39 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/printk.h>
 
+#define CATSTR_INIT(name)		\
+	name##_len = 0;
+
+#define CATSTR_DEFINE(name, max)	\
+	char name[max];			\
+	size_t name##_len = 0;		\
+	size_t name##_max = max;
+
+#define pr_cat(name, fmt, ...)		\
+	_catstr(name, &name##_len, name##_max, fmt, ##__VA_ARGS__)
+
+ssize_t _catstr(char *s, size_t *len, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	size_t r;
+
+	if (*len == size)
+		return -EINVAL;
+
+	va_start(args, fmt);
+	r = vsnprintf(s + *len, size - *len, fmt, args);
+	va_end(args);
+
+	if (r >= size - *len) {
+		*len = size;
+		return -EINVAL;
+	}
+
+	*len += r;
+	s[*len] = '\0';
+	return r;
+}
+
 /*
  * Architectures can override it:
  */
@@ -668,6 +701,47 @@ void __init setup_log_buf(int early)
 	char *new_log_buf;
 	int free;
 
+	CATSTR_DEFINE(line, 24)
+	CATSTR_DEFINE(line2, 16)
+	CATSTR_DEFINE(line3, 12)
+
+	pr_cat(line, "1:");
+	pr_cat(line, "-%i ", 12);
+	pr_cat(line, "-%i ", 34);
+	pr_cat(line, "-%i ", 56);
+	pr_cat(line, "-%i ", 78);
+	pr_warn("%s-%i\n", line, 90);
+
+	pr_cat(line2, "2:");
+	pr_cat(line2, "-%i ", 12);
+	pr_cat(line2, "-%i ", 34);
+	pr_cat(line2, "-%i ", 56);
+	pr_cat(line2, "-%i ", 78);
+	pr_warn("%s-%i\n", line2, 90);
+
+	pr_cat(line3, "3:");
+	pr_cat(line3, "-%i ", 12);
+	pr_cat(line3, "-%i ", 34);
+	pr_cat(line3, "-%i ", 56);
+	pr_cat(line3, "-%i ", 78);
+	pr_warn("%s-%i\n", line3, 90);
+
+	CATSTR_INIT(line3)
+	pr_cat(line3, "4:");
+	pr_cat(line3, "+%i ", 12);
+	pr_cat(line3, "+%i ", 34);
+	pr_cat(line3, "+%i ", 56);
+	pr_cat(line3, "+%i ", 78);
+	pr_warn("%s-%i\n", line3, 90);
+
+	CATSTR_INIT(line3)
+	pr_cat(line3, "5:");
+	pr_cat(line3, "~%i ", 12);
+	pr_cat(line3, "~%i ", 34);
+	pr_cat(line3, "~%i ", 56);
+	pr_cat(line3, "~%i ", 78);
+	pr_warn("%s-%i\n", line3, 90);
+
 	if (!new_log_buf_len)
 		return;
 

Re: pr_cat() + CATSTR(name, size)?

[Joe Perches]

On Wed, 2012-07-11 at 12:33 +0200, Kay Sievers wrote:
> Hey Joe,

Hi Kay.

> what do you think this?
> 
> It would make composing continuation lines at the caller side entirely
> race-free, and it might fit into the usual pattern.

Maybe.  comments below...

> The more interesting thing, this would allow us to completely race-free
> use the dev_printk() calls with continuation content, which we should
> avoid otherwise for integrity reasons.
> 
> The patch below is just hacked it into the printk.c file to illustrate
> the idea. It prints:
>   [    0.000000] Kernel command line: root=/dev/sda2 ...
>   [    0.000000] 12 34 56 78
>   [    0.000000] PID hash table entries: 2048 (order: 2, 16384 bytes)
> 
>   5,96,0;Kernel command line: root=/dev/sda2 ...
>   4,97,0;12 34 56 78
>   6,98,0;PID hash table entries: 2048 (order: 2, 16384 bytes)
> 
> Thanks,
> Kay
> 
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index dba1821..1fd00b0 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -48,6 +48,32 @@
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/printk.h>
>  
> +#define CATSTR(name, max)		\
> +	char name[max];			\
> +	size_t _##name_len = 0;		\
> +	size_t _##name_max = max;
> +
> +#define pr_cat(name, fmt, ...)		\
> +	_catstr(name, &_##name_len, _##name_max, fmt, ##__VA_ARGS__)
> +
> +ssize_t _catstr(char *s, size_t *len, size_t size, const char *fmt, ...)
> +{
> +	va_list args;
> +	size_t l = *len;
> +	size_t r;
> +
> +	va_start(args, fmt);
> +	r = vsnprintf(s + l, size - l, fmt, args);
> +	va_end(args);
> +
> +	if (r >= size - l)
> +		return -EINVAL;
> +
> +	*len += r;
> +	s[*len] = '\0';
> +	return r;
> +}
> +
>  /*
>   * Architectures can override it:
>   */
> @@ -668,6 +694,12 @@ void __init setup_log_buf(int early)
>  	char *new_log_buf;
>  	int free;
>  
> +	CATSTR(line, 80);
> +	pr_cat(line, "%i ", 12);
> +	pr_cat(line, "%i ", 34);
> +	pr_cat(line, "%i ", 56);
> +	pr_warn("%s%i\n", line, 78);
> +
>  	if (!new_log_buf_len)
>  		return;

Interesting idea, perhaps workable, but it has
a few defects I can see.

It works for most uses, but it doesn't work for
when there are multiple function sites like

void dump_info(struct foo *bar)
{
	if (bar->baz)
		pr_cont("baz...");
}

---

	pr_info("Some initiator: ")
	dump_info(&descriptor);

Another negative is that is uses a local stack
variable for the entire line which increases
stack pressure.

It also fails to immediately output after some
defect unlike your change to output directly to
console.

It would require all sites with continuation lines
be modified.  Because it requires in-situ code
modifications, I'd prefer a cookie based approach.

I think it's more flexible, allows the cookie to be
passed into extending functions and doesn't demand
(much) extra stack.

Something like:
https://lkml.org/lkml/2012/4/3/231
https://lkml.org/lkml/2011/11/14/349

cheers, Joe

Re: pr_cat() + CATSTR(name, size)?

[Kay Sievers]

On Wed, Jul 11, 2012 at 5:01 PM, Joe Perches <joe@perches.com> wrote:

> On Wed, 2012-07-11 at 12:33 +0200, Kay Sievers wrote:
> Interesting idea, perhaps workable, but it has
> a few defects I can see.
>
> It works for most uses, but it doesn't work for
> when there are multiple function sites like
>
> void dump_info(struct foo *bar)
> {
>         if (bar->baz)
>                 pr_cont("baz...");
> }
>
> ---
>
>         pr_info("Some initiator: ")
>         dump_info(&descriptor);

Yeah, it's just the common case, not for the "creative" ones. :)

> Another negative is that is uses a local stack
> variable for the entire line which increases
> stack pressure.

The thing is to avoid malloc(), and ~100 bytes are kind of OK for the
time we do the printk, I guess

> It also fails to immediately output after some
> defect unlike your change to output directly to
> console.

Which is really only that important for stuff that causes crashes
between the outputting fragments.

There are _many_ cases the console lock is held, and we don't print
stuff immediately out to the console, and we never ensured that in the
past. There was never a guarantee that stuff ended up on the console,
kmsg was always and needs to be a store+forward model.

> It would require all sites with continuation lines
> be modified.  Because it requires in-situ code
> modifications, I'd prefer a cookie based approach.

Well, it would be mostly for the dev_printk() stuff, which should
ideally never be merged with stuff that could go wrong.

> I think it's more flexible, allows the cookie to be
> passed into extending functions and doesn't demand
> (much) extra stack.
>
> Something like:
> https://lkml.org/lkml/2012/4/3/231
> https://lkml.org/lkml/2011/11/14/349

Hmm, how do we manage memory allocations here? We can get around that
somehow? It's something the common printk() must really avoid.

Kay

Re: pr_cat() + CATSTR(name, size)?

[Joe Perches]

On Wed, 2012-07-11 at 17:14 +0200, Kay Sievers wrote:
> On Wed, Jul 11, 2012 at 5:01 PM, Joe Perches <joe@perches.com> wrote:
[]
> There are _many_ cases the console lock is held, and we don't print
> stuff immediately out to the console, and we never ensured that in the
> past. There was never a guarantee that stuff ended up on the console,
> kmsg was always and needs to be a store+forward model.

I'm less concerned with kmsg than you.
I think it's more a nicety than anything.

> > It would require all sites with continuation lines
> > be modified.  Because it requires in-situ code
> > modifications, I'd prefer a cookie based approach.
> 
> Well, it would be mostly for the dev_printk() stuff, which should
> ideally never be merged with stuff that could go wrong.

Perhaps that's ideal, but not practical.
printk continuations are more prevalent.

> > I think it's more flexible, allows the cookie to be
> > passed into extending functions and doesn't demand
> > (much) extra stack.
> >
> > Something like:
> > https://lkml.org/lkml/2012/4/3/231
> > https://lkml.org/lkml/2011/11/14/349
> 
> Hmm, how do we manage memory allocations here? We can get around that
> somehow? It's something the common printk() must really avoid.

Well, I think the malloc costs are pretty low
and could devolve pretty easily when OOM.

cookie=NULL, directly emit.

Anyway, interesting idea, keep at it, see what
comes out of it.

cheers, Joe

Re: pr_cat() + CATSTR(name, size)?

[Kay Sievers]

On Wed, Jul 11, 2012 at 5:30 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2012-07-11 at 17:14 +0200, Kay Sievers wrote:
>> On Wed, Jul 11, 2012 at 5:01 PM, Joe Perches <joe@perches.com> wrote:
> []
>> There are _many_ cases the console lock is held, and we don't print
>> stuff immediately out to the console, and we never ensured that in the
>> past. There was never a guarantee that stuff ended up on the console,
>> kmsg was always and needs to be a store+forward model.
>
> I'm less concerned with kmsg than you.
> I think it's more a nicety than anything.

Sure, just saying, that there is never a guarantee that stuff lands
directly on the console  at print time, we never had that. It just
usually gets there directly. There is trylock(console_sem), if we can
not get that, we always leave the data in the buffer and let someone
else push it with the next unlock, and that can be later. That
behaviour was always the case.

>> > It would require all sites with continuation lines
>> > be modified.  Because it requires in-situ code
>> > modifications, I'd prefer a cookie based approach.
>>
>> Well, it would be mostly for the dev_printk() stuff, which should
>> ideally never be merged with stuff that could go wrong.
>
> Perhaps that's ideal, but not practical.
> printk continuations are more prevalent.

We can't have everything. We can not merge the data at least, for
integrity reasons. We can only make it *look* like it belonged
together, like we do when we run into a race or the console is locked
and we can not merge continuation fragments into one record.

>> > I think it's more flexible, allows the cookie to be
>> > passed into extending functions and doesn't demand
>> > (much) extra stack.
>> >
>> > Something like:
>> > https://lkml.org/lkml/2012/4/3/231
>> > https://lkml.org/lkml/2011/11/14/349
>>
>> Hmm, how do we manage memory allocations here? We can get around that
>> somehow? It's something the common printk() must really avoid.
>
> Well, I think the malloc costs are pretty low
> and could devolve pretty easily when OOM.

We need to avoid allocating memory in situations where we want to
printk(), it's just not possible. That's why all the kmsg/printk can
not really do any plain malloc. All printk memory needs to be static,
on the stack or somehow pre-allocated.

> Anyway, interesting idea, keep at it, see what
> comes out of it.

Just depends on us, I guess. :)

Thanks,
Kay

Re: pr_cat() + CATSTR(name, size)?

[Joe Perches]

On Wed, 2012-07-11 at 17:48 +0200, Kay Sievers wrote:
> On Wed, Jul 11, 2012 at 5:30 PM, Joe Perches <joe@perches.com> wrote:
> > Well, I think the malloc costs are pretty low
> > and could devolve pretty easily when OOM.
> 
> We need to avoid allocating memory in situations where we want to
> printk(), it's just not possible.

"it's just not possible???"  Kay, them's fightin' words. :)

> That's why all the kmsg/printk can
> not really do any plain malloc. All printk memory needs to be static,
> on the stack or somehow pre-allocated.

Maybe, I was planning to play with it after
refactoring printk in the next couple releases.

> > Anyway, interesting idea, keep at it, see what
> > comes out of it.
> 
> Just depends on us, I guess. :)

Yup.

If your solution is just for the dev_<level> messages
(ie: with vprintk_emit descriptors), then it's not
too ugly.

Did you look at the remaining dev_<level> and printk
continuations grep pattern?  There really aren't too
many to fix up.

Maybe in 3.6.  None of them appear particularly urgent.

One trivial style note:

Maybe CATSTR could use a struct and a DECLARE_ macro?

struct printk_continuation_buffer {
	size_t length;
	size_t pos;
	char buf[];
}

It's a pity gcc doesn't allow non-static declarations like:

#define DECLARE_PRINTK_BUF(name, size)		\
struct printk_continuation_buffer name = {	\
	.length = size;				\
	.pos = 0;				\
	.buf[size] = {0};			\
}

So maybe a DECLARE/DESTROY thing could work
with the appropriate malloc/free.

cheers, Joe

Re: pr_cat() + CATSTR(name, size)?

[Kay Sievers]

On Wed, Jul 11, 2012 at 6:55 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2012-07-11 at 17:48 +0200, Kay Sievers wrote:
>> On Wed, Jul 11, 2012 at 5:30 PM, Joe Perches <joe@perches.com> wrote:
>> > Well, I think the malloc costs are pretty low
>> > and could devolve pretty easily when OOM.
>>
>> We need to avoid allocating memory in situations where we want to
>> printk(), it's just not possible.
>
> "it's just not possible???"  Kay, them's fightin' words. :)

Nah, I meant it. :) It limits the usefulness of these functions. We
can not safely allocate memory, or do not get any memory in some
situations where we want to use printk(). Hey, it might be used to say
printk("out of memory\n").

>> That's why all the kmsg/printk can
>> not really do any plain malloc. All printk memory needs to be static,
>> on the stack or somehow pre-allocated.
>
> Maybe, I was planning to play with it after
> refactoring printk in the next couple releases.

Sounds good.

>> > Anyway, interesting idea, keep at it, see what
>> > comes out of it.
>>
>> Just depends on us, I guess. :)

> If your solution is just for the dev_<level> messages
> (ie: with vprintk_emit descriptors), then it's not
> too ugly.

Yeah, I thought only about these. But there might be more users where
it makes sense to do that in a more reliable manner, don't know. It
was surely no meant to replace the remaining 99.9% of the other cont
users. :)

> Did you look at the remaining dev_<level> and printk
> continuations grep pattern?  There really aren't too
> many to fix up.

Yeah, it looks fine to fix these few.

> Maybe in 3.6.  None of them appear particularly urgent.

Right.

> One trivial style note:
>
> Maybe CATSTR could use a struct and a DECLARE_ macro?
>
> struct printk_continuation_buffer {
>         size_t length;
>         size_t pos;
>         char buf[];
> }

Yeah, but then we lose the simplicity of passing the normal string
around, and we need accessor macros to get to the string when we pass
it around later. Maybe it's still OK, but it's surely not so intuitive
anymore.

> It's a pity gcc doesn't allow non-static declarations like:
>
> #define DECLARE_PRINTK_BUF(name, size)          \
> struct printk_continuation_buffer name = {      \
>         .length = size;                         \
>         .pos = 0;                               \
>         .buf[size] = {0};                       \
> }

Yeah, when the size changes, we have different type of struct. So we
can not name them all "printk_continuation_buffer", every different
size would conflict with each other.

Also  = {0} on an array forces a memset() of the entire array, nothing
wrong, but it's not really needed.

> So maybe a DECLARE/DESTROY thing could work
> with the appropriate malloc/free.

Hmm, I really don't think we can teach the people, or expect them to
know, that these printk() functions are fragile if used in some
critical code paths. It would at least need the GFP flags and in many
cases GFP_ATOMIC which can easily fail, and we would also need to do
error checking then, and printk() should just never fail, because it
is used to tell that something went wrong. We have the entire kmsg
buffer pre-allocated at bootup for that reason.

I think the only really sane option here is to use the (usually
~50-100 bytes) stack. Or did you have another idea here which I
missed?

Kay

Re: pr_cat() + CATSTR(name, size)?

[Joe Perches]

On Wed, 2012-07-11 at 19:25 +0200, Kay Sievers wrote:
> On Wed, Jul 11, 2012 at 6:55 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2012-07-11 at 17:48 +0200, Kay Sievers wrote:
> >> On Wed, Jul 11, 2012 at 5:30 PM, Joe Perches <joe@perches.com> wrote:
> >> > Well, I think the malloc costs are pretty low
> >> > and could devolve pretty easily when OOM.
> >>
> >> We need to avoid allocating memory in situations where we want to
> >> printk(), it's just not possible.
> >
> > "it's just not possible???"  Kay, them's fightin' words. :)
> 
> Nah, I meant it. :) It limits the usefulness of these functions. We
> can not safely allocate memory, or do not get any memory in some
> situations where we want to use printk(). Hey, it might be used to say
> printk("out of memory\n").

:)

> >> That's why all the kmsg/printk can
> >> not really do any plain malloc. All printk memory needs to be static,
> >> on the stack or somehow pre-allocated.
> >
> > Maybe, I was planning to play with it after
> > refactoring printk in the next couple releases.
> 
> Sounds good.
> 
> >> > Anyway, interesting idea, keep at it, see what
> >> > comes out of it.
> >>
> >> Just depends on us, I guess. :)
> 
> > If your solution is just for the dev_<level> messages
> > (ie: with vprintk_emit descriptors), then it's not
> > too ugly.
> 
> Yeah, I thought only about these. But there might be more users where
> it makes sense to do that in a more reliable manner, don't know. It
> was surely no meant to replace the remaining 99.9% of the other cont
> users. :)

I believe your current reassembly code only works
on a maximum of 2 interleaved threads.  Did that change?

> > Did you look at the remaining dev_<level> and printk
> > continuations grep pattern?  There really aren't too
> > many to fix up.
> 
> Yeah, it looks fine to fix these few.
> 
> > Maybe in 3.6.  None of them appear particularly urgent.
> 
> Right.
> 
> > One trivial style note:
> >
> > Maybe CATSTR could use a struct and a DECLARE_ macro?
> >
> > struct printk_continuation_buffer {
> >         size_t length;
> >         size_t pos;
> >         char buf[];
> > }
> 
> Yeah, but then we lose the simplicity of passing the normal string
> around, and we need accessor macros to get to the string when we pass
> it around later. Maybe it's still OK, but it's surely not so intuitive
> anymore.
> 
> > It's a pity gcc doesn't allow non-static declarations like:
> >
> > #define DECLARE_PRINTK_BUF(name, size)          \
> > struct printk_continuation_buffer name = {      \
> >         .length = size;                         \
> >         .pos = 0;                               \
> >         .buf[size] = {0};                       \
> > }
> 
> Yeah, when the size changes, we have different type of struct. So we
> can not name them all "printk_continuation_buffer", every different
> size would conflict with each other.

It doesn't work so it doesn't matter no?

> > So maybe a DECLARE/DESTROY thing could work
> > with the appropriate malloc/free.
> 
> Hmm, I really don't think we can teach the people, or expect them to
> know, that these printk() functions are fragile if used in some
> critical code paths.

Vigilance. (and maybe a checkpatch test :).
There just aren't many critical code paths.

> It would at least need the GFP flags and in many
> cases GFP_ATOMIC which can easily fail, and we would also need to do
> error checking then, and printk() should just never fail, because it
> is used to tell that something went wrong. We have the entire kmsg
> buffer pre-allocated at bootup for that reason.

I still think devolving to direct printks when OOM works
as a fallback just fine.

> I think the only really sane option here is to use the (usually
> ~50-100 bytes) stack. Or did you have another idea here which I
> missed?

Other than malloc, I don't think there's another option.
Anyone else?

Vegard?  Are you still around?

Do you want to revive something like the blocks in:
https://lkml.org/lkml/2007/10/4/367

Re: pr_cat() + CATSTR(name, size)?

[Kay Sievers]

On Wed, Jul 11, 2012 at 7:51 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2012-07-11 at 19:25 +0200, Kay Sievers wrote:

>> > If your solution is just for the dev_<level> messages
>> > (ie: with vprintk_emit descriptors), then it's not
>> > too ugly.
>>
>> Yeah, I thought only about these. But there might be more users where
>> it makes sense to do that in a more reliable manner, don't know. It
>> was surely no meant to replace the remaining 99.9% of the other cont
>> users. :)
>
> I believe your current reassembly code only works
> on a maximum of 2 interleaved threads.  Did that change?

No, two threads doing continuation printk at the same time, will still
race against each other, and we can not reconstruct full and correct
lines later. But it's much less likely, because multiple different
continuation prints at the same time are very rare.

>> Yeah, when the size changes, we have different type of struct. So we
>> can not name them all "printk_continuation_buffer", every different
>> size would conflict with each other.
>
> It doesn't work so it doesn't matter no?

Right, it can't work.

>> Hmm, I really don't think we can teach the people, or expect them to
>> know, that these printk() functions are fragile if used in some
>> critical code paths.
>
> Vigilance. (and maybe a checkpatch test :).
> There just aren't many critical code paths.

>> printk() should just never fail, because it
>> is used to tell that something went wrong. We have the entire kmsg
>> buffer pre-allocated at bootup for that reason.
>
> I still think devolving to direct printks when OOM works
> as a fallback just fine.

I don't think we should ever try to call malloc(), it would get us into
too much trouble.

I just played a bit more with the pr_cat() idea. We can completely race
free, and limited to pretty-looking line lengths, put out large lists of
items.

It would usually just use an 80 char buffer for the line on the
stack.

[    0.000000] Kernel command line: root=/dev/sda2 ...
[    0.000000] 2:-12 -34 -56 -90
[    0.000000] 3:-12 -34 -90
[    0.000000] 4:+12 +34 -90
[    0.000000] 5:~12 ~34 -90
[    0.000000] foo: #0 #1 #2 #3 #4 #5 #6 #7 #8 #9 #10 #11 #12 #13 #14 #15 #16 #17
[    0.000000] foo+: #18 #19 #20 #21 #22 #23 #24 #25 #26 #27 #28 #29 #30 #31 #32
[    0.000000] foo+: #33 #34 #35 #36 #37 #38 #39 #40 #41 #42 #43 #44 #45 #46 #47
[    0.000000] foo+: #48 #49
[    0.000000] PID hash table entries: 2048 (order: 2, 16384 bytes)

diff --git a/kernel/printk.c b/kernel/printk.c
index 177fa49..0f4df08 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -48,6 +48,40 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/printk.h>
 
+#define CATSTR_INIT(name)		\
+	name##_len = 0
+
+#define CATSTR_DEFINE(name, max)	\
+	char name[max];			\
+	size_t name##_len = 0;		\
+	size_t name##_max = max
+
+#define pr_cat(name, fmt, ...)		\
+	_pr_cat(name, &name##_len, name##_max, fmt, ##__VA_ARGS__)
+
+bool _pr_cat(char *s, size_t *len, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	size_t r;
+
+	if (*len == size)
+		return false;
+
+	va_start(args, fmt);
+	r = vsnprintf(s + *len, size - *len, fmt, args);
+	va_end(args);
+
+	if (r + 1 >= size - *len) {
+		s[*len] = '\0';
+		*len = size;
+		return false;
+	}
+
+	*len += r;
+	s[*len] = '\0';
+	return true;
+}
+
 /*
  * Architectures can override it:
  */
@@ -587,6 +621,11 @@ static int devkmsg_open(struct inode *inode, struct file *file)
 	struct devkmsg_user *user;
 	int err;
 
+	console_lock();
+	print_modules();
+	print_modules();
+	console_unlock();
+
 	/* write-only does not need any file context */
 	if ((file->f_flags & O_ACCMODE) == O_WRONLY)
 		return 0;
@@ -671,6 +710,59 @@ void __init setup_log_buf(int early)
 	unsigned long flags;
 	char *new_log_buf;
 	int free;
+	int i;
+
+	CATSTR_DEFINE(line, 64);
+	CATSTR_DEFINE(line2, 16);
+	CATSTR_DEFINE(line3, 12);
+
+	pr_cat(line, "1:");
+	pr_cat(line, "-%i ", 12);
+	pr_cat(line, "-%i ", 34);
+	pr_cat(line, "-%i ", 56);
+	pr_cat(line, "-%i ", 78);
+	pr_info("%s-%i\n", line, 90);
+
+	pr_cat(line2, "2:");
+	pr_cat(line2, "-%i ", 12);
+	pr_cat(line2, "-%i ", 34);
+	pr_cat(line2, "-%i ", 56);
+	pr_cat(line2, "-%i ", 78);
+	pr_info("%s-%i\n", line2, 90);
+
+	pr_cat(line3, "3:");
+	pr_cat(line3, "-%i ", 12);
+	pr_cat(line3, "-%i ", 34);
+	pr_cat(line3, "-%i ", 56);
+	pr_cat(line3, "-%i ", 78);
+	pr_info("%s-%i\n", line3, 90);
+
+	CATSTR_INIT(line3);
+	pr_cat(line3, "4:");
+	pr_cat(line3, "+%i ", 12);
+	pr_cat(line3, "+%i ", 34);
+	pr_cat(line3, "+%i ", 56);
+	pr_cat(line3, "+%i ", 78);
+	pr_info("%s-%i\n", line3, 90);
+
+	CATSTR_INIT(line3);
+	pr_cat(line3, "5:");
+	pr_cat(line3, "~%i ", 12);
+	pr_cat(line3, "~%i ", 34);
+	pr_cat(line3, "~%i ", 56);
+	pr_cat(line3, "~%i ", 78);
+	pr_info("%s-%i\n", line3, 90);
+
+	CATSTR_INIT(line);
+	pr_cat(line, "foo:");
+	for (i = 0; i < 50; i++) {
+		if (!pr_cat(line, " #%i", i)) {
+			pr_info("%s #%i\n", line, i);
+			CATSTR_INIT(line);
+			pr_cat(line, "foo+:");
+		}
+	}
+	pr_info("%s\n", line);
 
 	if (!new_log_buf_len)
 		return;