| Message ID | 20220316143646.13301-1-wander@redhat.com |
|---|---|
| Headers | show |
| Series | tty/8250: Use fifo in 8250 console driver | expand |
From: Wander Lairson Costa > Sent: 16 March 2022 14:37 > > Note: I am using a small test app + driver located at [0] for the > problem description. serco is a driver whose write function dispatches > to the serial controller. sertest is a user-mode app that writes n bytes > to the serial console using the serco driver. > > While investigating a bug in the RHEL kernel, I noticed that the serial > console throughput is way below the configured speed of 115200 bps in > a HP Proliant DL380 Gen9. I was expecting something above 10KB/s, but > I got 2.5KB/s. > > $ time ./sertest -n 2500 /tmp/serco > > real 0m0.997s > user 0m0.000s > sys 0m0.997s > > With the help of the function tracer, I then noticed the serial > controller was taking around 410us seconds to dispatch one single byte: Did you verify the baud rate? Or is there some horrid serial redirection going on. It is even possible there is a bios smm interrupt chugging through on another cpu core. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, Mar 16, 2022 at 11:36:39AM -0300, Wander Lairson Costa wrote: > This version fixes the bugs reported in version v3. The first patch > is the same patch of v3 as is. The following commits fix the issues in the > original patch. For details, please check the commit log of each patch. > > I tested these patches in the following systems: > > * IBM X3550 M3 > * HP ProLiant DL380 Gen9 > * HP ProLiant BL480c G1 > * Dell PowerEdge R910 > * Cisco UCSC-C220-M3S > > I cc everybody that reported problems with the previous version of this > patch so they can retest and confirm their systems work flawlessly. I have got this only message and I don't see any good changelog what has been done between v3 and v4. > Wander Lairson Costa (5): > serial/8250: Use fifo in 8250 console driver > serial/8250: Use the cache value of the FCR register > serial/8250: Use tx_loadsz as the transmitter fifo size > serial/8250: exclude BCM283x from console_fifo_write > serial/8250: Only use fifo after the port is initialized in > console_write
On Wed, Mar 16, 2022 at 06:13:19PM +0200, Andy Shevchenko wrote: > On Wed, Mar 16, 2022 at 11:36:39AM -0300, Wander Lairson Costa wrote: > > This version fixes the bugs reported in version v3. The first patch > > is the same patch of v3 as is. The following commits fix the issues in the > > original patch. For details, please check the commit log of each patch. > > > > I tested these patches in the following systems: > > > > * IBM X3550 M3 > > * HP ProLiant DL380 Gen9 > > * HP ProLiant BL480c G1 > > * Dell PowerEdge R910 > > * Cisco UCSC-C220-M3S > > > > I cc everybody that reported problems with the previous version of this > > patch so they can retest and confirm their systems work flawlessly. > > I have got this only message and I don't see any good changelog what has > been done between v3 and v4. > > > Wander Lairson Costa (5): > > serial/8250: Use fifo in 8250 console driver > > serial/8250: Use the cache value of the FCR register > > serial/8250: Use tx_loadsz as the transmitter fifo size > > serial/8250: exclude BCM283x from console_fifo_write > > serial/8250: Only use fifo after the port is initialized in > > console_write If you are going to (re-)send a new version, please Cc to Ilpo as well.
On Wed, Mar 16, 2022 at 1:15 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Mar 16, 2022 at 11:36:39AM -0300, Wander Lairson Costa wrote: > > This version fixes the bugs reported in version v3. The first patch > > is the same patch of v3 as is. The following commits fix the issues in the > > original patch. For details, please check the commit log of each patch. > > > > I tested these patches in the following systems: > > > > * IBM X3550 M3 > > * HP ProLiant DL380 Gen9 > > * HP ProLiant BL480c G1 > > * Dell PowerEdge R910 > > * Cisco UCSC-C220-M3S > > > > I cc everybody that reported problems with the previous version of this > > patch so they can retest and confirm their systems work flawlessly. > > I have got this only message and I don't see any good changelog what has > been done between v3 and v4. > Weird, the patches were sent [1,2,3,4,5] and reached out my inbox. [1] https://lore.kernel.org/all/20220316143646.13301-2-wander@redhat.com/ [2] https://lore.kernel.org/all/20220316143646.13301-3-wander@redhat.com/ [3] https://lore.kernel.org/all/20220316143646.13301-4-wander@redhat.com/ [4] https://lore.kernel.org/all/20220316143646.13301-5-wander@redhat.com/ [5] https://lore.kernel.org/all/20220316143646.13301-6-wander@redhat.com/ [snip]
On Thu, Mar 17, 2022 at 4:06 AM Jiri Slaby <jirislaby@kernel.org> wrote: > > On 16. 03. 22, 15:36, Wander Lairson Costa wrote: > > The serial driver set the value of uart_8250_port.fcr in the function > > serial8250_config_port, but only writes the value to the controller > > register later in the initalization code. > > > > That opens a small window in which is not safe to use the fifo for > > console write. > > > > Make sure the port is initialized correctly before reading the FCR > > cached value. > > > > Unfortunately, I lost track of who originally reported the issue. If > > s/he is reading this, please speak up so I can give you the due credit. > > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com> > > --- > > drivers/tty/serial/8250/8250_port.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c > > index 4acf620be241..7e2227161555 100644 > > --- a/drivers/tty/serial/8250/8250_port.c > > +++ b/drivers/tty/serial/8250/8250_port.c > > @@ -3416,6 +3416,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s, > > !(up->capabilities & UART_CAP_MINI) && > > up->tx_loadsz > 1 && > > (up->fcr & UART_FCR_ENABLE_FIFO) && > > + test_bit(TTY_PORT_INITIALIZED, &port->state->port.iflags) && > > Cannot be port->state be NULL sometimes here? > IIUC, state is assigned at early port registration in uart_add_one_port(), so this function wouldn't be called when state is NULL. But I think it causes no harm to add an extra check. Thanks! > > /* > > * After we put a data in the FIFO, the controller will send > > * it regardless of the CTS state. Therefore, only use fifo > > > -- > js > suse labs >
On Wed, Mar 16, 2022 at 12:27 PM David Laight <David.Laight@aculab.com> wrote: > > From: Wander Lairson Costa > > Sent: 16 March 2022 14:37 > > > > Note: I am using a small test app + driver located at [0] for the > > problem description. serco is a driver whose write function dispatches > > to the serial controller. sertest is a user-mode app that writes n bytes > > to the serial console using the serco driver. > > > > While investigating a bug in the RHEL kernel, I noticed that the serial > > console throughput is way below the configured speed of 115200 bps in > > a HP Proliant DL380 Gen9. I was expecting something above 10KB/s, but > > I got 2.5KB/s. > > > > $ time ./sertest -n 2500 /tmp/serco > > > > real 0m0.997s > > user 0m0.000s > > sys 0m0.997s > > > > With the help of the function tracer, I then noticed the serial > > controller was taking around 410us seconds to dispatch one single byte: > > Did you verify the baud rate? > Yes. > Or is there some horrid serial redirection going on. > It is even possible there is a bios smm interrupt > chugging through on another cpu core. > I would be surprised if that were the case because I see the problem even in low system activity. Someone in a previous patch-revision theorized this might be due to a bad serial controller emulator implemented in hardware (or something in this line of thought).
On Wed, 16 Mar 2022 11:36:39 -0300 Wander Lairson Costa <wander@redhat.com> wrote: > I cc everybody that reported problems with the previous version of this > patch so they can retest and confirm their systems work flawlessly. I didn't do an extensive test, but I quickly applied them and did not see any issues with the two machines that had problems with your first commits. But I'll do a more extensive testing on your v5 before I give a tested-by. Thanks, -- Steve
This version fixes the bugs reported in version v3. The first patch is the same patch of v3 as is. The following commits fix the issues in the original patch. For details, please check the commit log of each patch. I tested these patches in the following systems: * IBM X3550 M3 * HP ProLiant DL380 Gen9 * HP ProLiant BL480c G1 * Dell PowerEdge R910 * Cisco UCSC-C220-M3S I cc everybody that reported problems with the previous version of this patch so they can retest and confirm their systems work flawlessly. Wander Lairson Costa (5): serial/8250: Use fifo in 8250 console driver serial/8250: Use the cache value of the FCR register serial/8250: Use tx_loadsz as the transmitter fifo size serial/8250: exclude BCM283x from console_fifo_write serial/8250: Only use fifo after the port is initialized in console_write drivers/tty/serial/8250/8250_port.c | 67 ++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 6 deletions(-)