alloc_skb cannot be called with GFP_DMA

I think it's good idea to apply this patch to mainline:

@@ -1172,7 +1172,6 @@
 
 static inline void kmem_cache_alloc_head(kmem_cache_t *cachep, int flags)
 {
-#if DEBUG
 	if (flags & SLAB_DMA) {
 		if (!(cachep->gfpflags & GFP_DMA))
 			BUG();

@@ -1180,7 +1179,6 @@
 		if (cachep->gfpflags & GFP_DMA)
 			BUG();
 	}
-#endif
 }
 
 static inline void * kmem_cache_alloc_one_tail (kmem_cache_t *cachep,

This will trap anybody trying calling alloc_skb using GFP_DMA, that usage is illegal, it would work by luck only if _everybody_ calling alloc_skb would be passing GFP_DMA too (because the gfp_mask is passed to the GFP in order to get right things like GFP_WAIT) which is obviously not the case as the skb cache is shared by everybody.

Actually such bugcheck is been triggered by an IKD user because IKD #defines DEBUG, but I think we should enable such bugcheck it in mainline too because it's too easy to forget about such requirement of the slab cache.

In order to fix those bugs correctly and avoid memory corruption a new skbuff_head_cache_isadma skb cache will be needed.

A fast grep revealed a few buggy network drivers already (of course the bug is going to affect only ISA drivers so it's not a showstopper).

Andrea

"It doesnt affect my computer so its not a problem" - humm

If somoe can add the needed isa dma handling to alloc_skb (or alloc_isa_skb) I'll go and fix all the drivers up.

I never said anything like that, with "it's not a showstopper" I only meant "it doesn't affect 99% of the hardware configurations out there".

alloc_isa_skb will avoid to slowdown alloc_skb so I prefer it compared to hiding the logic inside alloc_skb.

Thanks. With the bugcheck patch I wanted to make _sure_ that somebody will do that (which is very far from "... so it's not a problem" :).

Andrea

Hello!

Stop! This is redundant. GFP_DMA is for skb data, not head!

So that, it is enough and right to clear GFP_DMA inside alloc_skb when allocating skb head.

Alexey

ah it's all metadata, so this should fix it (the bugcheck will still trap any skb_clone caller that uses GFP_DMA because it doesn't make sense to call skb_clone with GFP_DMA):

@@ -180,7 +180,7 @@
 	/* Get the HEAD */
 	skb = skb_head_from_pool();
 	if (skb == NULL) {
-		skb = kmem_cache_alloc(skbuff_head_cache, gfp_mask);
+		skb = kmem_cache_alloc(skbuff_head_cache, gfp_mask & ~__GFP_DMA);
 		if (skb == NULL)
 			goto nohead;
 	}

Andrea