* [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).