Hello, I wrote:
> This driver's 4-packet deep TX queue is too sensible to the "careless" callers
> ignoring its state (like netpoll in trapped mode), so add "queue full" check
> at
> the start of the hard_start_xmit() method (only under #ifndef RTL8139_NDEBUG,
> otherwise the queue will get stuck once dirty pointer gets out of sync);
> switch
> to using appropriate mnemonics for the return values while at it.
> Also, the out-of-sync dirty pointer check is misplaced in
> rtl8139_tx_interrupt()
> which causes TX descriptors to be inspected more than once in case the pointer
> really gets out-of-sync (and incrementing the dirty pointer always by 4 is
> just
> not enough, e.g. KGDBoE managed to stuff 20+ extra buffers into the queue) --
> place it before the loop and limit the loop to only look through 4 descriptors
> at most, so that already overwritten descriptors are just not counted.
> Signed-off-by: Sergei Shtylyov <[EMAIL PROTECTED]>
Jeff, do you have any opinion on this patch?
> Index: linux-2.6/drivers/net/8139too.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/8139too.c
> +++ linux-2.6/drivers/net/8139too.c
> @@ -90,7 +90,7 @@
> */
>
> #define DRV_NAME "8139too"
> -#define DRV_VERSION "0.9.28"
> +#define DRV_VERSION "0.9.31"
>
>
> #include <linux/module.h>
> @@ -1708,6 +1708,13 @@ static int rtl8139_start_xmit (struct sk
> unsigned int len = skb->len;
> unsigned long flags;
>
> +#ifndef RTL8139_NDEBUG
> + if (unlikely((tp->cur_tx - tp->dirty_tx) >= NUM_TX_DESC)) {
> + printk(KERN_ERR "%s: TX queue full!\n", dev->name);
> + return NETDEV_TX_BUSY;
> + }
> +#endif
> +
> /* Calculate the next Tx descriptor entry. */
> entry = tp->cur_tx % NUM_TX_DESC;
>
> @@ -1720,7 +1727,7 @@ static int rtl8139_start_xmit (struct sk
> } else {
> dev_kfree_skb(skb);
> tp->stats.tx_dropped++;
> - return 0;
> + return NETDEV_TX_OK;
> }
>
> spin_lock_irqsave(&tp->lock, flags);
> @@ -1740,7 +1747,7 @@ static int rtl8139_start_xmit (struct sk
> printk (KERN_DEBUG "%s: Queued Tx packet size %u to slot %d.\n",
> dev->name, len, entry);
>
> - return 0;
> + return NETDEV_TX_OK;
> }
>
>
> @@ -1755,6 +1762,16 @@ static void rtl8139_tx_interrupt (struct
>
> dirty_tx = tp->dirty_tx;
> tx_left = tp->cur_tx - dirty_tx;
> +
> +#ifndef RTL8139_NDEBUG
> + if (unlikely(tx_left > NUM_TX_DESC)) {
> + printk(KERN_ERR "%s: Out-of-sync dirty pointer, %ld vs. %ld.\n",
> + dev->name, dirty_tx, tp->cur_tx);
> + tx_left = NUM_TX_DESC;
> + dirty_tx = tp->cur_tx - NUM_TX_DESC;
> + }
> +#endif /* RTL8139_NDEBUG */
> +
> while (tx_left > 0) {
> int entry = dirty_tx % NUM_TX_DESC;
> int txstatus;
> @@ -1797,14 +1814,6 @@ static void rtl8139_tx_interrupt (struct
> tx_left--;
> }
>
> -#ifndef RTL8139_NDEBUG
> - if (tp->cur_tx - dirty_tx > NUM_TX_DESC) {
> - printk (KERN_ERR "%s: Out-of-sync dirty pointer, %ld vs.
> %ld.\n",
> - dev->name, dirty_tx, tp->cur_tx);
> - dirty_tx += NUM_TX_DESC;
> - }
> -#endif /* RTL8139_NDEBUG */
> -
> /* only wake the queue if we did work, and the queue is stopped */
> if (tp->dirty_tx != dirty_tx) {
> tp->dirty_tx = dirty_tx;
>
WBR, Sergei
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Kgdb-bugreport mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
