Re: [PATCH] mm: reuse the unshared swapcache page in do_wp_page

[Matthew Wilcox <willy@infradead.org>]

On Thu, Jan 20, 2022 at 04:51:47PM +0100, David Hildenbrand wrote:
> 
> >> Yes, we are talking past each other and no, I am talking about fully
> >> mapped THP, just mapped via PTEs.
> >>
> >> Please refer to our THP COW logic: do_huge_pmd_wp_page()
> > 
> > You're going to have to be a bit more explicit.  That's clearly handling
> > the case where there's a PMD mapping.  If there is _also_ a PTE mapping,
> > then obviously the page is mapped more than once and can't be reused!
> > 
> >>>
> >>>>> Anon THP is always going to start out aligned (and can be moved by
> >>>>> mremap()).  Arguably it should be broken up if it's moved so it can be
> >>>>> reformed into aligned THPs by khugepaged.
> >>>>
> >>>> Can you elaborate, I'm missing the point where something gets moved. I
> >>>> don't care about mremap() at all here.
> >>>>
> >>>>
> >>>> 1. You have a read-only, PTE mapped THP
> >>>> 2. Write fault on the THP
> >>>> 3. We PTE-map the THP because we run into a false positive in our COW
> >>>>    logic to handle COW on PTE
> >>>> 4. Write fault on the PTE
> >>>> 5. We always have to COW each and every sub-page and can never reuse,
> >>>>    because page_count() > 1
> >>>>
> >>>> That's essentially what reuse_swap_page() tried to handle before.
> >>>> Eventually optimizing for this is certainly the next step, but I'd like
> >>>> to document which effect the removal of reuse_swap_page() will have to THP.
> >>>
> >>> I'm talking about step 0.  How do we get a read-only, PTE-mapped THP?
> >>> Through mremap() or perhaps through an mprotect()/mmap()/munmap() that
> >>> failed to split the THP.
> >>
> >> do_huge_pmd_wp_page()
> > 
> > I feel you could be a little more verbose about what you think is
> > going on here.  Are you talking about the fallback: path where we
> > call __split_huge_pmd()?
> 
> Sorry, I was less verbose because I was just sending out the
> patch+description to Linus' reply and was assuming you're going to read
> it anyways ;)

This reply arrived before your reply to Linus ;-)  Anyway ...

> Yes, I'm speaking about exactly that fallback path.

OK, so in that fallback path, we're already determined the THP has
more than one reference to it (ok, maybe that extra reference was
temporary and now gone), but we've already split the PMD down into
PTEs, and COWed one of the other pages that was in the THP.  If
anything, we should be more aggressive about COWing the remaining
pages in the THP, not looking for reasons why we might be able to
avoid COWing this particular page.