linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: John Ogness <john.ogness@linutronix.de>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Michal Hocko <mhocko@suse.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] printk: Enough to disable preemption in printk deferred context
Date: Wed, 19 Apr 2023 14:03:29 +0200	[thread overview]
Message-ID: <ZD_Yka6MJ_-HOKpj@alley> (raw)
In-Reply-To: <87r0sg5jin.fsf@jogness.linutronix.de>

On Wed 2023-04-19 11:11:52, John Ogness wrote:
> On 2023-04-19, Petr Mladek <pmladek@suse.com> wrote:
> > it is safe to interrupt one writer now. The preemption still
> > has to be disabled because the deferred context is CPU specific.
> 
> Really it is enough to disable migration.

True. But it gets too far to my taste. As you describe below.
It affects all printk's on the CPU.

Sigh, even the enabled intrrupts might be questionable. For example,
when the iterrupt is from a watchdog and want's to report a stall.

> We need to keep an eye on the usage of this function. By allowing
> interrupts and preemption, it means that other printk's on that CPU will
> also be deferred if the context interrupted within the deferred block.

A solution would be to make this more clear in the comment.
Something like:

/*
 * The printk_deferred_enter/exit macros are available only as a hack.
 * They define a per-CPU context where all printk console printing is
 * deferred because it might cause a deadlock otherwise.
 *
 * The API user is responsible for calling the corresponding enter/exit
 * pair on the same CPU. It is highly recommended to use them only in
 * a context with interrupts disabled. Otherwise, other unrelated
 * printk() calls might be deferred when they interrupt/preempt
 * the deferred code section.
 */

Another solution would be to stay on the "safe" side and keep the
comment as is or even enforce disabling interrupts by the API.

I would personally just improve the comment. It is good to describe
the situation correctly. We could always add restrictions when
there are problems in practice.

Best Regards,
Petr


  reply	other threads:[~2023-04-19 12:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-19  7:42 [PATCH] printk: Enough to disable preemption in printk deferred context Petr Mladek
2023-04-19  8:23 ` Sergey Senozhatsky
2023-04-19  8:23 ` Michal Hocko
2023-04-19 10:31   ` Petr Mladek
2023-04-19  9:05 ` John Ogness
2023-04-19 12:03   ` Petr Mladek [this message]
2023-04-19 12:14     ` Michal Hocko
2023-04-19 13:45     ` John Ogness

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=ZD_Yka6MJ_-HOKpj@alley \
    --to=pmladek@suse.com \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    /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;
as well as URLs for NNTP newsgroup(s).