Messages in this thread | | | Date | Sat, 6 Aug 2005 11:37:06 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: [PATCH] fix madvise vma merging |
| |
On Fri, 5 Aug 2005, Prasanna Meda wrote: > Hugh Dickins wrote: > > > 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). > > 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.
And thanks for the confirmations.
> 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?
I probably am saying what you are saying there, but it's hard for me to understand it that way.
Missing out the "start > vma->vm_start" adjustment of prev introduces additional (but redundant: non-canonical) cases not considered at all by vma_merge, now entered with a "prev" which is remote and surely irrelevant to merging. "misses a needed split", yes, I saw that; indeed my test ended up taking the "cases 3, 8" path, when, given the right prev, it should have been handled as a "case 4".
mmap(0x80000000, 0x3000, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, fd, 0); madvise(0x80000000, 0x2000, MADV_RANDOM); madvise(0x80001000, 0x1000, MADV_NORMAL);
ended up (if I'm remembering it aright) as one single VM_RAND_READ vma, when it should have been a one page VM_RAND_READ vma followed by a two page normal vma. Merged into one because of the incorrect prev, and VM_RAND_READ rather than normal because of the misplaced success label.
Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |