On Wed, Oct 14, 2020 at 07:15:09PM +0100, Matthew Wilcox wrote:
> On Wed, Oct 14, 2020 at 09:53:35AM -0700, Linus Torvalds wrote:
> > In particular, what I _think_ we could do is:
> >
> > - lock the page tables
> >
> > - check that the page isn't locked
> >
> > - increment the page mapcount (atomic and ordered)
> >
> > - check that the page still isn't locked
> >
> > - insert pte
> >
> > without taking the page lock. And the reason that's safe is that *if*
> [...]
> > And they aren't necessarily a _lot_ more involved. In fact, I think we
> > may already hold the page table lock due to doing that
> > "pte_alloc_one_map()" thing over all of filemap_map_pages(). So I
> > think the only _real_ problem is that I think we increment the
> > page_mapcount() too late in alloc_set_pte().
>
> I'm not entirely sure why we require the page lock to be held in
> page_add_file_rmap():
>
> } else {
> if (PageTransCompound(page) && page_mapping(page)) {
> VM_WARN_ON_ONCE(!PageLocked(page));
>
> SetPageDoubleMap(compound_head(page));
> if (PageMlocked(page))
> clear_page_mlock(compound_head(page));
> }
>
> We have a reference to the page, so compound_head() isn't going
> to change. SetPageDoubleMap() is atomic. PageMlocked() is atomic.
> clear_page_mlock() does TestClearPageMlocked() as its first thing,
> so that's atomic too. What am I missing? (Kirill added it, so I
> assume he remembers ;-)
In general (with current scheme), we need page lock here to serialize
against try_to_unmap() and other rmap walkers.
For file THP, we also serialize against follow_trans_huge_pmd(FOLL_MLOCK).
--
Kirill A. Shutemov
