public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: John Ogness <john.ogness@linutronix.de>
Cc: Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Stephen Boyd <swboyd@chromium.org>,
	Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH next v4 1/2] lib/dump_stack: move cpu lock to printk.c
Date: Fri, 18 Jun 2021 12:31:52 -0400	[thread overview]
Message-ID: <20210618123152.35a992ea@oasis.local.home> (raw)
In-Reply-To: <877dirb4t2.fsf@jogness.linutronix.de>

On Fri, 18 Jun 2021 17:01:37 +0206
John Ogness <john.ogness@linutronix.de> wrote:

> Since the cpu lock is also taken in NMI context (for example, via
> nmi_cpu_backtrace()/dump_stack()), the main concerns are:
> 
> 1. locks that are taken by a CPU that is holding the cpu lock
> 
> 2. NMI contexts that take any type of lock
> 
> (Actually, #2 is just a special case of #1 where an NMI interrupted a
> task that was holding the cpu lock.)
> 
> For early_printk() the early USB devices look to be a
> problem. early_xdbc_write() will take a spinlock. Assuming the
> early_console was also registered as a normal console (via "keep") we
> could end up in the following deadlock between the normal console and
> early_printk() writes:

My use case of earlyprintk is to avoid all the crud in printk, and
just have something to print to the serial console. USB early printk is
not my use case, as that adds even more crud.

Thus, I don't care about this being an issue with USB early printk ;-)


> 
>     CPU0                          CPU1
>     ----                          ----
>     early_printk()                console->write()
>       cpu_lock()                    spinlock()
>       early_console->write()      *NMI*
>         spinlock()                cpu_lock()
> 
> The upcoming atomic console work addresses this by implementing a new
> write_atomic() callback that is lockless (and SMP-safe) or aware of the
> cpu lock to avoid dead locks such as above.
> 
> AFAICT, the USB devices (CONFIG_EARLY_PRINTK_USB) are the only
> early_printk() candidates that use locking. So for all other
> early_printk() implementations I think your suggestion would work fine.
> 
> Although, in general, early_printk() is not SMP-safe. So I'm not sure
> how much safety we need to include at this point.
> 

Right. I say we ignore the issue with usb earlyprintk. The only time I
can see that even useful, is for issues that are happening before
printk is initialized, and all you have for a console is the usb for
early printk, and the above scenario shouldn't be an issue. But for
places that would utilize early printk for later on in the boot process
(and normal activity), any console that takes locks would be useless.

-- Steve

  reply	other threads:[~2021-06-18 16:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-17  9:50 [PATCH next v4 0/2] introduce printk cpu lock John Ogness
2021-06-17  9:50 ` [PATCH next v4 1/2] lib/dump_stack: move cpu lock to printk.c John Ogness
2021-06-17 13:32   ` Steven Rostedt
2021-06-18 14:47     ` Petr Mladek
2021-06-18 16:25       ` Steven Rostedt
2021-06-19  0:22         ` John Ogness
2021-06-18 14:55     ` John Ogness
2021-06-18 16:31       ` Steven Rostedt [this message]
2021-06-17  9:50 ` [PATCH next v4 2/2] printk: fix cpu lock ordering John Ogness
2021-06-17 11:23 ` [PATCH next v4 0/2] introduce printk cpu lock Petr Mladek
2021-06-17 11:28   ` Stephen Rothwell
2021-06-17 11:39 ` Sergey Senozhatsky

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=20210618123152.35a992ea@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=bristot@redhat.com \
    --cc=glider@google.com \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=senozhatsky@chromium.org \
    --cc=sfr@canb.auug.org.au \
    --cc=swboyd@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox