linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: sergey.senozhatsky.work@gmail.com
Cc: linux-kernel@vger.kernel.org, mingo@redhat.com,
	peterz@infradead.org, pmladek@suse.com,
	sergey.senozhatsky@gmail.com, rostedt@goodmis.org
Subject: Re: [PATCH] printk: Add best-effort printk() buffering.
Date: Mon, 8 May 2017 22:05:13 +0900	[thread overview]
Message-ID: <201705082205.DAE12471.FOHJSFOQFtOVML@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20170508070500.GA522@jagdpanzerIV.localdomain>

Sergey Senozhatsky wrote:
> sorry for the delay.

No problem.

> 
> On (04/30/17 22:54), Tetsuo Handa wrote:
> > Sometimes we want to printk() multiple lines in a group without being
> > disturbed by concurrent printk() from interrupts and/or other threads.
> > For example, mixed printk() output of multiple thread's dump makes it
> > hard to interpret.
> 
> hm, it's very close to what printk-safe does [and printk-nmi, of course].
> the difference is that buffered-printk does not disable local IRQs,
> unlike printk-safe, which has to do it by design. so the question is,
> can buffered-printk impose atomicity requirements? it seems that it can
> (am I wrong?). and, if so, then can we use printk-safe instead? we can
> add a new printk_buffered_begin/printk_buffered_end API, for example,
> (or enter/exit) for that purpose, that would set a buffered-printk
> `printk_context' bit so we can flush buffers in a "special way", not via IRQ
> work, and may be avoid message loss (printk-safe buffers are bigger in size
> than proposed PAGE_SIZE buffers).

printk_buffered_begin()/printk_buffered_end() corresponds to
get_printk_buffer()/put_printk_buffer().
printk_context() distinguishes atomic contexts.
flush_printk_buffer() flushes from non-NMI context.

What does atomicity requirements mean?

> 
> 
> > This patch introduces fixed-sized statically allocated buffers for
> > buffering printk() output for each thread/context in best effort
> > (i.e. up to PAGE_SIZE bytes, up to 16 concurrent printk() users).
> 
> hm, 16 is rather random, it's too much for UP and probably not enough for
> a 240 CPUs system. for the time being there are 3 buffered-printk users
> (as far as I can see), but who knows how more will be added in the future.
> each CPU can have overlapping printks from process, IRQ and NMI contexts.
> for NMI we use printk-nmi buffers, so it's out of the list; but, in general,
> *it seems* that we better depend on the number of CPUs the system has.
> which, once again, returns us back to printk-safe...
> 
> thoughts?

I can make 16 a CONFIG_ option. Would you read 201705031521.EIJ39594.MFtOVOHSFLFOJQ@I-love.SAKURA.ne.jp ?
But as long as actually writing to console devices is slow, message loss
is inevitable no matter how big buffer is used. Rather, I'd expect an API
which allows printk() users in schedulable context (e.g. kmallocwd and/or
warn_alloc() for reporting allocation stalls) to wait until written to
console devices. That will more likely to reduce message loss.

> > +	while (1) {
> > +		char *text = ptr->buf;
> > +		unsigned int text_len = ptr->used;
> > +		char *cp = memchr(text, '\n', text_len);
> > +		char c;
> 
> what guarantees that there'll always be a terminating newline?

Nothing guarantees. Why need such a guarantee?

  reply	other threads:[~2017-05-08 13:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-30 13:54 [PATCH] printk: Add best-effort printk() buffering Tetsuo Handa
2017-04-30 16:11 ` Joe Perches
2017-05-03  6:21   ` Tetsuo Handa
2017-05-03  9:30     ` Joe Perches
2017-05-08  7:05 ` Sergey Senozhatsky
2017-05-08 13:05   ` Tetsuo Handa [this message]
2017-05-09  1:04     ` Sergey Senozhatsky
2017-05-09 11:41       ` Tetsuo Handa
2017-05-10  6:21         ` Sergey Senozhatsky
2017-05-10 11:27           ` Tetsuo Handa
2017-05-15 13:15             ` 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=201705082205.DAE12471.FOHJSFOQFtOVML@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    /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).