Re: [PATCH -mm 11/14] bootmem: respect goal more likely

[Johannes Weiner <hannes@saeurebad.de>]

Hi,

Yasunori Goto <y-goto@jp.fujitsu.com> writes:

> Hmm, my ia64 (NUMA) box can't boot up with this patch.
> I'll chase its cause deeply tomorrow.

I think I found it.

>> --- a/mm/bootmem.c
>> +++ b/mm/bootmem.c
>> @@ -408,6 +408,7 @@ static void * __init alloc_bootmem_core(
>>  				unsigned long size, unsigned long align,
>>  				unsigned long goal, unsigned long limit)
>>  {
>> +	unsigned long fallback = 0;
>>  	unsigned long min, max, start, step;
>>  
>>  	BUG_ON(!size);
>> @@ -441,10 +442,11 @@ static void * __init alloc_bootmem_core(
>>  
>>  	max -= PFN_DOWN(bdata->node_boot_start);
>>  	start -= PFN_DOWN(bdata->node_boot_start);
>> +	fallback -= PFN_DOWN(bdata->node_boot_start);
>>  
>>  	if (bdata->last_success > start) {
>> -		/* Set goal here to trigger a retry on failure */
>> -		start = goal = ALIGN(bdata->last_success, step);
>> +		fallback = start;
>> +		start = ALIGN(bdata->last_success, step);
>>  	}
>>  
>>  	while (1) {
>> @@ -491,10 +493,39 @@ find_block:
>>  		return region;
>>  	}
>>  
>> +	if (fallback) {
>> +		start = ALIGN(fallback, step);
>> +		fallback = 0;
>> +		goto find_block;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static void * __init ___alloc_bootmem_nopanic(unsigned long size,
>> +					unsigned long align,
>> +					unsigned long goal,
>> +					unsigned long limit)
>> +{
>> +	bootmem_data_t *bdata;
>> +
>> +restart:
>> +	list_for_each_entry(bdata, &bdata_list, list) {
>> +		void *region;
>> +
>> +		if (goal && goal < bdata->node_boot_start)
>> +			continue;

This check is backwards and probably made your boot fail.

>> +		if (limit && limit < bdata->node_boot_start)
>> +			continue;

Changed this to break, because we don't need to search any further if
the current node already starts at/above the limit (remember, we walk a
list sorted by ->node_boot_start here).

I also made the checks more intuitively understandable.

Could you try the following fix on top of this patch?

	Hannes

--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -513,10 +513,10 @@ restart:
 	list_for_each_entry(bdata, &bdata_list, list) {
 		void *region;
 
-		if (goal && goal < bdata->node_boot_start)
-			continue;
-		if (limit && limit < bdata->node_boot_start)
+		if (goal && bdata->node_low_pfn <= PFN_DOWN(goal))
 			continue;
+		if (limit && bdata->node_boot_start >= limit)
+			break;
 
 		region = alloc_bootmem_core(bdata, size, align, goal, limit);
 		if (region)