netdev
[Top] [All Lists]

Re: [PATCH] e1000 poll behavior

To: Robert Olsson <Robert.Olsson@xxxxxxxxxxx>
Subject: Re: [PATCH] e1000 poll behavior
From: Martin Josefsson <gandalf@xxxxxxxxxxxxxx>
Date: Fri, 10 Dec 2004 17:19:36 +0100
Cc: netdev@xxxxxxxxxxx
In-reply-to: <16824.30773.125766.220826@xxxxxxxxxxxx>
References: <16824.30773.125766.220826@xxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Thu, 2004-12-09 at 17:07, Robert Olsson wrote:
> Hello!
> 
> Seems e1000 never gets into poll mode when tx_cleaned is false. Compare
> irq's on RX interfaces eth0, eth2 in the forwarding test below.

Yes, we discussed this in private some time ago, unfortunately I havn't
had any proper hardware to test my changes on :(

> --- drivers/net/e1000/e1000_main.c.orig       2004-12-09 17:49:56.000000000 
> +0100
> +++ drivers/net/e1000/e1000_main.c    2004-12-09 19:05:07.000000000 +0100
> @@ -2179,8 +2179,8 @@
>       *budget -= work_done;
>       netdev->quota -= work_done;
>       
> -     /* if no Rx and Tx cleanup work was done, exit the polling mode */
> -     if(!tx_cleaned || (work_done < work_to_do) || 
> +     /* if no Tx and not enough Rx work done, exit the polling mode */
> +     if((!tx_cleaned && (work_done < work_to_do)) || 
>                               !netif_running(netdev)) {
>               netif_rx_complete(netdev);
>               e1000_irq_enable(adapter);

This patch is broken.
What about the case where tx_cleaned is true but (work_done <
work_to_do) is true. Then the statement is false and we continue, later
we return (work_done >= work_to_do) which equals 0 (not seen in patch).
We are not allowed to return 0 when still on poll_list. That will sort
of continue polling in a degraded way (no increase in quota) but will
screw up device refcounting badly.

That final return must be changed into "return 1;"

-- 
/Martin

Attachment: signature.asc
Description: This is a digitally signed message part

<Prev in Thread] Current Thread [Next in Thread>