Hi Rasmus,
On Thu, Apr 23, 2015 at 5:55 PM, Rasmus Villemoes
<linux@xxxxxxxxxxxxxxxxxx> wrote:
> On Wed, Apr 22 2015, Gavin Guo <gavin.guo@xxxxxxxxxxxxx> 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 <akpm@xxxxxxxxxxxxxxxxxxxx> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>