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

Linus Torvalds
2013-11-01T17:47:36Z

On Fri, Nov 1, 2013 at 7:09 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> 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