All of lore.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 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.