Re: Oops in a driver while using SLUB as a SLAB allocator

[Christoph Lameter]

On Fri, 22 Jun 2007, Hugh Dickins wrote:

> I'm quite happy with this approach for 2.6.23-rc, along with your ARM
> dma_map patch which (if I understood aright) rmk approved.  Except,
> haven't you misplaced that VM_BUG_ON between an if and its else?

Right.

> And I'd much prefer you to make it an outright BUG_ON, because
> many testers have those VM_BUG_ONs configured out.

Thought about it but doing so would create quite a load of BUG_ONs in the 
VM given the frequent use of that particular inline function. And AFAIK
we are only aware of one other potential call site that could cause 
trouble. Many arches have run SLUB now for awhile and would certainly have 
shown issues if they would do strange things with slab allocs. Even with 
SLAB they would have to be very careful in order to make this work. So I 
think its rather unlikely that this is going to be triggered. Its 
primarily useful for debugging if strange things start to happen.
The VM_BUG_ON could stay there for good to make sure development does not
result in similar issues in the future.

Fixed up patch:




Add VM_BUG_ON in case someone uses page_mapping on a slab page

Detect slab objects being passed to the page oriented functions of the VM.

It is not sufficient to simply return NULL because the functions calling
page_mapping may depend on other items of the page_struct also to be setup
properly. Moreover slab object may not be properly aligned. The page oriented
functions of the VM expect to operate on page aligned, page sized objects.
Operations on object straddling page boundaries may only affect the objects
partially which may lead to surprising results.

It is better to detect eventually remaining uses and eliminate them.

Signed-off-by: Christoph Lameter <clameter@xxxxxxx>

---
 include/linux/mm.h |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Index: linux-2.6/include/linux/mm.h
===================================================================
--- linux-2.6.orig/include/linux/mm.h	2007-06-22 10:33:27.000000000 -0700
+++ linux-2.6/include/linux/mm.h	2007-06-22 11:44:10.000000000 -0700
@@ -601,12 +601,9 @@ static inline struct address_space *page
 {
 	struct address_space *mapping = page->mapping;
 
+	VM_BUG_ON(PageSlab(page));
 	if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
-#ifdef CONFIG_SLUB
-	else if (unlikely(PageSlab(page)))
-		mapping = NULL;
-#endif
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
 	return mapping;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/