diff mbox series

[net,2/2] net: xilinx: axienet: Check if Tx queue enabled

Message ID 20241030062533.2527042-3-suraj.gupta2@amd.com (mailing list archive)
State New, archived
Headers show
Series net: xilinx: axienet: Fix kernel crash in dmaengine transmit path | expand

Commit Message

Gupta, Suraj Oct. 30, 2024, 6:25 a.m. UTC
Check return value of netif_txq_maybe_stop() in transmit
direction and start dma engine only if queue is enabled.

Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Nov. 3, 2024, 10:37 p.m. UTC | #1
On Wed, 30 Oct 2024 11:55:33 +0530 Suraj Gupta wrote:
> Check return value of netif_txq_maybe_stop() in transmit
> direction and start dma engine only if queue is enabled.

The first patch makes sense, let me apply that one.
But this one I don't understand - what is the problem you're trying 
to fix? netif_txq_maybe_stop() tries to stop the queue if the *next*
packet may not fit in the queue. The currently processed packet is
assumed to have already been queued.
Gupta, Suraj Nov. 4, 2024, 7:08 a.m. UTC | #2
> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, November 4, 2024 4:07 AM
> To: Gupta, Suraj <Suraj.Gupta2@amd.com>
> Cc: Pandey, Radhey Shyam <radhey.shyam.pandey@amd.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; Simek, Michal <michal.simek@amd.com>;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>
> Subject: Re: [PATCH net 2/2] net: xilinx: axienet: Check if Tx queue enabled
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Wed, 30 Oct 2024 11:55:33 +0530 Suraj Gupta wrote:
> > Check return value of netif_txq_maybe_stop() in transmit direction and
> > start dma engine only if queue is enabled.
> 
> The first patch makes sense, let me apply that one.
> But this one I don't understand - what is the problem you're trying to fix?
> netif_txq_maybe_stop() tries to stop the queue if the *next* packet may not fit in the
> queue. The currently processed packet is assumed to have already been queued.

I was under impression that it tries to stop if "current" packet may not fit in the queue. This check won't be required then, thanks for applying first patch.

Regards,
Suraj
diff mbox series

Patch

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 0f4b02fe6f85..620c19edeeee 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -926,8 +926,14 @@  axienet_start_xmit_dmaengine(struct sk_buff *skb, struct net_device *ndev)
 	dma_tx_desc->callback_result = axienet_dma_tx_cb;
 	txq = skb_get_tx_queue(lp->ndev, skb);
 	netdev_tx_sent_queue(txq, skb->len);
-	netif_txq_maybe_stop(txq, CIRC_SPACE(lp->tx_ring_head, lp->tx_ring_tail, TX_BD_NUM_MAX),
-			     MAX_SKB_FRAGS + 1, 2 * MAX_SKB_FRAGS);
+
+	/* Check if queue stopped */
+	if (!netif_txq_maybe_stop(txq, CIRC_SPACE(lp->tx_ring_head, lp->tx_ring_tail,
+						  TX_BD_NUM_MAX),
+						  MAX_SKB_FRAGS + 1, 2 * MAX_SKB_FRAGS)) {
+		dma_unmap_sg(lp->dev, skbuf_dma->sgl, sg_len, DMA_TO_DEVICE);
+		return NETDEV_TX_BUSY;
+	}
 
 	dmaengine_submit(dma_tx_desc);
 	dma_async_issue_pending(lp->tx_chan);