Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752453AbWCFWfI (ORCPT ); Mon, 6 Mar 2006 17:35:08 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752452AbWCFWfI (ORCPT ); Mon, 6 Mar 2006 17:35:08 -0500 Received: from smtp.osdl.org ([65.172.181.4]:41914 "EHLO smtp.osdl.org") by vger.kernel.org with ESMTP id S1751308AbWCFWfF (ORCPT ); Mon, 6 Mar 2006 17:35:05 -0500 Date: Mon, 6 Mar 2006 14:34:32 -0800 (PST) From: Linus Torvalds To: Jesper Juhl cc: Linux Kernel Mailing List , Andrew Morton , markhe@nextd.demon.co.uk, Andrea Arcangeli , Mike Christie , James Bottomley , Jens Axboe , Pekka Enberg Subject: Re: Slab corruption in 2.6.16-rc5-mm2 In-Reply-To: Message-ID: References: <200603060117.16484.jesper.juhl@gmail.com> <200603062136.17098.jesper.juhl@gmail.com> <9a8748490603061253u5e4d7561vd4e566f5798a5f4@mail.gmail.com> <9a8748490603061256h794c5af9wa6fbb616e8ddbd89@mail.gmail.com> <9a8748490603061354vaa53c72na161d26065b9302e@mail.gmail.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2633 Lines: 87 Ok, I have a new favorite suspect. It is this one: commit 4d268eba1187ef66844a6a33b9431e5d0dadd4ad: [PATCH] slab: extract slab order calculation to separate function This patch moves the ugly loop that determines the 'optimal' size (page order) of cache slabs from kmem_cache_create() to a separate function and cleans it up a bit. Thanks to Matthew Wilcox for the help with this patch. Signed-off-by: Matthew Dobson Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds and I think it may be broken. In particular, as far as I can tell, that + /* More than offslab_limit objects will cause problems */ + if (flags & CFLGS_OFF_SLAB && cachep->num > offslab_limit) + break; has been incorrectly translated for several reasons: - we shouldn't check "cachep->num > offslab_limit". We should check just "num > offslab_limit" (cachep->num is the _previous_ number we tested). - when we do "break", we've already incremented "gfporder", and we should correct for that. Now, maybe I'm just off my rocker again (I've certainly been batting 0.000 so far, even if I think I've been finding real bugs). So who knows. But I get the feeling that that patch is broken. Either revert it, or try this (TOTALLY UNTESTED!!!) patch.. And hey, maybe I'm just crazy. Linus ---- diff --git a/mm/slab.c b/mm/slab.c index 2b0b151..1cca41d 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -1628,25 +1628,22 @@ static inline size_t calculate_slab_orde size_t size, size_t align, unsigned long flags) { size_t left_over = 0; + int gfporder; - for (;; cachep->gfporder++) { + for (gfporder = 0 ; gfporder < MAX_GFP_ORDER; gfporder++) { unsigned int num; size_t remainder; - if (cachep->gfporder > MAX_GFP_ORDER) { - cachep->num = 0; - break; - } - - cache_estimate(cachep->gfporder, size, align, flags, - &remainder, &num); + cache_estimate(gfporder, size, align, flags, &remainder, &num); if (!num) continue; + /* More than offslab_limit objects will cause problems */ - if (flags & CFLGS_OFF_SLAB && cachep->num > offslab_limit) + if ((flags & CFLGS_OFF_SLAB) && num > offslab_limit) break; cachep->num = num; + cachep->gfporder = gfporder; left_over = remainder; /* - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/