All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	John Ogness <john.ogness@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Michal Hocko <mhocko@kernel.org>
Subject: Re: [RFC PATCH] printk: Introduce "store now but print later" prefix.
Date: Wed, 20 Mar 2019 16:04:16 +0100	[thread overview]
Message-ID: <20190320150416.4otf2343wmjfbw2c@pathway.suse.cz> (raw)
In-Reply-To: <0fc95fee-8b6e-5ff0-9a91-9e0ea66028f3@i-love.sakura.ne.jp>

On Thu 2019-03-07 03:24:25, Tetsuo Handa wrote:
> On 2019/03/06 19:04, Petr Mladek wrote:
> >> I'm suggesting to use a non-async printk() for $trailer_text_line line. I think
> >> that changing all printk() called from out_of_memory() to async is doable, if
> >> the caller of out_of_memory() wakes up a dedicated kthread described below.
> > 
> > This is error prone. The trailing printk is not guaranteed:
> > 
> >     + It might get lost by code refactoring.
> 
> Caller uses this async printk() with their own risk. This concern is same with
> "buffering into on-stack 'char buf[80]' using snprintf() and forgetting to flush
> it using printk()".

AFAIK, all pr_cont() users are self-contained. Most of them print all
pieces in the same function. Some of them print the part of the lines
using some helper scripts but these are defined in the same source
file, usually right above the caller.

> > 
> >     + People would miss that it is needed when async printk is used
> >       in a shared function, e.g. dump_stack().
> 
> I'm planning to allow async printk() for shared functions like dump_stack()
> because there will be some $trailer_text_line line after such shared functions.
> (By the way, for that reason, I expect that we won't do a tree-wide KERN_DEFAULT
> removal so that callers can pass KERN_DEFAULT_ASYNC as an function argument to
> such shared functions.)

Now, you are going to spread related lines all over the kernel sources.


> > But I would really like to avoid even more printk variants.
> > They have many problems:
> > 
> >     + Only a handful of people would know how to use them right.
> 
> The caller of async printk() knows when/how to use async printk().
> 
> > 
> >     + They are hard to maintain. Any incompatible printk() in the call
> >       chain might break the intention.
> 
> There is no incompatible printk(). Only when to wake up a thread which
> writes to consoles differs.

By incompatible I mean the following:

   + Adding any synchronous printk() into a chain of async printk's
     will break the chain. I mean that the consoles will get handled
     by the new normal printk() before the original author wanted.

   + Also the wanted normal printk() might get removed from some
     reason (by mistake). Then all the previous async lines will
     not get reliably flushed.

Both situations are bugs that might get fixed. But the API
is just too error prone from my POV.

We are repeating the same arguments by different words all over
again. I explained why the API is not acceptable for me and
suggested other approaches. This is my last mail about the API.

Best Regards,
Petr

  parent reply	other threads:[~2019-03-20 15:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-23  4:42 [RFC PATCH] printk: Introduce "store now but print later" prefix Tetsuo Handa
2019-03-04  3:22 ` Sergey Senozhatsky
2019-03-04 11:40   ` Tetsuo Handa
2019-03-04 12:09     ` Sergey Senozhatsky
2019-03-04 14:23     ` Petr Mladek
2019-03-04 14:37       ` Sergey Senozhatsky
2019-03-05  1:23       ` Tetsuo Handa
2019-03-05  7:52         ` Sergey Senozhatsky
2019-03-05 12:57         ` Michal Hocko
2019-03-06 10:04         ` Petr Mladek
2019-03-06 14:27           ` Sergey Senozhatsky
2019-03-06 18:24           ` Tetsuo Handa
2019-03-15 10:49             ` Tetsuo Handa
2019-03-20 15:04             ` Petr Mladek [this message]
2019-03-20 15:25             ` ratelimit API: was: " Petr Mladek
2019-03-21  8:13               ` Tetsuo Handa
2019-03-21  8:49                 ` Michal Hocko

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=20190320150416.4otf2343wmjfbw2c@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=torvalds@linux-foundation.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 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.