Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Dec 2022 11:32:46 +0100
From:   Petr Mladek <pmladek@suse.com>
To:     Thomas =?iso-8859-1?Q?Wei=DFschuh?= <linux@weissschuh.net>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>,
        linux-pm@vger.kernel.org,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        Andy Whitcroft <apw@canonical.com>,
        Joe Perches <joe@perches.com>, linux-kernel@vger.kernel.org,
        Steven Rostedt <rostedt@goodmis.org>,
        Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>
Subject: Re: [PATCH v2 0/3] printk: introduce new macros pr_<level>_cont()
Message-ID: <Y4iCzsAeEgaBZ4o4@alley>
References: <20221125190948.2062-1-linux@weissschuh.net>
 <CAJZ5v0i8pm1vxQeQu4GJqvf=rinU9dO2gswsLseyEt3E2CgbtA@mail.gmail.com>
 <d31b4a2b-fc6b-4084-9cac-ced83a37a8ad@t-8ch.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <d31b4a2b-fc6b-4084-9cac-ced83a37a8ad@t-8ch.de>
Precedence: bulk

On Thu 2022-12-01 00:37:15, Thomas Wei?schuh wrote:
> On 2022-11-30 18:57+0100, Rafael J. Wysocki wrote:
> > On Fri, Nov 25, 2022 at 8:10 PM Thomas Wei?schuh <linux@weissschuh.net> wrote:
> >>
> >> This series adds new printk wrapper macros pr_<level>_cont().
> >> These create continuation messages with an explicit level.
> >>
> >> Explicit levels are useful when a continuation message is split from its main
> >> message. Without the explicit level KERN_DEFAULT ("warn" by default) is used
> >> which can lead to stray partial log messages when filtering by level.
> >>
> >> Also checkpatch is modified to recommend the new macros over plain pr_cont().
> >>
> >> Lastly the new macros are used in kernel/power/process.c as this file uses
> >> continuation messages during system suspend-resume which creates a high
> >> likelyhood of interspersed messages.
> > 
> > Well, if process.c is the only problematic piece of code in this
> > respect, I'm not sure if adding the new infrastructure for its benefit
> > alone is worth it, because it can very well do without pr_cont() at
> > all.
> 
> In general all usages of pr_cont() are problematic.
> Any continuation can be split from its main message, leading to misleved
> continuations.

In most cases this happens "only" when a message from another CPU
or interrupt context is printed in parallel.

> process.c is just the one that I noticed reliably hitting this problem on my
> machine.

The situation in process.c was even worse. The error message was
printed in the middle of the to-be-continued message. As a result,
the loglevel of the pr_cont() part was always (reliably) broken
when the error message was printed.


> > Please see the patch below (compiled only, sorry for gmail-induced
> > white space damage).  I'll submit it properly later if it works for
> > everyone.
> 
> The patch looks fine to me and getting rid of usages of pr_cont() seems to be
> the better aproach where it is possible.

I agree. It is always better to avoid pr_cont() when possible.

> Petr: do you still want me to submit the new macros even if it is not used
> directly anymore?

Good question. In general, new API should not be added if there is
no user. So, I would prefer to do not add the API if the problem
will be fixed without it.

Best Regards,
Petr