All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org, linux-can@vger.kernel.org,
	kernel@pengutronix.de, Oliver Hartkopp <socketcan@hartkopp.net>,
	stable@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	Norbert Slusarek <nslusarek@gmx.net>,
	Thadeu Lima de Souza Cascardo <cascardo@canonical.com>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH net 01/15] can: bcm: use call_rcu() instead of costly synchronize_rcu()
Date: Mon,  4 Jul 2022 14:25:59 +0200	[thread overview]
Message-ID: <20220704122613.1551119-2-mkl@pengutronix.de> (raw)
In-Reply-To: <20220704122613.1551119-1-mkl@pengutronix.de>

From: Oliver Hartkopp <socketcan@hartkopp.net>

In commit d5f9023fa61e ("can: bcm: delay release of struct bcm_op
after synchronize_rcu()") Thadeu Lima de Souza Cascardo introduced two
synchronize_rcu() calls in bcm_release() (only once at socket close)
and in bcm_delete_rx_op() (called on removal of each single bcm_op).

Unfortunately this slow removal of the bcm_op's affects user space
applications like cansniffer where the modification of a filter
removes 2048 bcm_op's which blocks the cansniffer application for
40(!) seconds.

In commit 181d4447905d ("can: gw: use call_rcu() instead of costly
synchronize_rcu()") Eric Dumazet replaced the synchronize_rcu() calls
with several call_rcu()'s to safely remove the data structures after
the removal of CAN ID subscriptions with can_rx_unregister() calls.

This patch adopts Erics approach for the can-bcm which should be
applicable since the removal of tasklet_kill() in bcm_remove_op() and
the introduction of the HRTIMER_MODE_SOFT timer handling in Linux 5.4.

Fixes: d5f9023fa61e ("can: bcm: delay release of struct bcm_op after synchronize_rcu()") # >= 5.4
Link: https://lore.kernel.org/all/20220520183239.19111-1-socketcan@hartkopp.net
Cc: stable@vger.kernel.org
Cc: Eric Dumazet <edumazet@google.com>
Cc: Norbert Slusarek <nslusarek@gmx.net>
Cc: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/bcm.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 65ee1b784a30..e60161bec850 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -100,6 +100,7 @@ static inline u64 get_u64(const struct canfd_frame *cp, int offset)
 
 struct bcm_op {
 	struct list_head list;
+	struct rcu_head rcu;
 	int ifindex;
 	canid_t can_id;
 	u32 flags;
@@ -718,10 +719,9 @@ static struct bcm_op *bcm_find_op(struct list_head *ops,
 	return NULL;
 }
 
-static void bcm_remove_op(struct bcm_op *op)
+static void bcm_free_op_rcu(struct rcu_head *rcu_head)
 {
-	hrtimer_cancel(&op->timer);
-	hrtimer_cancel(&op->thrtimer);
+	struct bcm_op *op = container_of(rcu_head, struct bcm_op, rcu);
 
 	if ((op->frames) && (op->frames != &op->sframe))
 		kfree(op->frames);
@@ -732,6 +732,14 @@ static void bcm_remove_op(struct bcm_op *op)
 	kfree(op);
 }
 
+static void bcm_remove_op(struct bcm_op *op)
+{
+	hrtimer_cancel(&op->timer);
+	hrtimer_cancel(&op->thrtimer);
+
+	call_rcu(&op->rcu, bcm_free_op_rcu);
+}
+
 static void bcm_rx_unreg(struct net_device *dev, struct bcm_op *op)
 {
 	if (op->rx_reg_dev == dev) {
@@ -757,6 +765,9 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
 		if ((op->can_id == mh->can_id) && (op->ifindex == ifindex) &&
 		    (op->flags & CAN_FD_FRAME) == (mh->flags & CAN_FD_FRAME)) {
 
+			/* disable automatic timer on frame reception */
+			op->flags |= RX_NO_AUTOTIMER;
+
 			/*
 			 * Don't care if we're bound or not (due to netdev
 			 * problems) can_rx_unregister() is always a save
@@ -785,7 +796,6 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
 						  bcm_rx_handler, op);
 
 			list_del(&op->list);
-			synchronize_rcu();
 			bcm_remove_op(op);
 			return 1; /* done */
 		}

base-commit: 280e3a857d96f9ca8e24632788e1e7a0fec4e9f7
-- 
2.35.1



  reply	other threads:[~2022-07-04 12:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-04 12:25 [PATCH net 0/15] pull-request: can 2022-07-04 Marc Kleine-Budde
2022-07-04 12:25 ` Marc Kleine-Budde [this message]
2022-07-05  3:30   ` [PATCH net 01/15] can: bcm: use call_rcu() instead of costly synchronize_rcu() patchwork-bot+netdevbpf
2022-07-04 12:26 ` [PATCH net 02/15] Revert "can: xilinx_can: Limit CANFD brp to 2" Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 03/15] can: rcar_canfd: Fix data transmission failed on R-Car V3U Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 04/15] can: gs_usb: gs_usb_open/close(): fix memory leak Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 05/15] can: grcan: grcan_probe(): remove extra of_node_get() Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 06/15] can: m_can: m_can_chip_config(): actually enable internal timestamping Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 07/15] can: m_can: m_can_{read_fifo,echo_tx_event}(): shift timestamp to full 32 bits Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 08/15] can: kvaser_usb: replace run-time checks with struct kvaser_usb_driver_info Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 09/15] can: kvaser_usb: kvaser_usb_leaf: fix CAN clock frequency regression Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 10/15] can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 11/15] can: mcp251xfd: mcp251xfd_regmap_crc_read(): improve workaround handling for mcp2517fd Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 12/15] can: mcp251xfd: mcp251xfd_regmap_crc_read(): update workaround broken CRC on TBC register Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 13/15] can: mcp251xfd: mcp251xfd_stop(): add missing hrtimer_cancel() Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 14/15] can: mcp251xfd: mcp251xfd_register_get_dev_id(): use correct length to read dev_id Marc Kleine-Budde
2022-07-04 12:26 ` [PATCH net 15/15] can: mcp251xfd: mcp251xfd_register_get_dev_id(): fix endianness conversion Marc Kleine-Budde

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=20220704122613.1551119-2-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=cascardo@canonical.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nslusarek@gmx.net \
    --cc=socketcan@hartkopp.net \
    --cc=stable@vger.kernel.org \
    /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 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.