Re: [PATCH] fix madvise vma merging

[Prasanna Meda <pmeda@akamai.com>]

Hugh Dickins wrote:

> Better late than never, I've at last reviewed the madvise vma merging
> going into 2.6.13.  Remove a pointless check and fix two little bugs -
> a simple test (with /proc/<pid>/maps hacked to show ReadHints) showed
> both mismerges in practice: though being madvise, neither was disastrous.
>
> 1. Correct placement of the success label in madvise_behavior: as in
>    mprotect_fixup and mlock_fixup, it is necessary to update vm_flags
>    when vma_merge succeeds (to handle the exceptional Case 8 noted in
>    the comments above vma_merge itself).
>
> 2. Correct initial value of prev when starting part way into a vma: as
>    in sys_mprotect and do_mlock, it needs to be set to vma in this case
>    (vma_merge handles only that minimum of cases shown in its comments).
>
> 3. If find_vma_prev sets prev, then the vma it returns is prev->vm_next,
>    so it's pointless to make that same assignment again in sys_madvise.

Acknowledge corrections 1 and  3 readily.  Treated vma_merge
as block box that can handle all cases.  Motivation for pointless
case 3 is to skip holes and did not notice that has been covered.  Thanks for
corrections.

>  (vma_merge handles only that minimum of cases shown in its comments).
>

Correction 2 is tricky.  Sometimes it merges similar to case 3,
misses a needed split,  where after the fix  we can get case 4
merge. If that is what you are saying, we are in agreement.  Otherwise,
can you explain the real problem?


Thanks,
Prasanna.