Re: [PATCH v3 1/4] serial: revert guards in uart_wait_modem_status()

[Jiri Slaby]

On 07. 09. 26, 8:44, Johan Hovold wrote:
> Mixing scope-based and regular cleanup is discouraged and
> uart_port_deref() used by uart_wait_modem_status() falls in the latter
> category.
>
> Revert the premature guard conversion in preparation for fixing a hangup
> race.

It's not premature. Just convert uart_port_ref() to guards too. Gotos suck.

> Fixes: 56609c050051 ("serial: serial_core: use guard()s")
> Cc: stable@xxxxxxxxxxxxxxx	# 6.18
> Cc: Jiri Slaby (SUSE) <jirislaby@xxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>   drivers/tty/serial/serial_core.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 95774b0f1484..1553bc6cbe7b 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1213,15 +1213,16 @@ static int uart_wait_modem_status(struct uart_state *state, unsigned long arg)
>   	uport = uart_port_ref(state);
>   	if (!uport)
>   		return -EIO;
> -	scoped_guard(uart_port_lock_irq, uport) {
> -		memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
> -		uart_enable_ms(uport);
> -	}
> +	uart_port_lock_irq(uport);
> +	memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
> +	uart_enable_ms(uport);
> +	uart_port_unlock_irq(uport);
>   
>   	add_wait_queue(&port->delta_msr_wait, &wait);
>   	for (;;) {
> -		scoped_guard(uart_port_lock_irq, uport)
> -			memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
> +		uart_port_lock_irq(uport);
> +		memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
> +		uart_port_unlock_irq(uport);
>   
>   		set_current_state(TASK_INTERRUPTIBLE);
>   


--
js
suse labs