lkml.org 
[lkml]   [2023]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] mctp-i2c: increase the MCTP_I2C_TX_WORK_LEN to 500
From
Date
Hi,

On Fri, 2023-11-17 at 15:29 +0800, Jeremy Kerr wrote:
> spin_lock_irqsave(&midev->tx_queue.lock, flags);
> if (skb_queue_len(&midev->tx_queue) >= MCTP_I2C_TX_WORK_LEN) {
> netif_stop_queue(dev);
> spin_unlock_irqrestore(&midev->tx_queue.lock, flags);
> netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
> return NETDEV_TX_BUSY;
> }
>
> __skb_queue_tail(&midev->tx_queue, skb);
> if (skb_queue_len(&midev->tx_queue) == MCTP_I2C_TX_WORK_LEN) // normal stop
> netif_stop_queue(dev);
> spin_unlock_irqrestore(&midev->tx_queue.lock, flags);
>
> What looks like has happened here:
>
> 1) we have TX_WORK_LEN-1 packets queued
> 2) we release a flow, which queues the "marker" skb. the tx_queue now
> has TX_WORK_LEN items
> 3) we queue another packet, ending up with TX_WORK_LEN+1 in the queue
> 4) the == TX_WORK_LEN test fails, so we dont do a netif_stop_queue()
>
> A couple of potential fixes:
>
> * We do the check and conditional netif_stop_queue() in (2)
> * We change the check there to be `>= MCTP_I2C_TX_WORK_LEN`

My inclination would be to change the second comparison (the normal stop
condition) to 

/* -1 to allow space for an additional unlock_marker skb */
if (skb_queue_len(&midev->tx_queue) >= MCTP_I2C_TX_WORK_LEN-1)

Cheers,
Matt

\
 
 \ /
  Last update: 2023-11-20 14:02    [W:0.030 / U:0.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site