On Sat, Jan 05, 2013 at 06:24:47AM -0600, Simon Jeons wrote:

On Fri, 2013-01-04 at 17:32 -0800, Michel Lespinasse wrote:

quoted

On Fri, Jan 4, 2013 at 6:08 AM, Mel Gorman [off-list ref] wrote:

quoted

Despite the reason for these commits, NUMA balancing is not the direct
source of the problem. split_huge_page() expected the anon_vma lock to be
exclusive to serialise the whole split operation. Ordinarily it is expected
that the anon_vma lock would only be required when updating the avcs but
THP also uses it. The locking requirements for THP are complex and there
is some overlap but broadly speaking they include the following

1. mmap_sem for read or write prevents THPs being created underneath
2. anon_vma is taken for write if collapsing a huge page
3. mm->page_table_lock should be taken when checking if pmd_trans_huge as
   split_huge_page can run in parallel
4. wait_split_huge_page uses anon_vma taken for write mode to serialise
   against other THP operations
5. compound_lock is used to serialise between
   __split_huge_page_refcount() and gup

split_huge_page takes anon_vma for read but that does not serialise against
parallel split_huge_page operations on the same page (rule 2). One process
could be modifying the ref counts while the other modifies the page tables
leading to counters not being reliable. This patch takes the anon_vma
lock for write to serialise against parallel split_huge_page and parallel
collapse operations as it is the most fine-grained lock available that
protects against both.

Your comment about this being the most fine-grained lock made me
think, couldn't we use lock_page() on the THP page here ?

Now I don't necessarily want to push you that direction, because I
haven't fully thought it trough and because what you propose brings us
closer to what happened before anon_vma became an rwlock, which is
more obviously safe. But I felt I should still mention it, since we're
really only trying to protect from concurrent operations on the same
THP page, so locking at just that granularity would seem desirable.

Why you said that anon_vma lock who will protect page associated to a
list of vmas is fine-grained then page lock who just protect one page?

We did not say it was fine-grained, we said it was the most fine-grained
lock available. It's a coarse lock but using the page lock would be
problematic.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>