All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: John Ogness <john.ogness@linutronix.de>,
	linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2] printk: Skip console drivers on PREEMPT_RT.
Date: Mon, 25 Jul 2022 14:30:04 +0200	[thread overview]
Message-ID: <Yt6MzEEFfpyTBIIj@alley> (raw)
In-Reply-To: <YtrYdXWGb0NQLKNA@linutronix.de>

On Fri 2022-07-22 19:03:49, Sebastian Andrzej Siewior wrote:
> On 2022-07-22 14:39:44 [+0200], Petr Mladek wrote:
> > On Thu 2022-07-21 08:50:38, Sebastian Andrzej Siewior wrote:
> > > printk might be invoked in a context with disabled interrupts and or
> > > preemption and additionally disables interrupts before it invokes the
> > > console drivers. This is behaviour is not compatible with PREEMPT_RT.
> > 
> > Maybe I do not understand it correctly.
> 
> > Or is is somehow specific to the console drivers called from printk()
> > directly?
> 
> You can't acquire a sleeping lock with disabled interrupts and or

This is what I have missed. It might be obvious for people living by RT
kernel. But spinlocks do not sleep in normal kernel. So I did not get
that the spinlocks are the culprit. Please, make it more obvious
in the commit message, for example:

--- cut ---
Console drivers use spinlocks that might sleep in PREEMPT_RT. As a
result they must not be called with interrupts enabled...
--- cut ---

> > > Disable console printing until the return of atomic consoles and the
> > > printing thread. This allows to retrieve the log buffer from user space
> > > which is not possible by disable printk.
> > 
> > I guess that this is for RT tree because the kthreads and the atomic
> > consoles are still not in the mainline.
> 
> I would like to have this applied to the v5.20 upstream tree and then
> revoked once the missing bits have been  merged. Based on what I see,
> there shouldn't be any road blocks.

Huh, I do not think that it is a good idea. There are neither atomic
consoles nor printk kthreads in upstream. The patch effectively
completely disables the consoles in PREEMPT_RT. All consoles
will be _empty all the time_.

Also the consoles were never tested with interrupts enabled. I am
pretty sure that interrupts has to be disabled in some locations,
for example, when sending some data sequences on the ports. Are we
sure that they are always explicitly disabled inside the drivers?
This is one reason why we reverted the console kthreads in 5.19-rc4.
AFAIK, John is doing some inspection of all drivers.

That said, I am going to leave the decision on John. I am not sure
what are the expectations of RT users in mainline.

From my point, this patch does not make much sense. IMHO, it will
not make mainline usable with PREEMPT_RT. Any serious RT user will
need to revert it and apply a better printk solution from
the out-of-tree RT patchset.

Best Regards,
Petr

  reply	other threads:[~2022-07-25 12:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-20 15:48 [PATCH] printk: Skip console drivers on PREEMPT_RT Sebastian Andrzej Siewior
2022-07-20 16:26 ` John Ogness
2022-07-20 16:35   ` Sebastian Andrzej Siewior
2022-07-20 17:50     ` John Ogness
2022-07-21  6:50       ` [PATCH v2] " Sebastian Andrzej Siewior
2022-07-22 12:39         ` Petr Mladek
2022-07-22 17:03           ` Sebastian Andrzej Siewior
2022-07-25 12:30             ` Petr Mladek [this message]
2022-07-25 12:51               ` John Ogness
2022-07-25 13:55               ` Sebastian Andrzej Siewior
2022-07-25 14:24                 ` Petr Mladek
2022-07-25 15:16                   ` [PATCH v3] " Sebastian Andrzej Siewior
2022-07-26  7:39                     ` Petr Mladek
2022-07-26  7:57                     ` John Ogness
2022-07-26 13:11                       ` 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 \
    --in-reply-to=Yt6MzEEFfpyTBIIj@alley \
    --to=pmladek@suse.com \
    --cc=bigeasy@linutronix.de \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tglx@linutronix.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.