linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: Felix Hoffmann <f3lix.dev@gmx.de>
Cc: linux-can@vger.kernel.org,
	Robin van der Gracht <robin@protonic.nl>,
	kernel@pengutronix.de, Oliver Hartkopp <socketcan@hartkopp.net>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH can v2] can: j1939: avoid address-claim timer deadlock
Date: Tue, 4 Aug 2026 09:12:20 +0200	[thread overview]
Message-ID: <anGQ1Bx5OsMV7nAu@pengutronix.de> (raw)
In-Reply-To: <20260803101503.21023-1-f3lix.dev@gmx.de>

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: stable@vger.kernel.org
> Assisted-by: Codex:GPT5.6-Sol
> Signed-off-by: Felix Hoffmann <f3lix.dev@gmx.de>

Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>

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 |

      parent reply	other threads:[~2026-08-04  7:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 13:42 [PATCH net] can: j1939: avoid address-claim timer deadlock Felix Hoffmann
2026-08-01 13:43 ` sashiko-bot
2026-08-03  6:18 ` Oleksij Rempel
2026-08-03 10:15 ` [PATCH can v2] " Felix Hoffmann
2026-08-03 10:43   ` sashiko-bot
2026-08-04  7:12   ` Oleksij Rempel [this message]

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=anGQ1Bx5OsMV7nAu@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=f3lix.dev@gmx.de \
    --cc=kernel@pengutronix.de \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=robin@protonic.nl \
    --cc=socketcan@hartkopp.net \
    /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;
as well as URLs for NNTP newsgroup(s).