netdev
[Top] [All Lists]

Re: [patch] tg3 ring buffer allocation error handling

To: Sven Schuster <schuster.sven@xxxxxx>
Subject: Re: [patch] tg3 ring buffer allocation error handling
From: Matt Mackall <mpm@xxxxxxxxxxx>
Date: Wed, 14 Apr 2004 16:45:10 -0500
Cc: Jeff Garzik <jgarzik@xxxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <20040414205959.GA22051@zion.homelinux.com>
References: <20040414184642.GZ1175@waste.org> <20040414205959.GA22051@zion.homelinux.com>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
On Wed, Apr 14, 2004 at 10:59:59PM +0200, Sven Schuster wrote:
> On Wed, Apr 14, 2004 at 01:46:42PM -0500, Matt Mackall told us:
> >     if (tp->tg3_flags & TG3_FLAG_JUMBO_ENABLE) {
> >             for (i = 0; i < tp->rx_jumbo_pending; i++) {
> >                     if (tg3_alloc_rx_skb(tp, RXD_OPAQUE_RING_JUMBO,
> > -                                        -1, i) < 0)
> > -                           break;
> > +                                        -1, i) < 0) {
> > +                           printk("tg3_alloc_rx_skb %d failed\n", i);
> > +                           return -ENOMEM;
> 
> Please correct me if I'm wrong, but shouldn't the buffers from the 
> irst allocation (RXD_OPAQUE_RING_STD) be freed (maybe via 
> tg3_free_rings??) when we get an error here??

The buffers are freed in the module release path, which is invoked
when we return failure here. Without this, we don't notice the
allocation failure and instead blow up when we get traffic.

FYI, I last tested this on a mem=4M box, where it was very obvious
that the 1M of ring buffers weren't leaking.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

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