mm: unnecessary COW phenomenon

[Nadav Amit]

Andrea, Peter, others,

I encountered many unnecessary COW operations on my development kernel
(based on Linux 5.13), which I did not see a report about and I am not
sure how to solve. An advice would be appreciated.

Commit 09854ba94c6aa ("mm: do_wp_page() simplification”) prevents the reuse of
a page on write-protect fault if page_count(page) != 1. In that case,
wp_page_reuse() is not used and instead the page is COW'd by wp_page_copy
(). wp_page_copy() is obviously much more expensive, not only because of the
copying, but also because it requires a TLB flush and potentially a TLB
shootodwn.

The scenario I encountered happens when I use userfaultfd, but presumably it
might happen regardless of userfaultfd (perhaps swap device with
SWP_SYNCHRONOUS_IO). It involves two page faults: one that maps a new
anonymous page as read-only and a second write-protect fault that happens
shortly after on the same page. In this case the page count is almost always
elevated and therefore a COW is needed.

[ The specific scenario that I have as as follows: I map a page to the
monitored process using UFFDIO_COPY (actually a variant I am working on) as
write-protected. Then, shortly after an write access to the page triggers a
page fault. The uffd monitor quickly resolves the page fault using
UFFDIO_WRITEPROTECT. The kernel keeps the page write protected in the page
tables but marked logically as uffd-unprotected and the page table is
retried. The retry triggers a COW. ]

It turns out that the elevated page count is due to the caching of the page in
the local LRU cache (by lru_cache_add() which is called by
lru_cache_add_inactive_or_unevictable() in the case userfaultfd). Since the
first fault happened shortly before the second write-protect fault, the LRU
cache was still not drained, so the page count was not decreased and a COW is
needed.

Calling lru_add_drain() during this flow resolves the issue most of the time.
Obviously, it needs to be called on the core that allocated (i.e., faulted
in) the page initially to work. It is possible to do it conditionally only if
the page-count is greater than 1.

My questions to you (if I may) are:

1. Am I missing something?
2. Should it happen in other cases, specifically SWP_SYNCHRONOUS_IO?
3. Do you have a better solution?