Re: printk: what is going on with additional newlines?

Sergey Senozhatsky
2017-08-29T17:37:16Z

Hello,

On (08/29/17 10:00), Linus Torvalds wrote:
> On Tue, Aug 29, 2017 at 6:40 AM, Sergey Senozhatsky
> <sergey.senozhatsky.work@gmail.com> wrote:
> > Pavel reported that
> > printk("foo"); printk("bar");
> >
> > now does not produce a single continuation "foobar" line, but
> > instead produces two lines
> > foo
> > bar
>
> And that's the *correct* behavior.

ok. thanks for taking a look.

> Stop trying to fix that. Fix the printk's instead.
>
> In particular, the
>
> printk("bar");
>
> could have come from an interrupt, and have nothing what-so-ever to do
> with "foo".
>
> If you want continuations, you
>
> (a) make sure the first one doesn't end in a newline
>
> (b) make sure the second printk has a KERN_CONT
>
> (c) even after that, ask yourself how much you _really_ want
> continuations, because there are going to be situations where it still
> doesn't work.

yes, continuations are not really welcomed. I thought that this
particular case could be considered a regression. but your position
is pretty clear.

> I refuse to help those things. We mis-designed things, and the
> continuations were a mistake to begin with, but they were a mistake
> that was understandable in the timeframe they happened. But it's not
> something we should support, and it's most definitely is not something
> we should then say "oh, you were broken shit that didn't even bother
> to add the KERN_CONT, let me help your crap".
>
> No.
>
> Only acceptable use of continuations is basically boot-time testing,
> when you do things like
>
> printk("Testing feature XYZ..");
> this_may_blow_up_because_of_hw_bugs();
> printk(KERN_CONT " ... ok\n");
>
> and anything else you should seriously try to marshal the data
> *before* doing a printk(), and not expect printk() to marshal it for
> you.

ok. that's something several people asked for -- some sort of buffered
printk mode; but people don't want to use a buffer allocated on the stack
(or kmalloc-ed, etc.) to do sprintf() on it and then feed it to printk("%s"),
because this adds some extra cost:

void foo(void)
{
char cont_string[256];
size_t sz;

sz = sprintf(cont_string + sz, "%xxxx", data1...);
do_abc()
sz += sprintf(cont_string + sz, "%xxxx", data1...);

....

printk("%s\n", cont_string) // does "sprintf" again
// and then memcpy
}

I thought about re-using printk-safe per-CPU buffers for that purpose.
this saves us memory, because printk-safe buffers are always there, but
it has some disadvantages. namely, to use printk-safe buffer we need to
disable local interrupts. so something like this

printk_buffered_mode_begin(); // disables local irq

printk() // appends data to the per-CPU buffer
printk()
printk()

printk_buffered_mode_end(); // append messages to consequent logbuf
// entries
// enable local irqs.

... not sure, how usable this will end up to be.
probably not usable at all.

> But for legacy reasons, we do end up trying to support KERN_CONT.
> Just barely.
>
> I'd really like to get rid of it entirely, because the whole log-based
> structure really really doesn't work well for it (what if somebody has
> already read the partial line from the logs?)
>
> Our printk stuff didn't used to be log-based. It was just a plain
> character-based circular buffer. Back then that KERN_CONT made a whole
> lot more sense.

-ss