Hari Bathini <hbath...@linux.ibm.com> writes:

> On 15/07/20 8:09 am, Thiago Jung Bauermann wrote:
>> 
>> Hari Bathini <hbath...@linux.ibm.com> writes:
>> 
>
> <snip>
>  
>>> +/**
>>> + * __locate_mem_hole_top_down - Looks top down for a large enough memory 
>>> hole
>>> + *                              in the memory regions between buf_min & 
>>> buf_max
>>> + *                              for the buffer. If found, sets kbuf->mem.
>>> + * @kbuf:                       Buffer contents and memory parameters.
>>> + * @buf_min:                    Minimum address for the buffer.
>>> + * @buf_max:                    Maximum address for the buffer.
>>> + *
>>> + * Returns 0 on success, negative errno on error.
>>> + */
>>> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
>>> +                                 u64 buf_min, u64 buf_max)
>>> +{
>>> +   int ret = -EADDRNOTAVAIL;
>>> +   phys_addr_t start, end;
>>> +   u64 i;
>>> +
>>> +   for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
>>> +                          MEMBLOCK_NONE, &start, &end, NULL) {
>>> +           if (start > buf_max)
>>> +                   continue;
>>> +
>>> +           /* Memory hole not found */
>>> +           if (end < buf_min)
>>> +                   break;
>>> +
>>> +           /* Adjust memory region based on the given range */
>>> +           if (start < buf_min)
>>> +                   start = buf_min;
>>> +           if (end > buf_max)
>>> +                   end = buf_max;
>>> +
>>> +           start = ALIGN(start, kbuf->buf_align);
>>> +           if (start < end && (end - start + 1) >= kbuf->memsz) {
>> 
>> This is why I dislike using start and end to express address ranges:
>> 
>> While struct resource seems to use the [address, end] convention, my
>
> struct crash_mem also uses [address, end] convention.
> This off-by-one error did not cause any issues as the hole start and size we 
> try to find
> are at least page aligned.
>
> Nonetheless, I think fixing 'end' early in the loop with "end -= 1" would 
> ensure
> correctness while continuing to use the same convention for structs crash_mem 
> & resource.

Sounds good.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Reply via email to