On Sat 2016-12-10 12:10:22, Sergey Senozhatsky wrote: > On (12/09/16 17:46), Petr Mladek wrote: > > > -/* > > > - * Safe printk() for NMI context. It uses a per-CPU buffer to > > > - * store the message. NMIs are not nested, so there is always only > > > - * one writer running. But the buffer might get flushed from another > > > - * CPU, so we need to be careful. > > > - */ > > > > We should keep/create a good description here because the function > > has a non-trivial code. What about something like? > > > > which is really not related to this patch set.
I am sorry but I do not understand. This patch removes description that explained constrains of a rather complex code. In fact, the constrains has changed because we started using the function also in other context. When will be the right time/patchset to explain it? > > > > * Make sure that all old data have been read before the buffer was > > > @@ -261,14 +263,95 @@ void printk_safe_flush_on_panic(void) > > > printk_safe_flush(); > > > } > > > > > > +#ifdef CONFIG_PRINTK_NMI > > > +/* > > > + * Safe printk() for NMI context. It uses a per-CPU buffer to > > > + * store the message. NMIs are not nested, so there is always only > > > + * one writer running. But the buffer might get flushed from another > > > + * CPU, so we need to be careful. > > > + */ > > > > Hmm, I wanted to describe why we need another per-CPU buffer in NMI > > and I am not sure that we really need it. > > NMI-printk can interrupt safe-printk's vsnprintf() in the middle of > the "while (*fmt)" loop: safe-priNMI-PRINTK But this already happens when any of the WARNs is triggered inside vsnprintf(). Either this is safe or we are in trouble. Well, there is a difference. NMI can come at anytime and vsnprintf() continues printing the original string once we are back from NMI. But if we hit WARN() inside vsnprintf(), it usually means an error, vsnprintf() stops printing into the given buffer and returns. It means that it does not overwrite the message printed by the nested printks. The only exceptions are WARN_ONCE() calls in set_field_width() and set_precision(). They are self-repairing, vsnprintf() continues printing and will overwrite the nested warnings. Well, I am not sure if we should bother. By other words, we really need separate per-CPU buffer for NMI and the generic printk_safe. I am sorry for the noise. Well, is it that bad to ask for better comments? You see that I ended in quite some doubts, even found problems, when I tried to review the code carefully. Or am I dumb and it was all obvious? Best Regards, Petr PS: I know that I am sometimes in too nitpicking mode and it might be annoying and discouraging. I have to find the right boundaries.