Received-SPF: pass (google.com: domain of linux-kernel+bounces-192663-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 28 May 2024 17:29:33 +0200
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: xu xin <xu.xin.sc@gmail.com>
Cc: xu.xin16@zte.com.cn, john.ogness@linutronix.de,
	linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org,
	liu.chun2@zte.com.cn, rostedt@goodmis.org, si.hao@zte.com.cn,
	yang.yang29@zte.com.cn, zhang.yunkai@zte.com.cn
Subject: Re: [PATCH  5.10-rt] printk: ignore that console preempted by
 irq/softirq
Message-ID: <20240528152933.ns7uyFSD@linutronix.de>
References: <20240523235537910_yxfGMbTcSOmMkcfuK2d8@zte.com.cn>
 <20240528064003.776618-1-xu.xin16@zte.com.cn>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20240528064003.776618-1-xu.xin16@zte.com.cn>

On 2024-05-28 06:40:03 [+0000], xu xin wrote:
> > This does not apply.
> > There is `may_sleep' set earlier.
> >=20
> > There is no console_lock() around for each=E2=80=A6
> >=20
>=20
> Sorry, I don't get it.
>=20
> To clarify it again, this patch aims to solve the useless waiting of pr_f=
lush
> when the console is preempted by the current irq/softirq. This has nothin=
g to
> do with might_sleep().

There is a `may_sleep` variable set earlier. Couldn't that be re-used?

> > The other question is which kernel started enforcing might_sleep() for
> > pr_flush(). This should be applied to all kernel or none so we don't
> > have random behaviour across kernels (5.4 yes, 5.10 no, 5.15 yes).
> >
>=20
> Sorry, my understanding is that pr_flush didn't start enforcing might_sle=
ep().
> This patch can apply to 5.10 and 5.15 where the problem exist.

Starting with v6.1-RT there is a might_sleep() at the beginning of
pr_flush(). This means that atomic context can not be used anymore.
Therefore is patch needs only to be applied to 5.10 and 5.15 as you
said. I just didn't see the information the following kernels (>=3D6.1)
already had that might_sleep() check and the previous (<5.4) lack
pr_flush().

> > This is a delay of max 1 sec during bug() and panic(). Not sure how
> > "critical" this is=E2=80=A6
>=20
> In some industrial control scenarios, bugs and warnings containning a
> pr_flush delay of 1 sec is very critical to the upper services.
>=20
> Especiall for watchdog timeout(< 2s), just WARN can easily lead to system=
 reset,
> which is unacceptible.

Now this would be important piece of information for the changelog in
terms of _why_ we do this.
You don't have an atomic console, do you? Because if this BUG/ WARNING is
printed via the atomic console then it could also raise your 1sec limit.

Sebastian