Re: [RESEND PATCH 1/1] mm/percpu.c: correct max_distance calculation for pcpu_embed_first_chunk()

[zijun_hu]

On 2016/9/29 18:35, Tejun Heo wrote:
> Hello,
> 
> On Sat, Sep 24, 2016 at 07:20:49AM +0800, zijun_hu wrote:
>> it is error to represent the max range max_distance spanned by all the
>> group areas as the offset of the highest group area plus unit size in
>> pcpu_embed_first_chunk(), it should equal to the offset plus the size
>> of the highest group area
>>
>> in order to fix this issue,let us find the highest group area who has the
>> biggest base address among all the ones, then max_distance is formed by
>> add it's offset and size value
> 
>  [PATCH] percpu: fix max_distance calculation in pcpu_embed_first_chunk()
> 
>  pcpu_embed_first_chunk() calculates the range a percpu chunk spans
>  into max_distance and uses it to ensure that a chunk is not too big
>  compared to the total vmalloc area.  However, during calculation, it
>  used incorrect top address by adding a unit size to the higest
>  group's base address.
> 
>  This can make the calculated max_distance slightly smaller than the
>  actual distance although given the scale of values involved the error
>  is very unlikely to have an actual impact.
> 
>  Fix this issue by adding the group's size instead of a unit size.
> 
>> the type of variant max_distance is changed from size_t to unsigned long
>> to prevent potential overflow
> 
> This doesn't make any sense.  All the values involved are valid
> addresses (or +1 of it), they can't overflow and size_t is the same
> size as ulong.
> 
>> @@ -2025,17 +2026,18 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
>>  	}
>>  
>>  	/* base address is now known, determine group base offsets */
>> -	max_distance = 0;
>> +	i = 0;
>>  	for (group = 0; group < ai->nr_groups; group++) {
>>  		ai->groups[group].base_offset = areas[group] - base;
>> -		max_distance = max_t(size_t, max_distance,
>> -				     ai->groups[group].base_offset);
>> +		if (areas[group] > areas[i])
>> +			i = group;
>>  	}
>> -	max_distance += ai->unit_size;
>> +	max_distance = ai->groups[i].base_offset +
>> +		(unsigned long)ai->unit_size * ai->groups[i].nr_units;
> 
> I don't think you need ulong cast here.
> 
> Thanks.
> 
okay, thanks for your reply
i will correct this in another patch