From: Petr Mladek <email@example.com> To: Tetsuo Handa <firstname.lastname@example.org> Cc: Sergey Senozhatsky <email@example.com>, Linus Torvalds <firstname.lastname@example.org>, Sergey Senozhatsky <email@example.com>, Dmitriy Vyukov <firstname.lastname@example.org>, Steven Rostedt <email@example.com>, Alexander Potapenko <firstname.lastname@example.org>, Fengguang Wu <email@example.com>, Josh Poimboeuf <firstname.lastname@example.org>, LKML <email@example.com>, Andrew Morton <firstname.lastname@example.org>, email@example.com, Ingo Molnar <firstname.lastname@example.org>, Peter Zijlstra <email@example.com>, Will Deacon <firstname.lastname@example.org> Subject: Re: [PATCH 3/3] lockdep: Use line-buffered printk() for lockdep messages. Date: Fri, 9 Nov 2018 16:43:26 +0100 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Fri 2018-11-09 18:55:26, Tetsuo Handa wrote: > On 2018/11/09 15:12, Sergey Senozhatsky wrote: > > On (11/08/18 20:37), Tetsuo Handa wrote: > > > On 2018/11/08 13:45, Sergey Senozhatsky wrote: > How early_printk requirement affects line buffered printk() API? > > I don't think it is impossible to convert from > > printk("Testing feature XYZ.."); > this_may_blow_up_because_of_hw_bugs(); > printk(KERN_CONT " ... ok\n"); > > to > > printk("Testing feature XYZ:\n"); > this_may_blow_up_because_of_hw_bugs(); > printk("Testing feature XYZ.. ... ok\n"); > > in https://lore.kernel.org/lkml/CA+55aFwmwdY_mMqdEyFPpRhCKRyeqj=+aCqe5nN108v8ELFvPw@mail.gmail.com/ . I just wonder how this pattern is common. I have tried but I failed to find any instance. This problem looks like a big argument against explicit buffers. But I wonder if it is real. > > > > So in my email I was not advertising printk_safe as a "buffered printk for > > everyone", I was just talking about lockdep. It's a bit doubtful that Peter > > will ACK lockdep transition to buffered printk. > > What Linus is suggesting is "helper functions with no printk()", which is > more difficult than my "helper functions with printk()" because we need to > calculate enough buffer size (which may traverse many functions) because > we can't printk() from helper functions when buffer size is too small. > > I'm wondering if Linus still insists scattering "a data structure which > Linus suggested" around. Rather, I'd like to propose below one. This is not > perfect but modification is much smaller. > > + * Line buffered printk() tries to assign a buffer when printk() from a new > + * context identifier comes in. And it automatically releases that buffer when > + * one of three conditions listed below became true. > + * > + * (1) printk() from that context identifier emitted '\n' as the last > + * character of output. > + * (2) printk() from that context identifier tried to print a too long line > + * which cannot be stored into a buffer. > + * (3) printk() from a new context identifier noticed that some context > + * identifier is reserving a buffer for more than 10 seconds without > + * emitting '\n'. > + * > + * Since (3) is based on a heuristic that somebody forgot to emit '\n' as the > + * last character of output(), pr_cont()/KERN_CONT users are expected to emit > + * '\n' within 10 seconds even if they reserved a buffer. This is my main concern about this approach. It is so easy to omit the final '\n'. They are currently delayed until another printk(). Even this is bad. Unfortunately we could not setup timer from printk() because it would add more locks into the game. This patch will make it worse. They might get delayed by 10s or even more. Many other lines might appear in between. Also the code is more tricky[*]. Sign, I am really unhappy how the buffered_printk() conversion looks in lockdep.c. But I still think that the approach is more reliable. I am going to investigate much more pr_cont() users. I wonder how many would be that complicated. I do not want to give up just because one use-case that was complicated even before. [*] The buffer can get written and flushed by different processes. It is not trivial to do it correctly a lockless way. The proposed locking looks right on the first glance. But the code is a bit scary ;-)
next prev parent reply other threads:[~2018-11-09 15:43 UTC|newest] Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-02 13:31 [PATCH v6 1/3] printk: Add line-buffered printk() API Tetsuo Handa 2018-11-02 13:31 ` [PATCH 2/3] mm: Use line-buffered printk() for show_free_areas() Tetsuo Handa 2018-11-07 14:07 ` Petr Mladek 2018-11-02 13:31 ` [PATCH 3/3] lockdep: Use line-buffered printk() for lockdep messages Tetsuo Handa 2018-11-02 13:36 ` Peter Zijlstra 2018-11-03 2:00 ` Tetsuo Handa 2018-11-06 8:38 ` Petr Mladek 2018-11-06 9:05 ` Sergey Senozhatsky 2018-11-06 12:57 ` Petr Mladek 2018-11-06 9:56 ` Tetsuo Handa 2018-11-07 15:19 ` Petr Mladek 2018-11-08 4:45 ` Sergey Senozhatsky 2018-11-08 11:37 ` Tetsuo Handa 2018-11-09 6:12 ` Sergey Senozhatsky 2018-11-09 9:55 ` Tetsuo Handa 2018-11-09 15:43 ` Petr Mladek [this message] 2018-11-10 2:42 ` Tetsuo Handa 2018-11-23 12:46 ` Petr Mladek 2018-11-23 13:12 ` Tetsuo Handa 2018-11-23 15:56 ` Steven Rostedt 2018-11-24 0:24 ` Tetsuo Handa 2018-11-26 4:34 ` Sergey Senozhatsky 2018-11-28 13:29 ` David Laight 2018-11-29 10:09 ` Tetsuo Handa 2018-11-30 16:01 ` Petr Mladek 2018-11-10 8:52 ` Tetsuo Handa 2018-11-23 12:52 ` Petr Mladek 2018-11-09 14:08 ` Linus Torvalds 2018-11-09 14:42 ` Steven Rostedt 2018-11-08 11:53 ` Petr Mladek 2018-11-08 12:44 ` Sergey Senozhatsky 2018-11-08 14:21 ` Sergey Senozhatsky 2018-11-09 9:54 ` Tetsuo Handa 2018-11-02 14:40 ` [PATCH v6 1/3] printk: Add line-buffered printk() API Matthew Wilcox 2018-11-03 1:55 ` Tetsuo Handa 2018-11-02 18:12 ` kbuild test robot 2018-11-02 18:12 ` kbuild test robot 2018-11-06 14:35 ` Sergey Senozhatsky 2018-11-07 10:21 ` Petr Mladek 2018-11-07 12:54 ` Tetsuo Handa 2018-11-08 2:21 ` Sergey Senozhatsky 2018-11-08 11:24 ` Petr Mladek 2018-11-08 11:46 ` Tetsuo Handa 2018-11-08 12:30 ` Sergey Senozhatsky 2018-11-09 14:10 ` Petr Mladek 2018-11-12 7:59 ` Sergey Senozhatsky 2018-11-12 10:42 ` Tetsuo Handa 2018-11-17 10:14 ` Tetsuo Handa 2018-11-07 10:52 ` Tetsuo Handa 2018-11-07 11:01 ` David Laight 2018-11-07 12:00 ` Petr Mladek 2018-11-07 11:45 ` Petr Mladek 2018-11-08 2:30 ` Sergey Senozhatsky 2018-11-07 13:41 ` Petr Mladek 2018-11-07 14:06 ` Petr Mladek
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH 3/3] lockdep: Use line-buffered printk() for lockdep messages.' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.