Re: [PATCH can v2] can: j1939: avoid address-claim timer deadlock

Oleksij Rempel <[email protected]>
Newsgroups org.kernel.vger.linux-can,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hi Felix,

Please send next patch stand alone, not in replay to the previous patch
or discussion.

On Mon, Aug 03, 2026 at 12:15:03PM +0200, Felix Hoffmann wrote:
> j1939_ac_process() holds priv->lock while synchronously canceling an
> ECU's address-claim hrtimer. The timer callback takes the same lock. If the
> callback starts on another CPU after the receive path takes the lock, the
> callback waits for priv->lock while hrtimer_cancel() waits for the callback
> to finish. This deadlocks both CPUs and makes the system unresponsive.
> 
> Replace the callback's blocking lock acquisition with write_trylock(). If
> the lock is busy, move the expiry forward and restart the timer. This lets
> a concurrent hrtimer_cancel() observe the callback finish and remove the
> requeued timer. Without a cancellation, address mapping is retried shortly.
> 
> The failed write trylock does not necessarily mean that the address-claim
> writer owns priv->lock. Receive processing also takes its read side for
> other frames, and an active reader prevents write_trylock() from
> succeeding. Document both the address-claim writer and receive-path readers
> as possible sources of contention.
> 
> Name the 1 ms retry interval J1939_ECU_TIMER_RETRY_DELAY_MS instead of
> using a literal in the callback. Track consecutive failures per ECU, reset
> the counter when its timer starts, and emit a warning guarded by
> net_ratelimit() on the fifth retry. Five is a pragmatic threshold for
> detecting unusual delay; it is not specified by J1939.
> 
> The timer uses HRTIMER_MODE_REL_SOFT, so its callback runs from
> HRTIMER_SOFTIRQ with bottom halves already disabled. write_trylock() does
> not change bottom-half state and must therefore pair with write_unlock() on
> success. The write_lock_bh()/write_unlock_bh() pair is unnecessary in this
> context.
> 
> Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
> Cc: [email protected]
> Assisted-by: Codex:GPT5.6-Sol
> Signed-off-by: Felix Hoffmann <[email protected]>

Acked-by: Oleksij Rempel <[email protected]>

Thank you!

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.