All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/4] pull-request: can 2022-10-27
@ 2022-10-27 11:43 Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion Marc Kleine-Budde
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-10-27 11:43 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello Jakub, hello David,

this is a pull request of 4 patches for net/master.

Anssi Hannula fixes the use of the completions in the kvaser_usb
driver.

Biju Das contributes 2 patches for the rcar_canfd driver. A IRQ storm
that can be triggered by high CAN bus load and channel specific IRQ
handlers are fixed.

Yang Yingliang fixes the j1939 transport protocol by moving a
kfree_skb() out of a spin_lock_irqsave protected section.

regards,
Marc

---

The following changes since commit e2badb4bd33abe13ddc35975bd7f7f8693955a4b:

  net: ethernet: ave: Fix MAC to be in charge of PHY PM (2022-10-26 20:21:34 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-6.1-20221027

for you to fetch changes up to c3c06c61890da80494bb196f75d89b791adda87f:

  can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb() (2022-10-27 13:34:23 +0200)

----------------------------------------------------------------
linux-can-fixes-for-6.1-20221027

----------------------------------------------------------------
Anssi Hannula (1):
      can: kvaser_usb: Fix possible completions during init_completion

Biju Das (2):
      can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive
      can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L

Marc Kleine-Budde (1):
      Merge patch series "R-Car CAN-FD fixes"

Yang Yingliang (1):
      can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb()

 drivers/net/can/rcar/rcar_canfd.c                 | 24 +++++++++++------------
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c |  4 ++--
 drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c  |  4 ++--
 net/can/j1939/transport.c                         |  4 +++-
 4 files changed, 18 insertions(+), 18 deletions(-)



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion
  2022-10-27 11:43 [PATCH net 0/4] pull-request: can 2022-10-27 Marc Kleine-Budde
@ 2022-10-27 11:43 ` Marc Kleine-Budde
  2022-10-27 17:40   ` patchwork-bot+netdevbpf
  2022-10-27 11:43 ` [PATCH net 2/4] can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive Marc Kleine-Budde
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-10-27 11:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Anssi Hannula, Jimmy Assarsson,
	stable, Marc Kleine-Budde

From: Anssi Hannula <anssi.hannula@bitwise.fi>

kvaser_usb uses completions to signal when a response event is received
for outgoing commands.

However, it uses init_completion() to reinitialize the start_comp and
stop_comp completions before sending the start/stop commands.

In case the device sends the corresponding response just before the
actual command is sent, complete() may be called concurrently with
init_completion() which is not safe.

This might be triggerable even with a properly functioning device by
stopping the interface (CMD_STOP_CHIP) just after it goes bus-off (which
also causes the driver to send CMD_STOP_CHIP when restart-ms is off),
but that was not tested.

Fix the issue by using reinit_completion() instead.

Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
Link: https://lore.kernel.org/all/20221010185237.319219-2-extja@kvaser.com
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 4 ++--
 drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index 7b52fda73d82..66f672ea631b 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1875,7 +1875,7 @@ static int kvaser_usb_hydra_start_chip(struct kvaser_usb_net_priv *priv)
 {
 	int err;
 
-	init_completion(&priv->start_comp);
+	reinit_completion(&priv->start_comp);
 
 	err = kvaser_usb_hydra_send_simple_cmd(priv->dev, CMD_START_CHIP_REQ,
 					       priv->channel);
@@ -1893,7 +1893,7 @@ static int kvaser_usb_hydra_stop_chip(struct kvaser_usb_net_priv *priv)
 {
 	int err;
 
-	init_completion(&priv->stop_comp);
+	reinit_completion(&priv->stop_comp);
 
 	/* Make sure we do not report invalid BUS_OFF from CMD_CHIP_STATE_EVENT
 	 * see comment in kvaser_usb_hydra_update_state()
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
index 50f2ac8319ff..19958037720f 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -1320,7 +1320,7 @@ static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
 {
 	int err;
 
-	init_completion(&priv->start_comp);
+	reinit_completion(&priv->start_comp);
 
 	err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_START_CHIP,
 					      priv->channel);
@@ -1338,7 +1338,7 @@ static int kvaser_usb_leaf_stop_chip(struct kvaser_usb_net_priv *priv)
 {
 	int err;
 
-	init_completion(&priv->stop_comp);
+	reinit_completion(&priv->stop_comp);
 
 	err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_STOP_CHIP,
 					      priv->channel);

base-commit: e2badb4bd33abe13ddc35975bd7f7f8693955a4b
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH net 2/4] can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive
  2022-10-27 11:43 [PATCH net 0/4] pull-request: can 2022-10-27 Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion Marc Kleine-Budde
@ 2022-10-27 11:43 ` Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 3/4] can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 4/4] can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb() Marc Kleine-Budde
  3 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-10-27 11:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Marc Kleine-Budde, stable

