Re: [PATCH v2 2/2] memblock: Avoid useless checks in memblock_merge_regions().

[Peng Zhang]

Hi, Mike

> @@ -581,7 +586,7 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>   	bool insert = false;
>   	phys_addr_t obase = base;
>   	phys_addr_t end = base + memblock_cap_size(base, &size);
> -	int idx, nr_new;
> +	int idx, nr_new, start_rgn = -1, end_rgn;
>   	struct memblock_region *rgn;
>   
>   	if (!size)
> @@ -635,10 +640,14 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>   #endif
>   			WARN_ON(flags != rgn->flags);
>   			nr_new++;
> -			if (insert)
> +			if (insert) {
> +				if (start_rgn == -1)
We can't initialize start_rgn with 0 and use if(!start_rgn).

If start_rgn is equal to 0, it will be reassigned. It is possible that 
we will eventually need start_rgn to be equal to 0, and it may be 
reassigned to a larger value that does not meet expectations.

Thanks.

> +					start_rgn = idx;
> +				end_rgn = idx + 1;
>   				memblock_insert_region(type, idx++, base,
>   						       rbase - base, nid,
>   						       flags);
> +			}
>   		}
>   		/* area below @rend is dealt with, forget about it */
>   		base = min(rend, end);
> @@ -647,9 +656,13 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>   	/* insert the remaining portion */
>   	if (base < end) {
>   		nr_new++;
> -		if (insert)
> +		if (insert) {
> +			if (start_rgn == -1)
> +				start_rgn = idx;
> +			end_rgn = idx + 1;
>   			memblock_insert_region(type, idx, base, end - base,
>   					       nid, flags);
> +		}
>   	}
>   
>   	if (!nr_new)
> @@ -666,7 +679,7 @@ static int __init_memblock memblock_add_range(struct memblock_type *type,
>   		insert = true;
>   		goto repeat;
>   	} else {
> -		memblock_merge_regions(type);
> +		memblock_merge_regions(type, start_rgn, end_rgn);
>   		return 0;
>   	}
>   }
> @@ -902,7 +915,7 @@ static int __init_memblock memblock_setclr_flag(phys_addr_t base,
>   			r->flags &= ~flag;
>   	}
>   
> -	memblock_merge_regions(type);
> +	memblock_merge_regions(type, start_rgn, end_rgn);
>   	return 0;
>   }
>   
> @@ -1275,7 +1288,7 @@ int __init_memblock memblock_set_node(phys_addr_t base, phys_addr_t size,
>   	for (i = start_rgn; i < end_rgn; i++)
>   		memblock_set_region_node(&type->regions[i], nid);
>   
> -	memblock_merge_regions(type);
> +	memblock_merge_regions(type, start_rgn, end_rgn);
>   #endif
>   	return 0;
>   }
Sincerely yours,
Peng.