On Sun, 29 May 2011, Peter Zijlstra wrote:
> On Sat, 2011-05-28 at 17:12 -0700, Hugh Dickins wrote:
> > I believe that although it may no longer be the anon_vma that the page
> > is pointing to, it remains stable. Because even if page->anon_vma is
> > updated, it will certainly have the same anon_vma->root as before
> > (see the first BUG_ON in __page_check_anon_rmap() for reassurance),
> > so the mutex locking holds good.
> >
> > And the structure itself won't be freed: although the page is now
> > pointing to a less inclusive, more optimal anon_vma for reclaim to use,
> > the anon_vma which was originally pointed to remains on the same vma's
> > chains as it ever was, and only gets freed up when they're all gone.
> >
> > So, when there's this race with moving anon_vma, page_lock_anon_vma()
> > may end up returning a less than optimal anon_vma, but it's still valid
> > as a good though longer list of vmas to look through.
>
> Yes, and I think I see what you mean, if a page's anon_vma is changed
> while it remains mapped it will only ever be moved to a child of the
> original anon_vma. And because of the anon_vma ref-counting, the
> original anon_vma will stick around until that too is dead, which won't
> happen for as long as the page remains mapped.
Child or grandchild or more remote descendent, I think, yes.
Actually, it's not the anon_vma ref-counting that keeps them around
generally: I think it's the way we keep all those anon_vmas linked
to their vmas - the anon_vma will be freed only when all its possible
vmas have been unmapped. Just like when we didn't have ref-counting,
and just like before the anon_vma_chains.
For a while I thought that, if we were careful about the ordering of
the lists, always freeing root last, we could have a naughty patch
which even removes the additional counting on anon_vma->root.
But no, precisely because there is some ref-counting, which may hold
any anon_vma for a while, we cannot enforce such ordering and do need
additional holds on the root.
>
> Therefore, for as long as we observe page_mapped(), any anon_vma
> obtained from it remains valid.
>
> Talk about tricky.. shees. I bet that wants a comment or so.
I don't think anybody understood how this was working, until you
forced us to think about it yesterday: thanks a lot for doing so.
>
> > The previous code would have broken horribly, wouldn't it, were that
> > not the case?
>
> It would have, yes.
>
> ---
> Subject: mm, rmap: Add yet more comments to page_get_anon_vma/page_lock_anon_vma
>
> Inspired by an analysis from Hugh on why again all this doesn't explode
> in our face.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>
> ---
> mm/rmap.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 6bada99..487d5cc 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -350,7 +350,12 @@ void __init anon_vma_init(void)
> * have been relevant to this page.
> *
> * The page might have been remapped to a different anon_vma or the anon_vma
> - * returned may already be freed (and even reused).
> + * returned may already be freed (and even reused).
> + *
> + * In case it was remapped to a different anon_vma, the new anon_vma will be a
> + * child of the old anon_vma, and the anon_vma lifetime rules will therefore
> + * ensure that any anon_vma obtained from the page will still be valid for as
> + * long as we observe page_mapped() [ hence all those page_mapped() tests ].
> *
> * All users of this function must be very careful when walking the anon_vma
> * chain and verify that the page in question is indeed mapped in it
> @@ -421,7 +426,7 @@ struct anon_vma *page_lock_anon_vma(struct page *page)
> /*
> * If the page is still mapped, then this anon_vma is still
> * its anon_vma, and holding the mutex ensures that it will
> - * not go away, see __put_anon_vma().
> + * not go away, see anon_vma_free().
> */
> if (!page_mapped(page)) {
> mutex_unlock(&root_anon_vma->mutex);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx 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=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>