Re: [PATCH] tty: timbuart: kill tasklet after freeing IRQ

[Greg KH]

On Mon, Oct 19, 2026 at 09:31:55PM +0800, LI xiao wrote:
> The timbuart driver calls tasklet_kill() before uart_remove_one_port().
> An interrupt can schedule the tasklet again after tasklet_kill()
> returns but before timbuart_shutdown() frees the IRQ. The subsequent
> free_irq() synchronizes the IRQ handler, but does not drain a tasklet
> that the handler has already queued.
> 
> Move tasklet_kill() to timbuart_shutdown(), immediately after
> free_irq(), so that any tasklet queued by the IRQ handler is drained
> before uart_remove_one_port() releases the port resources.
> 
> I tested the intended synchronization ordering with a KASAN-enabled
> kernel in a QEMU guest using a test module. The module queued
> the tasklet, waited for the simulated IRQ handler to exit, called
> tasklet_kill(), and then freed the UART object. The tasklet completed
> without a KASAN report or scheduling warning.
> 
> This was a controlled test of the synchronization ordering. It did
> not exercise timbuart's actual IRQ handler or Timberdale UART hardware,
> which is not modeled by QEMU.

Ah, that might be tricky, I think you need to test this on real hardware
as you are changing the logic here.

> Signed-off-by: LI Xiao <lixiao0@xxxxxxxxxx>

No fixes: tag?  Cc: stable?


> 
> ---
>  drivers/tty/serial/timbuart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
> index 7a0326e71..c561ca702 100644
> --- a/drivers/tty/serial/timbuart.c
> +++ b/drivers/tty/serial/timbuart.c
> @@ -254,6 +254,7 @@ static void timbuart_shutdown(struct uart_port *port)
>  		container_of(port, struct timbuart_port, port);
>  	dev_dbg(port->dev, "%s\n", __func__);
>  	free_irq(port->irq, uart);
> +	tasklet_kill(&uart->tasklet);
>  	iowrite32(0, port->membase + TIMBUART_IER);
>  
>  	timbuart_flush_buffer(port);

Are you sure you can flush after the tasklet is killed?  Nothing
requires it to still be running at this point in time?

And why not stop it before the irq is freed as that's what it is
handling?  Note, I could be totally wrong, I'm just guessing here...

thanks,

greg k-h