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
