Messages in this thread | | | From | Kay Sievers <> | Date | Tue, 3 Apr 2012 17:50:23 +0200 | Subject | Re: [PATCH] printk(): add KERN_CONT where needed |
| |
On Tue, Apr 3, 2012 at 16:32, Joe Perches <joe@perches.com> wrote: > On Tue, 2012-04-03 at 12:30 +0200, Kay Sievers wrote: >> On Tue, Apr 3, 2012 at 05:47, Joe Perches <joe@perches.com> wrote: >> > I think you should do it "right" rather than add >> > trivial markers. >> >> The trivial markers _are_ correct. And they really fix things as soon >> as we start storing machine-readable records with printk(), instead of >> blindly glueing bytes together with each printk() call, for humans to >> puzzle with them if things go wrong. > > These KERN_CONT changes don't _fix_ things, > they just make it less likely to cause problems.
They sure changes things, as you can see in the first message in this thread. This is a real paste of output, not made up stuff.
It will matter as soon as we do not allow the next printk() to get appended to 'left-over garbage' of the last printk() without a trailing '\n'. We should not allow that, and the 'risk' of interleaving and garbled messages should be limited to threads which _both_ do continuation at the same time. The likeliness of that will get pretty low.
We should make absolutely sure, it will never affect the thread which does a proper single and complete line.
> Imagine two threads with printks extended with > KERN_CONT > > Thread 1: Thread 2: > printk(KERN_INFO "info message: "); > printk(KERN_ERR "err message: "); > printk(KERN_CONT "online\n"); > printk(KERN_CONT "offline\n"); > > Instead of a guarantee of "info message: online" and > "err message: offline", buffering could still join > the messages to "err message: online".
Yes, both threads play racy games with continuation, and that makes them both vulnerable to interleaving. But we must make sure, that single and complete lines are never affected by that.
I did not claim to address the problem of concurrent continuation line writers, and this patch has absolutely nothing to do with that problem. It _does_ fix encountered problems, and it is correct as it is. Please do not mix other issues you bring up here into it, they should go into a separate thread, unrelated to this patch.
> I believe the only _guaranteed_ way to correctly > assemble these messages is to use a initiator with > a cookie and pass that cookie to assembling printks. > > Something like: > > cookie = multi_printk_start() > multi_printk(cookie, level fmt, ...); > ... > multi_printk_end(cookie); > > Though get_current() might be a reasonable cookie > so perhaps the multi_ variants aren't needed.
Apart from *possibly* switching to per-cpu buffers for printk() core, I think the callers should probably just do that on their own, maybe just on the stack, if the message is small enough.
> git.kernel.org isn't responding right now. I > can't read the link you sent me privately to > check if you are using get_current() or some > other current_thread_info() constuct.
No, it does not change any current behaviour or gets any knowledge about threads, it just makes sure that full and single line always ends up in its own record instead of accepting unterminated left-overs from earlier printk()s.
Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |