Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200709132344.760-1-john.ogness@linutronix.de>
 <20200709132344.760-5-john.ogness@linutronix.de> <20200718121053.GA691245@elver.google.com>
 <20200719034312.GA566736@jagdpanzerIV.localdomain> <20200720064303.GA2144711@elver.google.com>
 <20200720084102.GC463@jagdpanzerIV.localdomain> <CANpmjNPbnwmHyif79MaXdA478UGUKysB=u=HmtBiD10PPPTEcw@mail.gmail.com>
In-Reply-To: <CANpmjNPbnwmHyif79MaXdA478UGUKysB=u=HmtBiD10PPPTEcw@mail.gmail.com>
From:   Dmitry Vyukov <dvyukov@google.com>
Date:   Mon, 20 Jul 2020 12:01:51 +0200
Message-ID: <CACT4Y+ZjpFYKosvGNPPWYyfZaAh6+Gieh4zKH2JzdVmkhPCg6Q@mail.gmail.com>
Subject: Re: [PATCH v5 4/4] printk: use the lockless ringbuffer
To:     Marco Elver <elver@google.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        John Ogness <john.ogness@linutronix.de>,
        Petr Mladek <pmladek@suse.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrea Parri <parri.andrea@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Paul McKenney <paulmck@kernel.org>, kexec@lists.infradead.org,
        LKML <linux-kernel@vger.kernel.org>,
        Andy Shevchenko <andy.shevchenko@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 20, 2020 at 11:41 AM Marco Elver <elver@google.com> wrote:
>
> On Mon, 20 Jul 2020 at 10:41, Sergey Senozhatsky
> <sergey.senozhatsky@gmail.com> wrote:
> >
> > On (20/07/20 08:43), Marco Elver wrote:
> > > On Sun, Jul 19, 2020 at 12:43PM +0900, Sergey Senozhatsky wrote:
> > >
> > > As I said, a number of debugging tools use them to format reports to be
> > > more readable (visually separate title and report body, and separate
> > > parts of the report). Also, such reports are often parsed by CI systems,
> > > and by changing the reports, these CI systems may break. But those are
> > > just the usecases I'm acutely aware of
> >
> > Can you give example of such CI systems? // that's a real question //
>
> None of ours should break; I agree the CI system is brittle if it
> relies on newlines. Parsed and displayed reports are changing, however
> -- what irks me is now all the reports sent to the LKML look ugly.
>
> Some random KASAN reports (just compare formatting):
> next (ugly): https://lore.kernel.org/lkml/000000000000c87b7305aadb6dba@google.com/
> mainline (normal):
> https://lore.kernel.org/lkml/000000000000f4ef6a05aa92ec6c@google.com/
>
> The same problem exists with lockdep reports, KCSAN reports, ... If
> newline-printks to insert blank lines are now banned, what are we to
> do? Send dozens of patches to switch everyone to printk(" \n")? Or
> some better suggestion? I cannot yet see how that is an improvement.
> (And if the behaviour is not reverted, please document the new
> behaviour.)
>
> That also doesn't yet address the ~400 other newline-printk users, and
> somebody needs to do the due diligence to understand if it's just a
> flush, or an intentional blank line.


Empty lines improve readability of long crash reports significantly.
New lines in sanitizer reports originated in Go race reports 7 years
ago and then spread to user-space ASAN/MSAN/TSAN b/c that was an
improvement and then were specifically added to kernel sanitizers.
This is even more important now that we have up to 5 stacks in KASAN
reports.
Please keep them.

Also having lots of printk("\n") sprinkled in kernel code and turning
them into no-op separately does not look like the right solution.
These printk("\n") are confusing and add clutter. A better solution
would be to remove these printk("\n") from the code. But this also
naturally allows selective removal. Say, keeping for sanitizers and
some other cases, but removing some that are not useful.