Re: [ath5k-devel] ath5k_tasklet_rx BUG_ON(bf->skb == NULL)

["Bob Copeland" <me@bobcopeland.com>]

On Tue, 13 Jan 2009 15:35:29 +0000 (GMT), Hugh Dickins wrote
> But I haven't seen any allocation failure for several days.
> I didn't get any while trying to simplify the reproduction,

Ok, well here's how I reproduced it manually.  Without the patch, you 
should see "can't alloc skbuff of size ...." followed shortly by the 
BUG.  With the patch, you see the same error but things proceed as 
normal.

(I chose 60 because anything < 40 will fail in probe).

I'll go ahead and push this upstream then today or tomorrow; let me 
know if you run into any problems with more testing.

> > +static
> > +struct sk_buff *ath5k_rx_skb_alloc(struct ath5k_softc *sc, dma_addr_t
*skb_addr)
> > +{
> > +	struct sk_buff *skb;
> > +	unsigned int off;

+    static int foo = 0; 

> > +	/*
> > +	 * Allocate buffer with headroom_needed space for the
> > +	 * fake physical layer header at the start.
> > +	 */
> > +	skb = dev_alloc_skb(sc->rxbufsize + sc->cachelsz - 1);

+    if (++foo == 60)
+        skb = NULL;

> > +
> > +	if (!skb) {
> > +		ATH5K_ERR(sc, "can't alloc skbuff of size %u\n",
> > +				sc->rxbufsize + sc->cachelsz - 1);
> > +		return NULL;
> > +	}


> > Changes-licensed-under: 3-Clause-BSD
> 
> Hmm, I haven't noticed anyone doing that before: hope you're not
> starting a trend!  I think you'll find (Documentation/SubmittingPatches)
> that your Signed-off-by agrees to the Developer's Certificate of Origin
> 1.1, which would put your patch under the same open source licence(s) as
> drivers/net/wireless/ath5k/base.c already contains - that's the usual
> case.

I agree with all of the above, but the SFLC suggests we do it anyway.

http://kerneltrap.org/Linux/Wireless_Project_Suggests_Changes-licensed-under_Tag

-- 
Bob Copeland %% www.bobcopeland.com