linux-kernel - [PATCH 16/18] can: m_can: Use tx_fifo_in_flight for netif

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 21 Dec 2022 16:25:35 +0100
From:   Markus Schneider-Pargmann <msp@...libre.com>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        Chandrasekar Ramakrishnan <rcsekar@...sung.com>,
        Wolfgang Grandegger <wg@...ndegger.com>
Cc:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>,
        linux-can@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Markus Schneider-Pargmann <msp@...libre.com>
Subject: [PATCH 16/18] can: m_can: Use tx_fifo_in_flight for netif_queue control

The network queue is currently always stopped in start_xmit and
continued in the interrupt handler. This is not possible anymore if we
want to keep multiple transmits in flight in parallel.

Use the previously introduced tx_fifo_in_flight counter to control the
network queue instead. This has the benefit of not needing to ask the
hardware about fifo status.

This patch stops the network queue in start_xmit if the number of
transmits in flight reaches the size of the fifo and wakes up the queue
from the interrupt handler once the transmits in flight drops below the
fifo size. This means any skbs over the limit will be rejected
immediately in start_xmit (it shouldn't be possible at all to reach that
state anyways).

The maximum number of transmits in flight is the size of the fifo.

Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
---
 drivers/net/can/m_can/m_can.c | 58 +++++++++++------------------------
 1 file changed, 18 insertions(+), 40 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 90c9ff474149..076fa60317c2 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -369,16 +369,6 @@ m_can_txe_fifo_read(struct m_can_classdev *cdev, u32 fgi, u32 offset, u32 *val)
 	return cdev->ops->read_fifo(cdev, addr_offset, val, 1);
 }
 
-static inline bool _m_can_tx_fifo_full(u32 txfqs)
-{
-	return !!(txfqs & TXFQS_TFQF);
-}
-
-static inline bool m_can_tx_fifo_full(struct m_can_classdev *cdev)
-{
-	return _m_can_tx_fifo_full(m_can_read(cdev, M_CAN_TXFQS));
-}
-
 static void m_can_config_endisable(struct m_can_classdev *cdev, bool enable)
 {
 	u32 cccr = m_can_read(cdev, M_CAN_CCCR);
@@ -1064,6 +1054,8 @@ static int m_can_echo_tx_event(struct net_device *dev)
 							  ack_fgi));
 
 	spin_lock(&cdev->tx_handling_spinlock);
+	if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops && processed > 0)
+		netif_wake_queue(cdev->net);
 	cdev->tx_fifo_in_flight -= processed;
 	spin_unlock(&cdev->tx_handling_spinlock);
 
@@ -1167,10 +1159,6 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
 			/* New TX FIFO Element arrived */
 			if (m_can_echo_tx_event(dev) != 0)
 				goto out_fail;
-
-			if (netif_queue_stopped(dev) &&
-			    !m_can_tx_fifo_full(cdev))
-				netif_wake_queue(dev);
 		}
 	}
 
@@ -1677,7 +1665,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
 	struct net_device *dev = cdev->net;
 	struct id_and_dlc fifo_header;
 	u32 cccr, fdflags;
-	u32 txfqs;
 	int err;
 	int putidx;
 
@@ -1733,24 +1720,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
 		char buf[TXB_ELEMENT_SIZE];
 		/* Transmit routine for version >= v3.1.x */
 
-		txfqs = m_can_read(cdev, M_CAN_TXFQS);
-
-		/* Check if FIFO full */
-		if (_m_can_tx_fifo_full(txfqs)) {
-			/* This shouldn't happen */
-			netif_stop_queue(dev);
-			netdev_warn(dev,
-				    "TX queue active although FIFO is full.");
-
-			if (cdev->is_peripheral) {
-				kfree_skb(skb);
-				dev->stats.tx_dropped++;
-				return NETDEV_TX_OK;
-			} else {
-				return NETDEV_TX_BUSY;
-			}
-		}
-
 		/* get put index for frame */
 		putidx = cdev->tx_fifo_putidx;
 
@@ -1787,11 +1756,6 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
 		m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
 		cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
 					0 : cdev->tx_fifo_putidx);
-
-		/* stop network queue if fifo full */
-		if (m_can_tx_fifo_full(cdev) ||
-		    m_can_next_echo_skb_occupied(dev, putidx))
-			netif_stop_queue(dev);
 	}
 
 	return NETDEV_TX_OK;
@@ -1830,10 +1794,16 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
 		return NETDEV_TX_OK;
 	}
 
-	netif_stop_queue(cdev->net);
-
 	spin_lock(&cdev->tx_handling_spinlock);
 	++cdev->tx_fifo_in_flight;
+	if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops) {
+		netif_stop_queue(cdev->net);
+		if (cdev->tx_fifo_in_flight > cdev->nr_tx_ops) {
+			netdev_err(cdev->net, "hard_xmit called while TX FIFO full\n");
+			spin_unlock(&cdev->tx_handling_spinlock);
+			return NETDEV_TX_BUSY;
+		}
+	}
 	spin_unlock(&cdev->tx_handling_spinlock);
 
 	m_can_tx_queue_skb(cdev, skb);
@@ -1846,6 +1816,14 @@ static netdev_tx_t m_can_start_fast_xmit(struct m_can_classdev *cdev,
 {
 	spin_lock(&cdev->tx_handling_spinlock);
 	++cdev->tx_fifo_in_flight;
+	if (cdev->tx_fifo_in_flight >= cdev->nr_tx_ops) {
+		netif_stop_queue(cdev->net);
+		if (cdev->tx_fifo_in_flight > cdev->nr_tx_ops) {
+			netdev_err(cdev->net, "hard_xmit called while TX FIFO full\n");
+			spin_unlock(&cdev->tx_handling_spinlock);
+			return NETDEV_TX_BUSY;
+		}
+	}
 	spin_unlock(&cdev->tx_handling_spinlock);
 
 	return m_can_tx_handler(cdev, skb);
-- 
2.38.1