Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758697AbbDWDKn (ORCPT ); Wed, 22 Apr 2015 23:10:43 -0400 Received: from mail-lb0-f174.google.com ([209.85.217.174]:32852 "EHLO mail-lb0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758682AbbDWDKl (ORCPT ); Wed, 22 Apr 2015 23:10:41 -0400 MIME-Version: 1.0 In-Reply-To: <20150422140039.19812721dff3fec674dc5134@linux-foundation.org> References: <1429691618-13884-1-git-send-email-gavin.guo@canonical.com> <20150422140039.19812721dff3fec674dc5134@linux-foundation.org> Date: Thu, 23 Apr 2015 11:10:40 +0800 Message-ID: Subject: Re: [PATCH v2] mm/slab_common: Support the slub_debug boot option on specific object size From: Gavin Guo To: Andrew Morton Cc: Christoph Lameter , penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, linux-mm@kvack.org, linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4773 Lines: 116 On Thu, Apr 23, 2015 at 5:00 AM, Andrew Morton wrote: > On Wed, 22 Apr 2015 16:33:38 +0800 Gavin Guo wrote: > >> The slub_debug=PU,kmalloc-xx cannot work because in the >> create_kmalloc_caches() the s->name is created after the >> create_kmalloc_cache() is called. The name is NULL in the >> create_kmalloc_cache() so the kmem_cache_flags() would not set the >> slub_debug flags to the s->flags. The fix here set up a kmalloc_names >> string array for the initialization purpose and delete the dynamic >> name creation of kmalloc_caches. >> >> --- a/mm/slab_common.c >> +++ b/mm/slab_common.c >> @@ -793,6 +793,26 @@ void __init create_kmalloc_caches(unsigned long flags) >> int i; >> >> /* >> + * The kmalloc_names is for temporary usage to make >> + * slub_debug=,kmalloc-xx option work in the boot time. The >> + * kmalloc_index() support to 2^26=64MB. So, the final entry of the >> + * table is kmalloc-67108864. >> + */ >> + static const char *kmalloc_names[] = { >> + "0", "kmalloc-96", "kmalloc-192", >> + "kmalloc-8", "kmalloc-16", "kmalloc-32", >> + "kmalloc-64", "kmalloc-128", "kmalloc-256", >> + "kmalloc-512", "kmalloc-1024", "kmalloc-2048", >> + "kmalloc-4196", "kmalloc-8192", "kmalloc-16384", >> + "kmalloc-32768", "kmalloc-65536", >> + "kmalloc-131072", "kmalloc-262144", >> + "kmalloc-524288", "kmalloc-1048576", >> + "kmalloc-2097152", "kmalloc-4194304", >> + "kmalloc-8388608", "kmalloc-16777216", >> + "kmalloc-33554432", "kmalloc-67108864" >> + }; >> + >> + /* >> * Patch up the size_index table if we have strange large alignment >> * requirements for the kmalloc array. This is only the case for >> * MIPS it seems. The standard arches will not generate any code here. >> @@ -835,7 +855,8 @@ void __init create_kmalloc_caches(unsigned long flags) >> } >> for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) { >> if (!kmalloc_caches[i]) { >> - kmalloc_caches[i] = create_kmalloc_cache(NULL, >> + kmalloc_caches[i] = create_kmalloc_cache( >> + kmalloc_names[i], >> 1 << i, flags); >> } > > You could do something like > > kmalloc_caches[i] = create_kmalloc_cache( > kmalloc_names[i], > kstrtoul(kmalloc_names[i] + 8), > flags); > > here, and remove those weird "96" and "192" cases. Thanks for your reply. I'm not sure if I am following your idea. Would you mean to simply replace the string like: kmalloc_caches[1] = create_kmalloc_cache( kmalloc_names[1], 96, flags); as follows: kmalloc_caches[1] = create_kmalloc_cache( kmalloc_names[1], kstrtoul(kmalloc_names[i] + 8), flags); or if you like to merge the last 2 if conditions for 96 and 192 cases to the first if condition check: if (!kmalloc_caches[i]) { kmalloc_caches[i] = create_kmalloc_cache(NULL, 1 << i, flags); } > > Or if that's considered too messy, make it > > static const struct { > const char *name; > unsigned size; > } kmalloc_cache_info[] = { > { NULL, 0 }, > { "kmalloc-96", 96 }, > ... > }; > > but I'm thinking the kstrtoul() trick will be OK. > >> - for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) { >> - struct kmem_cache *s = kmalloc_caches[i]; >> - char *n; >> - >> - if (s) { >> - n = kasprintf(GFP_NOWAIT, "kmalloc-%d", kmalloc_size(i)); >> - >> - BUG_ON(!n); >> - s->name = n; >> - } >> - } >> - > > slab_kmem_cache_release() still does kfree_const(s->name). It will > crash? > -- 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/