lkml.org 
[lkml]   [2010]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -v2] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA


On Mon, 12 Apr 2010, Rik van Riel wrote:

> On 04/12/2010 12:01 PM, Peter Zijlstra wrote:
>
> > @@ -864,15 +889,8 @@ void page_remove_rmap(struct page *page)
> > __dec_zone_page_state(page, NR_FILE_MAPPED);
> > mem_cgroup_update_file_mapped(page, -1);
> > }
> > - /*
> > - * It would be tidy to reset the PageAnon mapping here,
> > - * but that might overwrite a racing page_add_anon_rmap
> > - * which increments mapcount after us but sets mapping
> > - * before us: so leave the reset to free_hot_cold_page,
> > - * and remember that it's only reliable while mapped.
> > - * Leaving it set also helps swapoff to reinstate ptes
> > - * faster for those pages still in swapcache.
> > - */
> > +
> > + page->mapping = NULL;
> > }
>
> That would be a bug for file pages :)
>
> I could see how it could work for anonymous memory, though.

I think it's scary for anonymous pages too. The _common_ case of
page_remove_rmap() is from unmap/exit, which holds no locks on the page
what-so-ever. So assuming the page could be reachable some other way (swap
cache etc), I think the above is pretty scary.

Also do note that the bug we've been chasing has _always_ had that test
for "page_mapped(page)". See my other email about why the unmapped case
isn't even interesting, because it's so easy to see how page->mapping can
be stale for unmapped pages.

It's the _mapped_ case that is interesting, not the unmapped one. So
setting page->mapping to NULL when unmapping is perhaps a nice consistency
issue ("never have stale pointers"), but it's missing the fact that it's
not really the case we care about.

Linus


\
 
 \ /
  Last update: 2010-04-12 18:53    [W:1.518 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site