mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Cunhao Lu" <1579567540@qq.com>
To: netdev-bot+sashiko <netdev-bot+sashiko@kernel.org>
Cc: mkl <mkl@pengutronix.de>, mailhol <mailhol@kernel.org>,
	heiko <heiko@sntech.de>, davem <davem@davemloft.net>,
	wg <wg@grandegger.com>, socketcan <socketcan@hartkopp.net>,
	kernel <kernel@pengutronix.de>,
	linux-can <linux-can@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-rockchip <linux-rockchip@lists.infradead.org>,
	stable <stable@vger.kernel.org>, kuba <kuba@kernel.org>
Subject: Re: [PATCH net v3 1/5] can: dev: prevent bus-off recovery when the interface is not running
Date: Fri, 16 Oct 2026 13:38:15 +0800	[thread overview]
Message-ID: <tencent_F6326E09D62380013188767E4F4D66345808@qq.com> (raw)
In-Reply-To: <179212694915.1406898.5096495212984871051@kernel.org>

Hi,

> [Severity: Medium]
> Is this check enough to prevent the failure described in the commit
> message, "potentially re-enabling hardware while the driver is tearing it
> down"?

No, netif_running() alone does not serialize restart with ndo_stop(). A
restart that has already passed the check can still overlap with hardware
teardown. The commit message explicitly documents this limitation and the
need for drivers to cancel restart work before tearing down resources.

For Rockchip, the series already calls close_candev_cancel_restart_sync()
at the start of rkcanfd_stop(). This cancels pending work and waits for any
restart already in progress before disabling interrupts, freeing the IRQ
or stopping the controller. The final close_candev() remains after IRQ
teardown for final cancellation and echo cleanup.

As discussed earlier, this series limits the driver-side fix to Rockchip.
You are right that sja1000 and flexcan still have the window you describe.
Those pre-existing races need separate driver fixes; the core checks do
not resolve them. For Rockchip, the series uses the early-cancellation
approach you suggest rather than adding RTNL locking to the worker.

> [Severity: Low]
> This isn't a bug, but the comment above can_bus_off() still describes the
> old condition:
>
> /* CAN bus-off
>  *
>  * This functions should be called when the device goes bus-off to
>  * tell the netif layer that no more packets can be sent or received.
>  * If enabled, a timer is started to trigger bus-off recovery.
>  */
>
> Could it mention that recovery is now only scheduled when the interface
> is running?

Agreed. I will update the comment to refer to delayed work and state that
recovery is scheduled only when automatic recovery is enabled and the
interface is running. I will also clarify the running-interface condition
in the restart-ms documentation. If v5 is accepted, I will send a
separate documentation cleanup patch later.

Regards,
Cunhao Lu

  reply	other threads:[~2026-10-16  5:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 13:24 [PATCH net v3 0/5] can: fix bus-off restart and device close races Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 1/5] can: dev: prevent bus-off recovery when the interface is not running Cunhao Lu via B4 Relay
2026-10-16  5:02   ` netdev-bot+sashiko
2026-10-16  5:38     ` Cunhao Lu [this message]
2026-10-14 13:24 ` [PATCH net v3 2/5] can: dev: add close_candev_cancel_restart_sync() Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 3/5] can: rockchip_canfd: cancel bus-off restart before closing the device Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 4/5] can: rockchip_canfd: prevent timestamp work from rearming after bus-off Cunhao Lu via B4 Relay
2026-10-14 13:24 ` [PATCH net v3 5/5] can: rockchip_canfd: serialize TX with bus-off and restart Cunhao Lu via B4 Relay
2026-10-16  5:02   ` netdev-bot+sashiko
2026-10-15  2:30 ` [PATCH net v3 0/5] can: fix bus-off restart and device close races Cunhao Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_F6326E09D62380013188767E4F4D66345808@qq.com \
    --to=1579567540@qq.com \
    --cc=davem@davemloft.net \
    --cc=heiko@sntech.de \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mailhol@kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev-bot+sashiko@kernel.org \
    --cc=socketcan@hartkopp.net \
    --cc=stable@vger.kernel.org \
    --cc=wg@grandegger.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®