On Wed, Nov 9, 2011 at 9:25 AM, Andrea Arcangeli <aarcange@xxxxxxxxxx> wrote: > On Mon, Nov 07, 2011 at 04:28:08PM +0000, Mel Gorman wrote: >> Note that I didn't suddenly turn that ack into a nack although > > :) > >> 1) A small comment on why we need to call anon_vma_moveto_tail in the >> error path would be nice > > I can add that. > >> 2) It is unfortunate that we need the faulted_in_anon_vma just >> for a VM_BUG_ON check that only exists for CONFIG_DEBUG_VM >> but not earth shatting > > It should be optimized away at build time. It thought it was better > not to leave that path without a VM_BUG_ON. It should be a slow path > in the first place (probably we should even mark it unlikely). And > it's obscure enough that I think a check will clarify things. In the > common case (i.e. some pte faulted in) that vma_merge on self if it > succeeds, it couldn't possibly be safe because the vma->vm_pgoff vs > page->index linearity couldn't be valid for the same vma and the same > page on two different virtual addresses. So checking for it I think is > sane. Especially given at some point it was mentioned we could > optimize away the check all together, so it's a bit of an obscure path > that the VM_BUG_ON I think will help document (and verify). > >> What I said was taking the anon_vma lock may be slower but it was >> generally easier to understand. I'm happy with the new patch too >> particularly as it keeps the "ordering game" consistent for fork >> and mremap but I previously missed move_page_tables in the error >> path so was worried if there was something else I managed to miss >> particularly in light of the "Allocating a new vma, copy first and >> merge later" direction. > > I liked that direction a lot. I thought with that we could stick to > the exact same behavior of fork and not need to reorder stuff. But the > error path is still in the way, and we've to undo the move in place > without tearing down the vmas. Plus it would have required to write > mode code, and the allocation path wouldn't have necessarily been > faster than a reordering if the list is not huge. > >> I'm also prefectly happy with my human meat brain and do not expect >> to replace it with an aliens. > > 8-) > > On a totally different but related topic, unmap_mapping_range_tree > walks the prio tree the same way try_to_unmap_file walks it and if > truncate can truncate "dst" before "src" then supposedly the > try_to_unmap_file could miss a migration entry copied into the "child" > ptep while fork runs too... But I think there is no risk there because > we don't establish migration ptes there, and we just unmap the > pagecache, so worst case we'll abort migration if the race trigger and > we'll retry later. But I wonder what happens if truncate runs against > fork, if truncate can drop ptes from dst before src (like mremap > comment says), we could still end up with some pte mapped to the file > in the ptes of the child, even if the pte was correctly truncated in > the parent... > > Overall I think fork/mremap vs fully_reliable_rmap_walk/truncate > aren't fundamentally different in relation. If we relay on ordering > for anon pages in fork it's not adding too much mess to also relay on > ordering for mremap. If we take the i_mmap_mutex in mremap because we > can't enforce a order in the prio tree, then we need the i_mmap_mutex > in fork too (and that's missing). But nothing prevents us to use a > lock in mreamp and ordering in fork. I think the decision should be > based more on performance expectations. > > So we could add the ordering to mremap (patch posted), and add the > i_mmap_mutex to fork, or we add the anon_vma lock in both mremap and > fork, and the i_mmap_lock to fork. > > Also note, if we find a way to enforce orderings in the prio tree (not > sure if it's possible, apparently it's already using list_add_tail > so..), then we could also remove the i_mmap_lock from mremap and fork. > Oh, well, I had thought that for partial remap the src and dst VMA are inserted as different prio tree nodes, instead of being list_add_tail linked, which means they can not be reordered back and force at all... -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href