From: Biju Das <biju.das.jz@bp.renesas.com>

We are seeing an IRQ storm on the global receive IRQ line under heavy
CAN bus load conditions with both CAN channels enabled.

Conditions:

The global receive IRQ line is shared between can0 and can1, either of
the channels can trigger interrupt while the other channel's IRQ line
is disabled (RFIE).

When global a receive IRQ interrupt occurs, we mask the interrupt in
the IRQ handler. Clearing and unmasking of the interrupt is happening
in rx_poll(). There is a race condition where rx_poll() unmasks the
interrupt, but the next IRQ handler does not mask the IRQ due to
NAPIF_STATE_MISSED flag (e.g.: can0 RX FIFO interrupt is disabled and
can1 is triggering RX interrupt, the delay in rx_poll() processing
results in setting NAPIF_STATE_MISSED flag) leading to an IRQ storm.

This patch fixes the issue by checking IRQ active and enabled before
handling the IRQ on a particular channel.

Fixes: dd3bd23eb438 ("can: rcar_canfd: Add Renesas R-Car CAN FD driver")
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/all/20221025155657.1426948-2-biju.das.jz@bp.renesas.com
Cc: stable@vger.kernel.org
[mkl: adjust commit message]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 567620d215f8..ea828c1bd3a1 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1157,11 +1157,13 @@ static void rcar_canfd_handle_global_receive(struct rcar_canfd_global *gpriv, u3
 {
 	struct rcar_canfd_channel *priv = gpriv->ch[ch];
 	u32 ridx = ch + RCANFD_RFFIFO_IDX;
-	u32 sts;
+	u32 sts, cc;
 
 	/* Handle Rx interrupts */
 	sts = rcar_canfd_read(priv->base, RCANFD_RFSTS(gpriv, ridx));
-	if (likely(sts & RCANFD_RFSTS_RFIF)) {
+	cc = rcar_canfd_read(priv->base, RCANFD_RFCC(gpriv, ridx));
+	if (likely(sts & RCANFD_RFSTS_RFIF &&
+		   cc & RCANFD_RFCC_RFIE)) {
 		if (napi_schedule_prep(&priv->napi)) {
 			/* Disable Rx FIFO interrupts */
 			rcar_canfd_clear_bit(priv->base,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH net 3/4] can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L
  2022-10-27 11:43 [PATCH net 0/4] pull-request: can 2022-10-27 Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 2/4] can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive Marc Kleine-Budde
@ 2022-10-27 11:43 ` Marc Kleine-Budde
  2022-10-27 11:43 ` [PATCH net 4/4] can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb() Marc Kleine-Budde
  3 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-10-27 11:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, stable, Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

RZ/G2L has separate channel specific IRQs for transmit and error
interrupts. But the IRQ handler processes both channels, even if there
no interrupt occurred on one of the channels.

This patch fixes the issue by passing a channel specific context
parameter instead of global one for the IRQ register and the IRQ
handler, it just handles the channel which is triggered the interrupt.

Fixes: 76e9353a80e9 ("can: rcar_canfd: Add support for RZ/G2L family")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://lore.kernel.org/all/20221025155657.1426948-3-biju.das.jz@bp.renesas.com
Cc: stable@vger.kernel.org
[mkl: adjust commit message]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index ea828c1bd3a1..198da643ee6d 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1246,11 +1246,9 @@ static void rcar_canfd_handle_channel_tx(struct rcar_canfd_global *gpriv, u32 ch
 
 static irqreturn_t rcar_canfd_channel_tx_interrupt(int irq, void *dev_id)
 {
-	struct rcar_canfd_global *gpriv = dev_id;
-	u32 ch;
+	struct rcar_canfd_channel *priv = dev_id;
 
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
-		rcar_canfd_handle_channel_tx(gpriv, ch);
+	rcar_canfd_handle_channel_tx(priv->gpriv, priv->channel);
 
 	return IRQ_HANDLED;
 }
@@ -1278,11 +1276,9 @@ static void rcar_canfd_handle_channel_err(struct rcar_canfd_global *gpriv, u32 c
 
 static irqreturn_t rcar_canfd_channel_err_interrupt(int irq, void *dev_id)
 {
-	struct rcar_canfd_global *gpriv = dev_id;
-	u32 ch;
+	struct rcar_canfd_channel *priv = dev_id;
 
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
-		rcar_canfd_handle_channel_err(gpriv, ch);
+	rcar_canfd_handle_channel_err(priv->gpriv, priv->channel);
 
 	return IRQ_HANDLED;
 }
@@ -1723,6 +1719,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	priv->ndev = ndev;
 	priv->base = gpriv->base;
 	priv->channel = ch;
+	priv->gpriv = gpriv;
 	priv->can.clock.freq = fcan_freq;
 	dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
 
@@ -1751,7 +1748,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 		}
 		err = devm_request_irq(&pdev->dev, err_irq,
 				       rcar_canfd_channel_err_interrupt, 0,
-				       irq_name, gpriv);
+				       irq_name, priv);
 		if (err) {
 			dev_err(&pdev->dev, "devm_request_irq CH Err(%d) failed, error %d\n",
 				err_irq, err);
@@ -1765,7 +1762,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 		}
 		err = devm_request_irq(&pdev->dev, tx_irq,
 				       rcar_canfd_channel_tx_interrupt, 0,
-				       irq_name, gpriv);
+				       irq_name, priv);
 		if (err) {
 			dev_err(&pdev->dev, "devm_request_irq Tx (%d) failed, error %d\n",
 				tx_irq, err);
@@ -1791,7 +1788,6 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 
 	priv->can.do_set_mode = rcar_canfd_do_set_mode;
 	priv->can.do_get_berr_counter = rcar_canfd_get_berr_counter;
-	priv->gpriv = gpriv;
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 
 	netif_napi_add_weight(ndev, &priv->napi, rcar_canfd_rx_poll,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH net 4/4] can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb()
  2022-10-27 11:43 [PATCH net 0/4] pull-request: can 2022-10-27 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2022-10-27 11:43 ` [PATCH net 3/4] can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L Marc Kleine-Budde
@ 2022-10-27 11:43 ` Marc Kleine-Budde
  3 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-10-27 11:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Yang Yingliang, Oleksij Rempel,
	stable, Marc Kleine-Budde

From: Yang Yingliang <yangyingliang@huawei.com>

It is not allowed to call kfree_skb() from hardware interrupt context
or with interrupts being disabled. The skb is unlinked from the queue,
so it can be freed after spin_unlock_irqrestore().

Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/all/20221027091237.2290111-1-yangyingliang@huawei.com
Cc: stable@vger.kernel.org
[mkl: adjust subject]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/j1939/transport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index d7d86c944d76..55f29c9f9e08 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -342,10 +342,12 @@ static void j1939_session_skb_drop_old(struct j1939_session *session)
 		__skb_unlink(do_skb, &session->skb_queue);
 		/* drop ref taken in j1939_session_skb_queue() */
 		skb_unref(do_skb);
+		spin_unlock_irqrestore(&session->skb_queue.lock, flags);
 
 		kfree_skb(do_skb);
+	} else {
+		spin_unlock_irqrestore(&session->skb_queue.lock, flags);
 	}
-	spin_unlock_irqrestore(&session->skb_queue.lock, flags);
 }
 
 void j1939_session_skb_queue(struct j1939_session *session,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion
  2022-10-27 11:43 ` [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion Marc Kleine-Budde
@ 2022-10-27 17:40   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-10-27 17:40 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: netdev, davem, kuba, linux-can, kernel, anssi.hannula, extja, stable

Hello:

This series was applied to netdev/net.git (master)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Thu, 27 Oct 2022 13:43:53 +0200 you wrote:
> From: Anssi Hannula <anssi.hannula@bitwise.fi>
> 
> kvaser_usb uses completions to signal when a response event is received
> for outgoing commands.
> 
> However, it uses init_completion() to reinitialize the start_comp and
> stop_comp completions before sending the start/stop commands.
> 
> [...]

Here is the summary with links:
  - [net,1/4] can: kvaser_usb: Fix possible completions during init_completion
    https://git.kernel.org/netdev/net/c/2871edb32f46
  - [net,2/4] can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive
    https://git.kernel.org/netdev/net/c/702de2c21eed
  - [net,3/4] can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L
    https://git.kernel.org/netdev/net/c/d887087c8968
  - [net,4/4] can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb()
    https://git.kernel.org/netdev/net/c/c3c06c61890d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-27 17:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-27 11:43 [PATCH net 0/4] pull-request: can 2022-10-27 Marc Kleine-Budde
2022-10-27 11:43 ` [PATCH net 1/4] can: kvaser_usb: Fix possible completions during init_completion Marc Kleine-Budde
2022-10-27 17:40   ` patchwork-bot+netdevbpf
2022-10-27 11:43 ` [PATCH net 2/4] can: rcar_canfd: rcar_canfd_handle_global_receive(): fix IRQ storm on global FIFO receive Marc Kleine-Budde
2022-10-27 11:43 ` [PATCH net 3/4] can: rcar_canfd: fix channel specific IRQ handling for RZ/G2L Marc Kleine-Budde
2022-10-27 11:43 ` [PATCH net 4/4] can: j1939: transport: j1939_session_skb_drop_old(): spin_unlock_irqrestore() before kfree_skb() Marc Kleine-Budde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.