public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	"Qixuan.Wu" <qixuan.wu@linux.alibaba.com>,
	linux-kernel-owner <linux-kernel-owner@vger.kernel.org>,
	Petr Mladek <pmladek@suse.com>, Jan Kara <jack@suse.cz>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	"chenggang.qin" <chenggang.qin@linux.alibaba.com>,
	caijingxian <caijingxian@linux.alibaba.com>,
	"yuanliang.wyl" <yuanliang.wyl@alibaba-inc.com>,
	Tejun Heo <tj@kernel.org>
Subject: Re: Would you help to tell why async printk solution was not taken to upstream kernel ?
Date: Tue, 6 Mar 2018 11:43:58 +0900	[thread overview]
Message-ID: <20180306024358.GC6713@jagdpanzerIV> (raw)
In-Reply-To: <20180306015222.GA6713@jagdpanzerIV>

One more thing

On (03/06/18 10:52), Sergey Senozhatsky wrote:
[..]
> > If you know the baud rate, logbuf size * console throughput is actually
> > trivial to calculate.

It's trivial when your setup is trivial. In a less trivial case if you
set watchdog threshold based on "logbuf size * console throughput" then
things are still too bad.

So this is what a typical printk over serial console looks like

printk()
 console_unlock()
  for (;;) {
   local_irq_save()
    call_console_drivers()
     foo_console_write()
      spin_lock_irqsave(&port->lock, flags);
      uart_console_write(port, s, count, foo_console_putchar);
      spin_unlock_irqrestore(&port->lock, flags);
   local_irq_restore()
  }

Notice that call_console_drivers->foo_console_write spins on
port->lock every time it wants to print out a logbuf line.
Why does it do this?

In short, because of printf(). Yes, printk() may depend on printf().

printf()
 n_tty_write()
  uart_write()
   uart_port_lock(state, flags)                  // spin_lock_irqsave(&uport->lock, flags)
    memcpy(circ->buf + circ->head, buf, c);
   uart_port_unlock(port, flags)                 // spin_unlock_irqrestore(&port->lock, flags);

Now, printf() messages stored in uart circ buffer must be printed
to the console. And this is where console's IRQ handler jumps in.

A typical IRQ handler does something like this

static irqreturn_t foo_console_irq_handler(...)
{
	spin_lock(&port->lock);
	rx_chars(port, status);
	tx_chars(port, status);
	spin_unlock(&port->lock);
}

Where tx_chars() usually does something like this

	while (...) {
		write_char(port, xmit->buf[xmit->tail]);
		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
		if (uart_circ_empty(xmit))
			break;
	}

Some drivers flush all pending chars, some drivers limit the number
of TX chars to some number, e.g. 512.

But in any case, printk() -> call_console_drivers() -> foo_console_write()
must spin on port->lock as long as foo_console_irq_handler() has chars to
TX / RX.

Thus, if you have O(logbuf) of kernel messages, and O(circ->buf) of user
space messages, then printk() will spend O(logbuf) + O(circ->buf) + O(RX).

So the watchdog threshold value based purely on O(logbuf) (printing to
_all_ of the consoles) will not always work.

	-ss

  reply	other threads:[~2018-03-06  2:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1eb584e2-a479-46dd-8a25-820da7a34e85.qixuan.wu@linux.alibaba.com>
2018-03-04 13:01 ` Would you help to tell why async printk solution was not taken to upstream kernel ? Sergey Senozhatsky
2018-03-04 15:08   ` Qixuan.Wu
2018-03-04 15:43     ` Steven Rostedt
2018-03-05  2:14       ` Sergey Senozhatsky
2018-03-05 20:45         ` Steven Rostedt
2018-03-06  2:00           ` Sergey Senozhatsky
2018-03-06  2:47             ` Steven Rostedt
2018-03-06  2:53               ` Sergey Senozhatsky
2018-03-06  3:16                 ` Steven Rostedt
2018-03-06  8:10                   ` Sergey Senozhatsky
2018-03-05 20:58         ` Steven Rostedt
2018-03-06  1:52           ` Sergey Senozhatsky
2018-03-06  2:43             ` Sergey Senozhatsky [this message]
2018-03-06  3:18               ` Steven Rostedt
2018-03-05  6:56       ` Qixuan.Wu
2018-03-05 13:29         ` 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=20180306024358.GC6713@jagdpanzerIV \
    --to=sergey.senozhatsky.work@gmail.com \
    --cc=caijingxian@linux.alibaba.com \
    --cc=chenggang.qin@linux.alibaba.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel-owner@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=qixuan.wu@linux.alibaba.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tj@kernel.org \
    --cc=yuanliang.wyl@alibaba-inc.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