[v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

[PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

Posted by Fushuai Wang 1 month, 2 weeks ago

From: Fushuai Wang <wangfushuai@baidu.com>

When two PnP devices map to the same physical port, the serial8250 driver
removes and re-registers the console structure for the same port.

During re-registration, the console structure still has CON_PRINTBUFFER set
from the initial registration, which causes console_init_seq() to set
console->seq to syslog_seq. This results in re-printing the entire
system log buffer, which may lead to RCU stall on slow serial consoles.

Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
log printing.

Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
---
V1->V2: Add Fixes tag
previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u

Please ignore previous email if you received it before. There is something wrong with my email client.

 drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index a428e88938eb..01b14392d9f7 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 {
 	struct uart_8250_port *uart;
 	int ret;
+	bool was_removed = false;
 
 	if (up->port.uartclk == 0)
 		return -EINVAL;
@@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 	if (uart->port.type == PORT_8250_CIR)
 		return -ENODEV;
 
-	if (uart->port.dev)
+	if (uart->port.dev) {
 		uart_remove_one_port(&serial8250_reg, &uart->port);
+		was_removed = true;
+	}
 
 	uart->port.ctrl_id	= up->port.ctrl_id;
 	uart->port.port_id	= up->port.port_id;
@@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 					&uart->capabilities);
 
 		serial8250_apply_quirks(uart);
+
+		if (was_removed && uart_console(&uart->port))
+			uart->port.cons->flags &= ~CON_PRINTBUFFER;
+
 		ret = uart_add_one_port(&serial8250_reg,
 					&uart->port);
 		if (ret)
-- 
2.36.1

Re: [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

Posted by Greg KH 1 month ago

On Tue, Apr 28, 2026 at 05:03:49PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
> 
> When two PnP devices map to the same physical port, the serial8250 driver
> removes and re-registers the console structure for the same port.
> 
> During re-registration, the console structure still has CON_PRINTBUFFER set
> from the initial registration, which causes console_init_seq() to set
> console->seq to syslog_seq. This results in re-printing the entire
> system log buffer, which may lead to RCU stall on slow serial consoles.
> 
> Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
> log printing.
> 
> Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> ---
> V1->V2: Add Fixes tag
> previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u
> 
> Please ignore previous email if you received it before. There is something wrong with my email client.
> 
>  drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index a428e88938eb..01b14392d9f7 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>  {
>  	struct uart_8250_port *uart;
>  	int ret;
> +	bool was_removed = false;
>  
>  	if (up->port.uartclk == 0)
>  		return -EINVAL;
> @@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>  	if (uart->port.type == PORT_8250_CIR)
>  		return -ENODEV;
>  
> -	if (uart->port.dev)
> +	if (uart->port.dev) {
>  		uart_remove_one_port(&serial8250_reg, &uart->port);
> +		was_removed = true;
> +	}
>  
>  	uart->port.ctrl_id	= up->port.ctrl_id;
>  	uart->port.port_id	= up->port.port_id;
> @@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>  					&uart->capabilities);
>  
>  		serial8250_apply_quirks(uart);
> +
> +		if (was_removed && uart_console(&uart->port))
> +			uart->port.cons->flags &= ~CON_PRINTBUFFER;

Why not set the flag up above when you remove the port?  Why down here?

thanks,

greg k-h

Re: [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

Posted by Fushuai Wang 1 month ago

>> From: Fushuai Wang <wangfushuai@baidu.com>
>> 
>> When two PnP devices map to the same physical port, the serial8250 driver
>> removes and re-registers the console structure for the same port.
>> 
>> During re-registration, the console structure still has CON_PRINTBUFFER set
>> from the initial registration, which causes console_init_seq() to set
>> console->seq to syslog_seq. This results in re-printing the entire
>> system log buffer, which may lead to RCU stall on slow serial consoles.
>> 
>> Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
>> log printing.
>> 
>> Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
>> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
>> ---
>> V1->V2: Add Fixes tag
>> previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u
>> 
>> Please ignore previous email if you received it before. There is something wrong with my email client.
>> 
>>  drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index a428e88938eb..01b14392d9f7 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>>  {
>>  	struct uart_8250_port *uart;
>>  	int ret;
>> +	bool was_removed = false;
>>  
>>  	if (up->port.uartclk == 0)
>>  		return -EINVAL;
>> @@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>>  	if (uart->port.type == PORT_8250_CIR)
>>  		return -ENODEV;
>>  
>> -	if (uart->port.dev)
>> +	if (uart->port.dev) {
>>  		uart_remove_one_port(&serial8250_reg, &uart->port);
>> +		was_removed = true;
>> +	}
>>  
>>  	uart->port.ctrl_id	= up->port.ctrl_id;
>>  	uart->port.port_id	= up->port.port_id;
>> @@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>>  					&uart->capabilities);
>>  
>>  		serial8250_apply_quirks(uart);
>> +
>> +		if (was_removed && uart_console(&uart->port))
>> +			uart->port.cons->flags &= ~CON_PRINTBUFFER;
> 
> Why not set the flag up above when you remove the port?  Why down here?
> 
> thanks,
> 
> greg k-h

Hi, Greg

I just felt it's cleaner to clear it only when re-registration happens.
Do you think there is any problem with doing it right after the removal?

-- 
Regards,
WANG

Re: [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

Posted by Greg KH 3 weeks, 4 days ago

On Fri, May 15, 2026 at 03:44:02PM +0800, Fushuai Wang wrote:
> >> From: Fushuai Wang <wangfushuai@baidu.com>
> >> 
> >> When two PnP devices map to the same physical port, the serial8250 driver
> >> removes and re-registers the console structure for the same port.
> >> 
> >> During re-registration, the console structure still has CON_PRINTBUFFER set
> >> from the initial registration, which causes console_init_seq() to set
> >> console->seq to syslog_seq. This results in re-printing the entire
> >> system log buffer, which may lead to RCU stall on slow serial consoles.
> >> 
> >> Clear CON_PRINTBUFFER when re-registering a port to prevent duplicate
> >> log printing.
> >> 
> >> Fixes: 835d844d1a28 ("8250_pnp: do pnp probe before legacy probe")
> >> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> >> ---
> >> V1->V2: Add Fixes tag
> >> previous discussion: https://lore.kernel.org/all/20260416092917.27301-1-fushuai.wang@linux.dev/T/#u
> >> 
> >> Please ignore previous email if you received it before. There is something wrong with my email client.
> >> 
> >>  drivers/tty/serial/8250/8250_core.c | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> >> index a428e88938eb..01b14392d9f7 100644
> >> --- a/drivers/tty/serial/8250/8250_core.c
> >> +++ b/drivers/tty/serial/8250/8250_core.c
> >> @@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
> >>  {
> >>  	struct uart_8250_port *uart;
> >>  	int ret;
> >> +	bool was_removed = false;
> >>  
> >>  	if (up->port.uartclk == 0)
> >>  		return -EINVAL;
> >> @@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
> >>  	if (uart->port.type == PORT_8250_CIR)
> >>  		return -ENODEV;
> >>  
> >> -	if (uart->port.dev)
> >> +	if (uart->port.dev) {
> >>  		uart_remove_one_port(&serial8250_reg, &uart->port);
> >> +		was_removed = true;
> >> +	}
> >>  
> >>  	uart->port.ctrl_id	= up->port.ctrl_id;
> >>  	uart->port.port_id	= up->port.port_id;
> >> @@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
> >>  					&uart->capabilities);
> >>  
> >>  		serial8250_apply_quirks(uart);
> >> +
> >> +		if (was_removed && uart_console(&uart->port))
> >> +			uart->port.cons->flags &= ~CON_PRINTBUFFER;
> > 
> > Why not set the flag up above when you remove the port?  Why down here?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi, Greg
> 
> I just felt it's cleaner to clear it only when re-registration happens.
> Do you think there is any problem with doing it right after the removal?

If you do that, no flag is needed.

Re: [PATCH v2] serial: 8250: Clear CON_PRINTBUFFER on port re-registration

Posted by Fushuai Wang 3 weeks, 4 days ago

>> >> --- a/drivers/tty/serial/8250/8250_core.c
>> >> +++ b/drivers/tty/serial/8250/8250_core.c
>> >> @@ -694,6 +694,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>> >>  {
>> >>  	struct uart_8250_port *uart;
>> >>  	int ret;
>> >> +	bool was_removed = false;
>> >>  
>>> >>  	if (up->port.uartclk == 0)
>> >>  		return -EINVAL;
>> >> @@ -716,8 +717,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>> >>  	if (uart->port.type == PORT_8250_CIR)
>> >>  		return -ENODEV;
>> >>  
>> >> -	if (uart->port.dev)
>> >> +	if (uart->port.dev) {
>> >>  		uart_remove_one_port(&serial8250_reg, &uart->port);
>> >> +		was_removed = true;
>> >> +	}
>> >>  
>> >>  	uart->port.ctrl_id	= up->port.ctrl_id;
>> >>  	uart->port.port_id	= up->port.port_id;
>> >> @@ -819,6 +822,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
>> >>  					&uart->capabilities);
>> >>  
>> >>  		serial8250_apply_quirks(uart);
>> >> +
>> >> +		if (was_removed && uart_console(&uart->port))
>> >> +			uart->port.cons->flags &= ~CON_PRINTBUFFER;
>> > 
>> > Why not set the flag up above when you remove the port?  Why down here?
>> > 
>> > thanks,
>> > 
>> > greg k-h
>> 
>> Hi, Greg
>> 
>> I just felt it's cleaner to clear it only when re-registration happens.
>> Do you think there is any problem with doing it right after the removal?
>
> If you do that, no flag is needed.

OK, I will send a v3 shortly.

-- 
Regards,
WANG