Re: [PATCH 1/4] mm/rmap: per anon_vma lock

From: Linus Torvalds
Date: Fri Nov 01 2013 - 13:47:38 EST


On Fri, Nov 1, 2013 at 7:09 AM, Yuanhan Liu <yuanhan.liu@xxxxxxxxxxxxxxx> wrote:
>
> Sorry, I may be wrong again this time. But, isn't vma->anon_vma_chain
> list being protect by mmap_sem & page_table_lock?

No.

The mmap_sem (and the page_table_lock) only protects a single VM instance.

The anon_vma chains, in contrast, can span multiple VM instances,
since it grows over fork().

Now, you're right that we used to not always lock the root every time,
but the code has changed since. We used to lock each anon_vma as we
copied them, now we lock the root only once (see anon_vma_clone()).
And that works exactly because they all share the same root anon_vma
on the chain. You can't change that to just randomly lock one non-root
vma.

So I'll add my voice to the chorus that says "that's wrong", when you
changed "anon_vma->root->lock" to "anon_vma->lock".

We *may* be able to do finer-grained anon_vma locking again, but it's
definitely not some mindless "let's just change it back" thing. It
needs real code changes (and I bet it's not just anon_vma_clone()),
and it needs a lot of care.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/