Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933834AbbDWLPn (ORCPT ); Thu, 23 Apr 2015 07:15:43 -0400 Received: from mail-la0-f46.google.com ([209.85.215.46]:33417 "EHLO mail-la0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933549AbbDWLPm (ORCPT ); Thu, 23 Apr 2015 07:15:42 -0400 MIME-Version: 1.0 In-Reply-To: <87egnbmamr.fsf@rasmusvillemoes.dk> References: <1429691618-13884-1-git-send-email-gavin.guo@canonical.com> <87egnbmamr.fsf@rasmusvillemoes.dk> Date: Thu, 23 Apr 2015 19:15:41 +0800 Message-ID: Subject: Re: [PATCH v2] mm/slab_common: Support the slub_debug boot option on specific object size From: Gavin Guo To: Rasmus Villemoes Cc: Christoph Lameter , penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, Andrew Morton , 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: 2556 Lines: 70 Hi Rasmus, On Thu, Apr 23, 2015 at 5:55 PM, Rasmus Villemoes wrote: > On Wed, Apr 22 2015, Gavin Guo wrote: > >> /* >> + * 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[] = { > > The array itself could be const, but more importantly it should be > marked __initconst so that the 27*sizeof(char*) bytes can be released after init. > >> + "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-4096" Good catch!! > >> + "kmalloc-32768", "kmalloc-65536", >> + "kmalloc-131072", "kmalloc-262144", >> + "kmalloc-524288", "kmalloc-1048576", >> + "kmalloc-2097152", "kmalloc-4194304", >> + "kmalloc-8388608", "kmalloc-16777216", >> + "kmalloc-33554432", "kmalloc-67108864" >> + }; > > On Wed, Apr 22 2015, Andrew Morton wrote: > >> 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. > > Eww. At least spell 8 as strlen("kmalloc-"). > >> 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 }, >> ... >> }; > > I'd vote for this color for the bikeshed :-) > > Rasmus Thanks for the review! I'll come out another version soon. -- 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/