Messages in this thread | | | From | KOSAKI Motohiro <> | Subject | Re: [PATCH 02/13] mm: Revalidate anon_vma in page_lock_anon_vma() | Date | Fri, 9 Apr 2010 11:19:52 +0900 (JST) |
| |
Hi
> > - Then checks page_mapped() without having any apparent defence > > against page_mapped() becoming untrue one nanosecond later. > > > > - Checks page_mapped() inside the rcu_read_locked() section for > > inscrutable reasons. > > Right, I think the page_mapped() stuff is just an early bail out.
FWIW, not only early bail out. page_remove_rmap() don't have "page->mapping = NULL". page_remove_rmap's comment says
/* * 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. */
So, If following scenario happen, we can't dereference page->mapping (iow, can't call spin_lock(&anon_vma->lock)). It might be pointed invalid address.
CPU0 CPU1 =============================================================== page->mappings become -1 anon_vma_unlink() -- grace period -- page_lock_anon_vma page_mapped() spin_lock(&anon_vma->lock);
Of cource, this statement doesn't mean I'm against your patch at all. I like it.
| |