Re: [PATCH] mm/mmap.c: Remove useless statement "vma = NULL" in find_vma()

[Chen Gang <xili_gchen_5257@hotmail.com>]

On 9/10/15 00:26, Oleg Nesterov wrote:
> On 09/08, Chen Gang wrote:
>>
>> I also want to consult: the comments of find_vma() says:
>
> Sorry, I don't understand the question ;)
>
>> "Look up the first VMA which satisfies addr < vm_end, ..."
>>
>> Is it OK?
>
> Why not?
>

We will continue discuss about it below. Please help check, thanks.

>> (why not "vm_start <= addr < vm_end"),
>
> Because this some callers actually want to find the 1st vma which
> satisfies addr < vm_end? For example, shift_arg_pages().
>
> OTOH, I think that another helper,
>
> find_vma_xxx(mm, addr)
> {
> vma = find_vma(...)
> if (vma && vma->vm_start> addr)
> vma = NULL;
> return vma;
> }
>
> makes sense. It can have a lot of users.
>

OK. thank you very much. :-)

>> need we let "vma = tmp"
>> in "if (tmp->vm_start <= addr)"? -- it looks the comments is not match
>> the implementation, precisely (maybe not 1st VMA).
>
> This contradicts with above... I mean, it is not clear what exactly do
> you blame, semantics or implementation.
>
> The implementation looks correct. Why do you think it can be not 1st vma?
>

It is in while (rb_node) {...}.

- When we set "vma = tmp", it is alreay match "addr < vm_end".

- If "addr>= vm_start", we return this vma (else continue searching).

If "the first left" is the real first, when "addr>= vm_start", it
will return (may not return 1st left matched vma).

If "the first find" is the real first, when "addr < vm_start", it
will continue searching (may not return 1st find matched vma).

For me, if we only focus on "addr < vm_end", we need remove "vm_start <=
addr" checking). If we have to consider about "addr>= vm_start", we may
need additional parameter or implement a new function for it.


Welcome any ideas, suggestions and completions.


Thanks.
--
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed