From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
Vincent Mailhol <mailhol@kernel.org>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
Claudiu Beznea <claudiu.beznea@tuxon.dev>,
Dario Binacchi <dario.binacchi@amarulasolutions.com>,
Max Staudt <max@enpas.org>,
Markus Schneider-Pargmann <msp@baylibre.com>,
Heiko Stuebner <heiko@sntech.de>,
Manivannan Sadhasivam <mani@kernel.org>,
Thomas Kopp <thomas.kopp@microchip.com>,
Ming Yu <tmyu0@nuvoton.com>
Cc: kernel@pengutronix.de, linux-can@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
NXP S32 Linux Team <s32@nxp.com>,
imx@lists.linux.dev, Haibo Chen <haibo.chen@nxp.com>,
Enric Balletbo <eballetb@redhat.com>
Subject: Re: [PATCH v6 0/3] can: rx-offload: add a per-IRQ receive context
Date: Tue, 29 Sep 2026 11:21:21 +0300 [thread overview]
Message-ID: <dd896eed-5bdc-48b8-ab6c-4a65fba14a88@oss.nxp.com> (raw)
In-Reply-To: <20260925144558.2909639-1-ciprianmarian.costea@oss.nxp.com>
On 9/25/2026 5:45 PM, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>
> The rx-offload IRQ handler fills skb_irq_queue without a lock and the
> finish helpers splice it into skb_queue under skb_queue.lock. This only
> works with a single producer. flexcan requests the same handler on every
> IRQ line: the mailbox line, the bus off and error lines
> (FLEXCAN_QUIRK_NR_IRQ_3) and a second mailbox line
> (FLEXCAN_QUIRK_SECONDARY_MB_IRQ). On NXP S32G2 all four are used, so the
> handlers can run at the same time on different CPUs and corrupt the queue.
>
> As Marc suggested, each IRQ line now gets its own receive context, similar
> to NAPI. struct can_rx_offload_irq holds skb_irq_queue, skb_queue_len_max
> and the mailbox range, skb_queue and napi stay in struct can_rx_offload.
> With a single context the finish helpers still use
> skb_queue_splice_tail_init(), with more than one the skbs are sorted into
> skb_queue.
>
> Patch 1 is an at91_can fix, independent of the rest. Patch 2 adds the
> per-IRQ context and converts all rx-offload users. Patch 3 gives each
> flexcan IRQ line its own context.
>
> All lines still read the whole mailbox range, so two of them can read the
> same mailbox. This series does not change that, the range split is part
> of the flexcan multi-IRQ patches along with the S32N79 FlexCAN support.
>
> Testing on S32G2 is in progress, I will follow up in this thread.
>
Finished testing on the S32G274A-RDB2 board between can0 and can1 on the
same bus, CAN FD + classic CAN, with the IRQ lines pinned to different
CPUs. The rx-offload queue works fine with the series applied.
As mentioned in patch 3, the handlers still process all mailboxes and
the ESR on every line. With the lines on different CPUs, a frame can
rarely be received twice and a state change can be reported twice. This
is a pre-existing issue and will be fixed in a follow-up flexcan series
that splits the handling per IRQ line, along with the S32N79-RDB FlexCAN
support [1] (needs a new version once the current rx-offload patchset
gets accepted).
[1]
https://lore.kernel.org/all/20260831143449.12828-1-ciprianmarian.costea@oss.nxp.com/
> Changes since v5:
>
> - Replaced the per-CPU skb_irq_queue, which does not work in preemptible
> context, with a per-IRQ receive context, as suggested by Marc Kleine-Budde.
> - Added patch 3 with the flexcan conversion. The bus off and error lines get
> their own context too, not only the mailbox lines.
> - Moved the at91_can fix to its own patch.
> - Dropped the gs_usb can_rx_offload_add_manual() return value check, the
> NULL pointer dereference it guarded against went away with the per-CPU
> allocation.
> - Dropped Haibo's Reviewed-by from the rx-offload patch, since it was
> rewritten.
> - Added Assisted-by tags.
>
> Changes since v4:
> - rx-offload: expand the comment above the for_each_possible_cpu() loop
> in can_rx_offload_threaded_irq_finish() to add the single-producer
> assumption (IRQ requested with IRQF_ONESHOT / handler non-reentrant).
> Suggested by Haibo Chen.
> - rx-offload: add Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
>
> Changes since v3:
>
> - In gs_usb driver, check the can_rx_offload_add_manual() return value,
> the same NULL-deref the per-CPU change exposes.
>
> Changes since v2:
>
> - at91_can: also add can_rx_offload_del() on the register_candev() error
> path and check the can_rx_offload_add_timestamp() return value.
>
> Changes since v1:
>
> - The enqueue helpers used this_cpu_ptr() without disabling preemption.
> All four enqueue helpers now use get_cpu_ptr()/put_cpu_ptr().
> - Guard can_rx_offload_del() against skb_irq_queue == NULL.
> - Fix 'at91_can' memory leak by adding missing 'can_rx_offload_del'.
>
> Ciprian Marian Costea (3):
> can: at91_can: release the rx-offload on teardown
> can: rx-offload: add a per-IRQ receive context
> can: flexcan: use one rx-offload context per IRQ line
>
> drivers/net/can/at91_can.c | 28 ++-
> drivers/net/can/bxcan.c | 14 +-
> drivers/net/can/can327.c | 8 +-
> drivers/net/can/dev/rx-offload.c | 196 ++++++++++++------
> drivers/net/can/flexcan/flexcan-core.c | 109 ++++++++--
> drivers/net/can/flexcan/flexcan-ethtool.c | 4 +-
> drivers/net/can/flexcan/flexcan.h | 4 +
> drivers/net/can/m_can/m_can.c | 7 +-
> drivers/net/can/m_can/m_can.h | 1 +
> .../net/can/rockchip/rockchip_canfd-core.c | 9 +-
> drivers/net/can/rockchip/rockchip_canfd-rx.c | 2 +-
> drivers/net/can/rockchip/rockchip_canfd-tx.c | 2 +-
> drivers/net/can/rockchip/rockchip_canfd.h | 1 +
> .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 13 +-
> drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c | 2 +-
> drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c | 2 +-
> drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 1 +
> drivers/net/can/ti_hecc.c | 18 +-
> drivers/net/can/usb/gs_usb.c | 23 +-
> drivers/net/can/usb/nct6694_canfd.c | 20 +-
> include/linux/can/rx-offload.h | 48 ++++-
> 21 files changed, 349 insertions(+), 163 deletions(-)
>
prev parent reply other threads:[~2026-09-29 8:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-25 14:45 Ciprian Costea
2026-09-25 14:45 ` [PATCH v6 1/3] can: at91_can: release the rx-offload on teardown Ciprian Costea
2026-09-26 14:53 ` Max Staudt
2026-09-25 14:45 ` [PATCH v6 2/3] can: rx-offload: add a per-IRQ receive context Ciprian Costea
2026-09-26 15:26 ` Max Staudt
2026-09-28 7:39 ` Ciprian Marian Costea
2026-09-25 14:45 ` [PATCH v6 3/3] can: flexcan: use one rx-offload context per IRQ line Ciprian Costea
2026-09-29 8:21 ` Ciprian Marian Costea [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=dd896eed-5bdc-48b8-ab6c-4a65fba14a88@oss.nxp.com \
--to=ciprianmarian.costea@oss.nxp.com \
--cc=alexandre.belloni@bootlin.com \
--cc=claudiu.beznea@tuxon.dev \
--cc=dario.binacchi@amarulasolutions.com \
--cc=eballetb@redhat.com \
--cc=haibo.chen@nxp.com \
--cc=heiko@sntech.de \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--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=mani@kernel.org \
--cc=max@enpas.org \
--cc=mkl@pengutronix.de \
--cc=msp@baylibre.com \
--cc=nicolas.ferre@microchip.com \
--cc=s32@nxp.com \
--cc=thomas.kopp@microchip.com \
--cc=tmyu0@nuvoton.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®