All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Organov <sorganov@gmail.com>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org,
	Russell King <linux@armlinux.org.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: Serial console and interrupts latency.
Date: Fri, 27 Mar 2020 16:58:33 +0300	[thread overview]
Message-ID: <87ftdtzypi.fsf@osv.gnss.ru> (raw)
In-Reply-To: <aa0174f8-7cef-46c9-4164-605191393abd@suse.cz> (Jiri Slaby's message of "Fri, 27 Mar 2020 14:13:12 +0100")

Jiri Slaby <jslaby@suse.cz> writes:

> On 24. 03. 20, 10:04, Sergey Organov wrote:
>> Hello,
>> 
>> [Extended CC list to try to get some attention]
>> 
>> I was investigating random serial overruns on my embedded board and
>> figured it strongly correlates with serial output (to another serial
>> port) from kernel printk() calls, that forced me to dig into the kernel
>> sources, and now I'm very confused.
>> 
>> I'm reading drivers/tty/serial/8250/8250_port.c, and
>> serial8250_console_write() function in particular (being on tty-next
>> branch).
>> 
>> What I see is that it locks interrupts
>> 
>> 3141:		spin_lock_irqsave(&port->lock, flags);
>> 
>> and then calls wait_for_xmitr() both indirectly here:
>> 
>> 3159:	uart_console_write(port, s, count, serial8250_console_putchar);
>> 
>> and then directly as well:
>> 
>> 3165:	wait_for_xmitr(up, BOTH_EMPTY);
>> 
>> before re-enabling interrupts at:
>> 
>> 3179:		spin_unlock_irqrestore(&port->lock, flags);
>> 
>> Now, wait_for_xmitr(), even according to comments, could busy-wait for
>> up to 10+1000 milliseconds, and in this case this huge delay will happen
>> at interrupts disabled?
>> 
>> Does it mean any serial console output out of printk() could cause 10
>> milliseconds or even 1 second interrupts latency? Somehow I can't
>> believe it.
>> 
>> What do I miss?
>
> 1 second _timeout_ is for flow-control-enabled consoles.

Yeah, sure. So it does mean interrupts could be disabled for up to 1
second, on already up and running system. Too bad.

Actually, I use 8250 only as a reference implementation, my actual
chip is handled by imx.c, and the latter even has no timeouts on this
path, so apparently may block (the entire kernel) indefinitely!

> 10 ms is _timeout_ for a character. With slow 9600 baud console, sending
> one character takes 0.8 ms. With 115200, it is 70 us.

70us of disabled interrupts is a huge number, and for FIFO-enabled chips
the estimate should be multiplied by FIFO size (say, x16) that brings us
close to 1ms even on 115200, right?

Anyway, it must cause receiving overruns on another port running at
higher or the same baud rate and no DMA, sooner or later, as it does
for me.

>
> If you send one line (80 chars), it is really 66 and 5.5 ms, respectively.
>
> So yes, serial consoles can slow down the boot and add latency. Use
> faster speeds or faster devices for consoles, if you mind. And do not
> enable flow control. Serial is serial.

I don't care about slowing-down boot. I care about huge interrupt
latency on up and running system, causing loss of characters  (overruns)
on other serial ports.

To be sure, it is this code that works on already running system as
well, not only on boot-time, right? Or is my system somehow
misconfigured?

I'm confused as this seems to be a major issue and nobody but me seems
to care or to suffer from it, and I can't figure why.

For reference, I figured this locking was introduced by:

commit d8a5a8d7cc32e4474326e0ecc1b959063490efc9
Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
Date:   Tue May 2 16:04:29 2006 +0100

    [SERIAL] 8250: add locking to console write function
    
    x86 SMP breaks as a result of the previous change, we have no real
    option other than to add locking to the 8250 console write function.
    If an oops is in progress, try to acquire the lock.  If we fail to
    do so, continue anyway.


> You can also try to eliminate the interrupts disablement, but that would
> be a bit tough task, IMO.

It seems like I need to, and yeah, it'd be a somewhat tough task indeed,
but then there is one simple question: why isn't console output handled
through usual buffer/ISR paths?

I have an idea to investigate the netconsole module to see how it does
similar job for networks. Hopefully it has no such problems and will
give some useful tips. 

Thanks,
-- Sergey

  parent reply	other threads:[~2020-03-27 13:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24  9:04 Serial console and interrupts latency Sergey Organov
2020-03-27 13:13 ` Jiri Slaby
2020-03-27 13:38   ` Russell King - ARM Linux admin
2020-03-27 14:03     ` Sergey Organov
2020-03-27 13:58   ` Sergey Organov [this message]
2020-03-27 23:24     ` Russell King - ARM Linux admin
2020-04-14  9:50       ` Sergey Organov
2020-04-14  9:56         ` Russell King - ARM Linux admin
2020-04-14 12:14           ` Sergey Organov
  -- strict thread matches above, loose matches on Subject: below --
2020-03-20 15:15 Sergey Organov

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=87ftdtzypi.fsf@osv.gnss.ru \
    --to=sorganov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    /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.