* [PATCH] sis900: come alive after temporary memory shortage [not found] ` <4337FF9D.70200@sw.ru> @ 2005-10-06 10:40 ` Daniele Venzano 2005-10-08 13:09 ` Vasily Averin 0 siblings, 1 reply; 4+ messages in thread From: Daniele Venzano @ 2005-10-06 10:40 UTC (permalink / raw) To: Jeff Garzik; +Cc: NetDev, Jeff Garzik, Stanislav Protassov, Vasily Averin This patch is good and fixes some corner cases. Please consider for inclusion. On Mon, Sep 26, 2005 at 06:03:09PM +0400, Konstantin Khorenko wrote: > Patch solves following problems: > 1) Forgotten counter incrementation in sis900_rx() in case > it doesn't get memory for skb, that leads to whole interface failure. > Problem is accompanied with messages: > eth0: Memory squeeze,deferring packet. > eth0: NULL pointer encountered in Rx ring, skipping > 2) If counter cur_rx overflows and there'll be temporary memory problems > buffer can't be recreated later, when memory IS avaliable. > 3) Limit the work in handler to prevent the endless packets processing if > new packets are generated faster then handled. > > Signed-off-by: Konstantin Khorenko <khorenko@sw.ru> > Signed-off-by: Vasily Averin <vvs@sw.ru> Signed-off-by: Daniele Venzano <venza@brownhat.org> --- main-2.6.13.1/drivers/net/sis900.c.sis900 2005-08-29 03:41:01.000000000 +0400 +++ main-2.6.13.1/drivers/net/sis900.c 2005-09-19 14:34:42.000000000 +0400 @@ -1696,6 +1696,8 @@ static int sis900_rx(struct net_device * long ioaddr = net_dev->base_addr; unsigned int entry = sis_priv->cur_rx % NUM_RX_DESC; u32 rx_status = sis_priv->rx_ring[entry].cmdsts; + int rx_work_limit = + (sis_priv->dirty_rx - sis_priv->cur_rx) % NUM_RX_DESC; if (netif_msg_rx_status(sis_priv)) printk(KERN_DEBUG "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d " @@ -1705,6 +1713,8 @@ static int sis900_rx(struct net_device * while (rx_status & OWN) { unsigned int rx_size; + if (--rx_work_limit < 0) + break; rx_size = (rx_status & DSIZE) - CRC_SIZE; if (rx_status & (ABORT|OVERRUN|TOOLONG|RUNT|RXISERR|CRCERR|FAERR)) { @@ -1770,6 +1780,7 @@ static int sis900_rx(struct net_device * sis_priv->rx_ring[entry].cmdsts = 0; sis_priv->rx_ring[entry].bufptr = 0; sis_priv->stats.rx_dropped++; + sis_priv->cur_rx++; break; } skb->dev = net_dev; @@ -1787,7 +1798,7 @@ static int sis900_rx(struct net_device * /* refill the Rx buffer, what if the rate of refilling is slower * than consuming ?? */ - for (;sis_priv->cur_rx - sis_priv->dirty_rx > 0; sis_priv->dirty_rx++) { + for (; sis_priv->cur_rx != sis_priv->dirty_rx; sis_priv->dirty_rx++) { struct sk_buff *skb; entry = sis_priv->dirty_rx % NUM_RX_DESC; -- ------------------------------ Daniele Venzano Web: http://teg.homeunix.org ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sis900: come alive after temporary memory shortage 2005-10-06 10:40 ` [PATCH] sis900: come alive after temporary memory shortage Daniele Venzano @ 2005-10-08 13:09 ` Vasily Averin 2005-10-10 8:37 ` Daniele Venzano 2005-10-28 20:46 ` Jeff Garzik 0 siblings, 2 replies; 4+ messages in thread From: Vasily Averin @ 2005-10-08 13:09 UTC (permalink / raw) To: Daniele Venzano, Andrew Morton; +Cc: Jeff Garzik, NetDev, Stanislav Protassov [-- Attachment #1: Type: text/plain, Size: 1386 bytes --] Daniele Venzano wrote: > This patch is good and fixes some corner cases. Please consider for > inclusion. Daniele, sorry, but unfortunately our patch was wrong. > + int rx_work_limit = > + (sis_priv->dirty_rx - sis_priv->cur_rx) % NUM_RX_DESC; when dirty_rx = cur_rx it computes limit=0, but should be NUM_RX_DESC Also I've noticed that 'if (sis_priv->rx_skbuff[entry] == NULL)' statement inside the while loop is really impossible now. Therefore I've increased message loglevel up to KERN_WARNING and added some useful debug. Daniele, could you please check our new patch carefully? Andrew, could you please drop our old patch and replace it by the new one? Thank you, Vasily Averin, SWsoft Linux kernel team Patch solves following problems: 1) Forgotten counter incrementation in sis900_rx() in case it doesn't get memory for skb, that leads to whole interface failure. Problem is accompanied with messages: eth0: Memory squeeze,deferring packet. eth0: NULL pointer encountered in Rx ring, skipping 2) If counter cur_rx overflows and there'll be temporary memory problems buffer can't be recreated later, when memory IS available. 3) Limit the work in handler to prevent the endless packets processing if new packets are generated faster then handled. Signed-off-by: Konstantin Khorenko <khorenko@sw.ru> Signed-off-by: Vasily Averin <vvs@sw.ru> [-- Attachment #2: diff-drv-sis900-20051008 --] [-- Type: text/plain, Size: 1995 bytes --] --- a/drivers/net/sis900.c 2005-10-08 12:22:53.000000000 +0400 +++ b/drivers/net/sis900.c 2005-10-08 15:12:29.000000000 +0400 @@ -1696,15 +1696,20 @@ static int sis900_rx(struct net_device * long ioaddr = net_dev->base_addr; unsigned int entry = sis_priv->cur_rx % NUM_RX_DESC; u32 rx_status = sis_priv->rx_ring[entry].cmdsts; + int rx_work_limit; if (netif_msg_rx_status(sis_priv)) printk(KERN_DEBUG "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d " "status:0x%8.8x\n", sis_priv->cur_rx, sis_priv->dirty_rx, rx_status); + rx_work_limit = sis_priv->dirty_rx + NUM_RX_DESC - sis_priv->cur_rx; while (rx_status & OWN) { unsigned int rx_size; + if (--rx_work_limit < 0) + break; + rx_size = (rx_status & DSIZE) - CRC_SIZE; if (rx_status & (ABORT|OVERRUN|TOOLONG|RUNT|RXISERR|CRCERR|FAERR)) { @@ -1732,9 +1737,11 @@ static int sis900_rx(struct net_device * we are working on NULL sk_buff :-( */ if (sis_priv->rx_skbuff[entry] == NULL) { if (netif_msg_rx_err(sis_priv)) - printk(KERN_INFO "%s: NULL pointer " - "encountered in Rx ring, skipping\n", - net_dev->name); + printk(KERN_WARNING "%s: NULL pointer " + "encountered in Rx ring\n" + "cur_rx:%4.4d, dirty_rx:%4.4d\n", + net_dev->name, sis_priv->cur_rx, + sis_priv->dirty_rx); break; } @@ -1770,6 +1777,7 @@ static int sis900_rx(struct net_device * sis_priv->rx_ring[entry].cmdsts = 0; sis_priv->rx_ring[entry].bufptr = 0; sis_priv->stats.rx_dropped++; + sis_priv->cur_rx++; break; } skb->dev = net_dev; @@ -1787,7 +1795,7 @@ static int sis900_rx(struct net_device * /* refill the Rx buffer, what if the rate of refilling is slower * than consuming ?? */ - for (;sis_priv->cur_rx - sis_priv->dirty_rx > 0; sis_priv->dirty_rx++) { + for (; sis_priv->cur_rx != sis_priv->dirty_rx; sis_priv->dirty_rx++) { struct sk_buff *skb; entry = sis_priv->dirty_rx % NUM_RX_DESC; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sis900: come alive after temporary memory shortage 2005-10-08 13:09 ` Vasily Averin @ 2005-10-10 8:37 ` Daniele Venzano 2005-10-28 20:46 ` Jeff Garzik 1 sibling, 0 replies; 4+ messages in thread From: Daniele Venzano @ 2005-10-10 8:37 UTC (permalink / raw) To: Vasily Averin; +Cc: Andrew Morton, Jeff Garzik, NetDev, Stanislav Protassov On Sat, Oct 08, 2005 at 05:09:30PM +0400, Vasily Averin wrote: > Daniele, could you please check our new patch carefully? This time I tested it as thoroughly as possible. It kept working during all the time the memory was filling up, until the OOM killer woke up from his dark pit. After OOM frenzy, the driver was still alive and kicking. > Andrew, could you please drop our old patch and replace it by the new one? Yes, this one is better, thanks. > Patch solves following problems: > 1) Forgotten counter incrementation in sis900_rx() in case > it doesn't get memory for skb, that leads to whole interface failure. > Problem is accompanied with messages: > eth0: Memory squeeze,deferring packet. > eth0: NULL pointer encountered in Rx ring, skipping > 2) If counter cur_rx overflows and there'll be temporary memory problems > buffer can't be recreated later, when memory IS available. > 3) Limit the work in handler to prevent the endless packets processing if > new packets are generated faster then handled. > > Signed-off-by: Konstantin Khorenko <khorenko@sw.ru> > Signed-off-by: Vasily Averin <vvs@sw.ru> Signed-off-by: Daniele Venzano <venza@brownhat.org> --- a/drivers/net/sis900.c 2005-10-08 12:22:53.000000000 +0400 +++ b/drivers/net/sis900.c 2005-10-08 15:12:29.000000000 +0400 @@ -1696,15 +1696,20 @@ static int sis900_rx(struct net_device * long ioaddr = net_dev->base_addr; unsigned int entry = sis_priv->cur_rx % NUM_RX_DESC; u32 rx_status = sis_priv->rx_ring[entry].cmdsts; + int rx_work_limit; if (netif_msg_rx_status(sis_priv)) printk(KERN_DEBUG "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d " "status:0x%8.8x\n", sis_priv->cur_rx, sis_priv->dirty_rx, rx_status); + rx_work_limit = sis_priv->dirty_rx + NUM_RX_DESC - sis_priv->cur_rx; while (rx_status & OWN) { unsigned int rx_size; + if (--rx_work_limit < 0) + break; + rx_size = (rx_status & DSIZE) - CRC_SIZE; if (rx_status & (ABORT|OVERRUN|TOOLONG|RUNT|RXISERR|CRCERR|FAERR)) { @@ -1732,9 +1737,11 @@ static int sis900_rx(struct net_device * we are working on NULL sk_buff :-( */ if (sis_priv->rx_skbuff[entry] == NULL) { if (netif_msg_rx_err(sis_priv)) - printk(KERN_INFO "%s: NULL pointer " - "encountered in Rx ring, skipping\n", - net_dev->name); + printk(KERN_WARNING "%s: NULL pointer " + "encountered in Rx ring\n" + "cur_rx:%4.4d, dirty_rx:%4.4d\n", + net_dev->name, sis_priv->cur_rx, + sis_priv->dirty_rx); break; } @@ -1770,6 +1777,7 @@ static int sis900_rx(struct net_device * sis_priv->rx_ring[entry].cmdsts = 0; sis_priv->rx_ring[entry].bufptr = 0; sis_priv->stats.rx_dropped++; + sis_priv->cur_rx++; break; } skb->dev = net_dev; @@ -1787,7 +1795,7 @@ static int sis900_rx(struct net_device * /* refill the Rx buffer, what if the rate of refilling is slower * than consuming ?? */ - for (;sis_priv->cur_rx - sis_priv->dirty_rx > 0; sis_priv->dirty_rx++) { + for (; sis_priv->cur_rx != sis_priv->dirty_rx; sis_priv->dirty_rx++) { struct sk_buff *skb; entry = sis_priv->dirty_rx % NUM_RX_DESC; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sis900: come alive after temporary memory shortage 2005-10-08 13:09 ` Vasily Averin 2005-10-10 8:37 ` Daniele Venzano @ 2005-10-28 20:46 ` Jeff Garzik 1 sibling, 0 replies; 4+ messages in thread From: Jeff Garzik @ 2005-10-28 20:46 UTC (permalink / raw) To: Vasily Averin; +Cc: Daniele Venzano, Andrew Morton, NetDev, Stanislav Protassov applied ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-10-28 20:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4337E76B.1090003@sw.ru>
[not found] ` <8204E0D1-F30D-494F-8E97-CDCC26A82807@libero.it>
[not found] ` <4337FF9D.70200@sw.ru>
2005-10-06 10:40 ` [PATCH] sis900: come alive after temporary memory shortage Daniele Venzano
2005-10-08 13:09 ` Vasily Averin
2005-10-10 8:37 ` Daniele Venzano
2005-10-28 20:46 ` Jeff Garzik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).