Re: [PATCH] RTL 8139 oops cured

From: Jeff Garzik (jgarzik@mandrakesoft.com)
Date: Sun Oct 15 2000 - 15:51:22 EST


tori@tellus.mine.nu wrote:
>
> On Sun, 15 Oct 2000, Shane Shrybman wrote:
> > I applied these changes to 2.4.0-test10-pre3 and I got these messages in
> > the system log:
> >
> > Oct 15 11:24:05 mars kernel: __alloc_pages: 5-order allocation failed.
> > Oct 15 11:24:05 mars kernel: eth0: Memory squeeze, dropping packet.
> > Oct 15 11:24:05 mars kernel: __alloc_pages: 5-order allocation failed.
> > Oct 15 11:24:05 mars kernel: eth0: Memory squeeze, dropping packet.
> > Oct 15 11:24:05 mars kernel: __alloc_pages: 5-order allocation failed.
> > Oct 15 11:24:05 mars kernel: eth0: Memory squeeze, dropping packet.
> >
> > And then a spontaneous reboot. :( Which is what I have been
> > seeing with this driver for a while.
>
> What happens if you try the following patch instead? The original
> out-of-memory behaviour seems a bit bogus to me. This patch is untested,
> but should work.
>
> (After looking at the specification for the 8139, I am acctually ashamed
> of having bough one.

Not an uncommon thought :)

> How they could leave out a description of the receive
> ring buffer in the spec is truly amazing. I wonder if the status=0 case I
> encoutered was just a case of getting lost in the ring buffer.)

Can you
>
> /Tobias
>
> --- 8139too.c.orig Sun Oct 15 01:49:47 2000
> +++ 8139too.c Sun Oct 15 22:23:34 2000
> @@ -1736,8 +1736,11 @@
> /* if Rx err received, Rx process gets reset, so
> * we abort any further Rx processing
> */
> - if (rx_status &
> - (RxBadSymbol | RxRunt | RxTooLong | RxCRCErr | RxBadAlign)) {
> + if (pkt_size < 0 || !(rx_status & RxStatusOK)) {
> + if (pkt_size < 0)
> + printk (KERN_WARNING
> + "%s: Negative packet size received (%04x).\n",
> + dev->name, pkt_size);
> rtl8139_rx_err (rx_status, dev, tp, ioaddr);
> return;
> }
> @@ -1752,23 +1755,23 @@

I made 'rx_size' and 'pkt_size' unsigned ints instead of signed, and
then added an '(pkt_size > 1536)' check. The !RxStatusOK check is
pretty good too.

To answer your supposition above, yes, you're probably getting lost in
the ring buffer somewhere.

> */
>
> skb = dev_alloc_skb (pkt_size + 2);
> - if (skb == NULL) {
> + if (skb) {
> + skb->dev = dev;
> + skb_reserve (skb, 2); /* 16 byte align the IP fields. */
> +
> + eth_copy_and_sum (skb, &rx_ring[ring_offset + 4], pkt_size, 0);
> + skb_put (skb, pkt_size);
> +
> + skb->protocol = eth_type_trans (skb, dev);
> + netif_rx (skb);
> + tp->stats.rx_bytes += pkt_size;
> + tp->stats.rx_packets++;
> + } else {
> printk (KERN_WARNING
> "%s: Memory squeeze, dropping packet.\n",
> dev->name);
> tp->stats.rx_dropped++;
> - break;
> }
> - skb->dev = dev;
> - skb_reserve (skb, 2); /* 16 byte align the IP fields. */
> -
> - eth_copy_and_sum (skb, &rx_ring[ring_offset + 4], pkt_size, 0);
> - skb_put (skb, pkt_size);
> -
> - skb->protocol = eth_type_trans (skb, dev);
> - netif_rx (skb);
> - tp->stats.rx_bytes += pkt_size;
> - tp->stats.rx_packets++;
>
> cur_rx = (cur_rx + rx_size + 4 + 3) & ~3;
> RTL_W16_F (RxBufPtr, cur_rx - 16);

good idea, applied.

-- 
Jeff Garzik                    | The difference between laziness and
Building 1024                  | prioritization is the end result.
MandrakeSoft                   |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Oct 15 2000 - 21:00:29 EST