Skip to content

Commit f4a4d12

Browse files
Max Staudtmarckleinebudde
authored andcommitted
can: can327: flush TX_work on ldisc .close()
Additionally, remove it from .ndo_stop(). This ensures that the worker is not called after being freed, and that the UART TX queue remains active to send final commands when the netdev is stopped. Thanks to Jiri Slaby for finding this in slcan: https://lore.kernel.org/linux-can/[email protected]/ A variant of this patch for slcan, with the flush in .ndo_stop() still present, has been tested successfully on physical hardware: https://bugzilla.suse.com/show_bug.cgi?id=1205597 Fixes: 43da2f0 ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters") Cc: "Jiri Slaby (SUSE)" <[email protected]> Cc: Max Staudt <[email protected]> Cc: Wolfgang Grandegger <[email protected]> Cc: Marc Kleine-Budde <[email protected]> Cc: "David S. Miller" <[email protected]> Cc: Eric Dumazet <[email protected]> Cc: Jakub Kicinski <[email protected]> Cc: Paolo Abeni <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Signed-off-by: Max Staudt <[email protected]> Link: https://lore.kernel.org/all/[email protected] Cc: [email protected] Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent fb855e9 commit f4a4d12

File tree

1 file changed

+10
-7
lines changed

1 file changed

+10
-7
lines changed

drivers/net/can/can327.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -796,9 +796,9 @@ static int can327_netdev_close(struct net_device *dev)
796796

797797
netif_stop_queue(dev);
798798

799-
/* Give UART one final chance to flush. */
800-
clear_bit(TTY_DO_WRITE_WAKEUP, &elm->tty->flags);
801-
flush_work(&elm->tx_work);
799+
/* We don't flush the UART TX queue here, as we want final stop
800+
* commands (like the above dummy char) to be flushed out.
801+
*/
802802

803803
can_rx_offload_disable(&elm->offload);
804804
elm->can.state = CAN_STATE_STOPPED;
@@ -1069,12 +1069,15 @@ static void can327_ldisc_close(struct tty_struct *tty)
10691069
{
10701070
struct can327 *elm = (struct can327 *)tty->disc_data;
10711071

1072-
/* unregister_netdev() calls .ndo_stop() so we don't have to.
1073-
* Our .ndo_stop() also flushes the TTY write wakeup handler,
1074-
* so we can safely set elm->tty = NULL after this.
1075-
*/
1072+
/* unregister_netdev() calls .ndo_stop() so we don't have to. */
10761073
unregister_candev(elm->dev);
10771074

1075+
/* Give UART one final chance to flush.
1076+
* No need to clear TTY_DO_WRITE_WAKEUP since .write_wakeup() is
1077+
* serialised against .close() and will not be called once we return.
1078+
*/
1079+
flush_work(&elm->tx_work);
1080+
10781081
/* Mark channel as dead */
10791082
spin_lock_bh(&elm->lock);
10801083
tty->disc_data = NULL;

0 commit comments

Comments
 (0)