Re: [PATCH v3 2/7] mm, slab/slub: introduce kmalloc-reclaimable caches

[Vlastimil Babka <vbabka@suse.cz>]

On 07/19/2018 10:23 AM, Mel Gorman wrote:
>>  /*
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 4614248ca381..614fb7ab8312 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -1107,10 +1107,21 @@ void __init setup_kmalloc_cache_index_table(void)
>>  	}
>>  }
>>  
>> -static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
>> +static void __init
>> +new_kmalloc_cache(int idx, int type, slab_flags_t flags)
>>  {
>> -	kmalloc_caches[KMALLOC_NORMAL][idx] = create_kmalloc_cache(
>> -					kmalloc_info[idx].name,
>> +	const char *name;
>> +
>> +	if (type == KMALLOC_RECLAIM) {
>> +		flags |= SLAB_RECLAIM_ACCOUNT;
>> +		name = kasprintf(GFP_NOWAIT, "kmalloc-rcl-%u",
>> +						kmalloc_info[idx].size);
>> +		BUG_ON(!name);
>> +	} else {
>> +		name = kmalloc_info[idx].name;
>> +	}
>> +
>> +	kmalloc_caches[type][idx] = create_kmalloc_cache(name,
>>  					kmalloc_info[idx].size, flags, 0,
>>  					kmalloc_info[idx].size);
>>  }
> 
> I was going to query that BUG_ON but if I'm reading it right, we just
> have to be careful in the future that the "normal" kmalloc cache is always
> initialised before the reclaimable cache or there will be issues.

Yeah, I was just copying how the dma-kmalloc code does it.

>> @@ -1122,22 +1133,25 @@ static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
>>   */
>>  void __init create_kmalloc_caches(slab_flags_t flags)
>>  {
>> -	int i;
>> -	int type = KMALLOC_NORMAL;
>> +	int i, type;
>>  
>> -	for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
>> -		if (!kmalloc_caches[type][i])
>> -			new_kmalloc_cache(i, flags);
>> +	for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) {
>> +		for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
>> +			if (!kmalloc_caches[type][i])
>> +				new_kmalloc_cache(i, type, flags);
>>  
> 
> I don't see a problem here as such but the values of the KMALLOC_* types
> is important both for this function and the kmalloc_type(). It might be
> worth adding a warning that these functions be examined if updating the
> types but then again, anyone trying and getting it wrong will have a
> broken kernel so;

OK

> Acked-by: Mel Gorman <mgorman@techsingularity.net>

Thanks!