mm: avoid unnecessary page fault retires on shared memory types
authorPeter Xu <peterx@redhat.com>
Mon, 30 May 2022 18:34:50 +0000 (14:34 -0400)
committerakpm <akpm@linux-foundation.org>
Fri, 17 Jun 2022 02:48:27 +0000 (19:48 -0700)
I observed that for each of the shared file-backed page faults, we're very
likely to retry one more time for the 1st write fault upon no page.  It's
because we'll need to release the mmap lock for dirty rate limit purpose
with balance_dirty_pages_ratelimited() (in fault_dirty_shared_page()).

Then after that throttling we return VM_FAULT_RETRY.

We did that probably because VM_FAULT_RETRY is the only way we can return
to the fault handler at that time telling it we've released the mmap lock.

However that's not ideal because it's very likely the fault does not need
to be retried at all since the pgtable was well installed before the
throttling, so the next continuous fault (including taking mmap read lock,
walk the pgtable, etc.) could be in most cases unnecessary.

It's not only slowing down page faults for shared file-backed, but also add
more mmap lock contention which is in most cases not needed at all.

To observe this, one could try to write to some shmem page and look at
"pgfault" value in /proc/vmstat, then we should expect 2 counts for each
shmem write simply because we retried, and vm event "pgfault" will capture
that.

To make it more efficient, add a new VM_FAULT_COMPLETED return code just to
show that we've completed the whole fault and released the lock.  It's also
a hint that we should very possibly not need another fault immediately on
this page because we've just completed it.

This patch provides a ~12% perf boost on my aarch64 test VM with a simple
program sequentially dirtying 400MB shmem file being mmap()ed and these are
the time it needs:

  Before: 650.980 ms (+-1.94%)
  After:  569.396 ms (+-1.38%)

I believe it could help more than that.

We need some special care on GUP and the s390 pgfault handler (for gmap
code before returning from pgfault), the rest changes in the page fault
handlers should be relatively straightforward.

Another thing to mention is that mm_account_fault() does take this new
fault as a generic fault to be accounted, unlike VM_FAULT_RETRY.

I explicitly didn't touch hmm_vma_fault() and break_ksm() because they do
not handle VM_FAULT_RETRY even with existing code, so I'm literally keeping
them as-is.

Link: https://lkml.kernel.org/r/20220530183450.42886-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Vineet Gupta <vgupta@kernel.org>
Acked-by: Guo Ren <guoren@kernel.org>
Acked-by: Max Filippov <jcmvbkbc@gmail.com>
Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Alistair Popple <apopple@nvidia.com>
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> [arm part]
Acked-by: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Stafford Horne <shorne@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Brian Cain <bcain@quicinc.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Richard Weinberger <richard@nod.at>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Janosch Frank <frankja@linux.ibm.com>
Cc: Albert Ou <aou@eecs.berkeley.edu>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Will Deacon <will@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Chris Zankel <chris@zankel.net>
Cc: Hugh Dickins <hughd@google.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Rich Felker <dalias@libc.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Helge Deller <deller@gmx.de>
Cc: Yoshinori Sato <ysato@users.osdn.me>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
26 files changed:
arch/alpha/mm/fault.c
arch/arc/mm/fault.c
arch/arm/mm/fault.c
arch/arm64/mm/fault.c
arch/csky/mm/fault.c
arch/hexagon/mm/vm_fault.c
arch/ia64/mm/fault.c
arch/m68k/mm/fault.c
arch/microblaze/mm/fault.c
arch/mips/mm/fault.c
arch/nios2/mm/fault.c
arch/openrisc/mm/fault.c
arch/parisc/mm/fault.c
arch/powerpc/mm/copro_fault.c
arch/powerpc/mm/fault.c
arch/riscv/mm/fault.c
arch/s390/mm/fault.c
arch/sh/mm/fault.c
arch/sparc/mm/fault_32.c
arch/sparc/mm/fault_64.c
arch/um/kernel/trap.c
arch/x86/mm/fault.c
arch/xtensa/mm/fault.c
include/linux/mm_types.h
mm/gup.c
mm/memory.c

index ec20c1004abf5a8e4695ec23147b79b58a56dfd4..ef427a6bdd1ab91be0445b0261d38bb49bb2be93 100644 (file)
@@ -155,6 +155,10 @@ retry:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index dad27e4d69ff1af6d68313a982802aa101808613..5ca59a482632a880f0a40d31c44d9b1159581b85 100644 (file)
@@ -146,6 +146,10 @@ retry:
                return;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        /*
         * Fault retry nuances, mmap_lock already relinquished by core mm
         */
index a062e07516dd2289b9800fbf7ecdd21eef7b0afe..46cccd6bf705a6745feefb5d58c9e4e2733b31c1 100644 (file)
@@ -322,6 +322,10 @@ retry:
                return 0;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return 0;
+
        if (!(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_RETRY) {
                        flags |= FAULT_FLAG_TRIED;
index c5e11768e5c141ae854ef3b71cfbf8a1ca17d907..de166cdeb89a79be70d83fceb6e601f4c0ebf6b2 100644 (file)
@@ -608,6 +608,10 @@ retry:
                return 0;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return 0;
+
        if (fault & VM_FAULT_RETRY) {
                mm_flags |= FAULT_FLAG_TRIED;
                goto retry;
index 7215a46b6b8eb8ee4fe81b781decafa75064b4e1..e15f736cca4b4a43fb14989e099051fbdbf0a082 100644 (file)
@@ -285,6 +285,10 @@ good_area:
                return;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely((fault & VM_FAULT_RETRY) && (flags & FAULT_FLAG_ALLOW_RETRY))) {
                flags |= FAULT_FLAG_TRIED;
 
index 4fac4b9eb3164f20444e19305aa32abf35d6773a..f73c7cbfe32603c425269f80af6e767bd212ad68 100644 (file)
@@ -96,6 +96,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        /* The most common case -- we are done. */
        if (likely(!(fault & VM_FAULT_ERROR))) {
                if (fault & VM_FAULT_RETRY) {
index 07379d1a227fe83318c108ca3f03beb285ce6733..ef78c2d66cdde243b8bb1887054faab51b13d838 100644 (file)
@@ -139,6 +139,10 @@ retry:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                /*
                 * We ran out of memory, or some other thing happened
index 71aa9f6315dc8028dcb17243d6e63df0068119ec..4d2837eb3e2a3eea5b5523c3f9e79b8e11be580a 100644 (file)
@@ -141,6 +141,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return 0;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return 0;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index a9626e6a68af93f34962129870c5d5c91cee40a0..5c40c3ebe52f770522b7e98b3c3ca327aec1d94b 100644 (file)
@@ -222,6 +222,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index b08bc556d30dd779d973f30754f44a491d6b2038..a27045f5a556dfead73cef3c54151674a8565152 100644 (file)
@@ -162,6 +162,10 @@ good_area:
                return;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index a32f14cd72f2891b5d7856fee5b98f8283a08cae..edaca0a6c1c1ca5bc467237e4235bdcd9dba1748 100644 (file)
@@ -139,6 +139,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index 53b760af3bb75b6ce12ad738428b723558be3f6a..b4762d66e9efe83de2487214053f45d2f0ebf664 100644 (file)
@@ -165,6 +165,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index 84bc437be5cd1f4d5efea94d024389650d6bd46a..9ad80d4d3389cf7711b594ca6ec67dd41dba4543 100644 (file)
@@ -311,6 +311,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                /*
                 * We hit a shared mapping outside of the file, or some
index c1cb21a008843dde20ebe82c907286a7599407d7..7c507fb48182bd461da33036b67cdcef946f93fb 100644 (file)
@@ -65,6 +65,11 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
 
        ret = 0;
        *flt = handle_mm_fault(vma, ea, is_write ? FAULT_FLAG_WRITE : 0, NULL);
+
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (*flt & VM_FAULT_COMPLETED)
+               return 0;
+
        if (unlikely(*flt & VM_FAULT_ERROR)) {
                if (*flt & VM_FAULT_OOM) {
                        ret = -ENOMEM;
index d53fed4eccbd3621bc8123f91d4552739c474335..01400542868731439de4b6a3ff20c058215cc2d2 100644 (file)
@@ -511,6 +511,10 @@ retry:
        if (fault_signal_pending(fault, regs))
                return user_mode(regs) ? 0 : SIGBUS;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               goto out;
+
        /*
         * Handle the retry right now, the mmap_lock has been released in that
         * case.
@@ -525,6 +529,7 @@ retry:
        if (unlikely(fault & VM_FAULT_ERROR))
                return mm_fault_error(regs, address, fault);
 
+out:
        /*
         * Major/minor page fault accounting.
         */
index 40694f0cab9e51c8f4cb1a6dae6d7545e48d8dd6..f2fbd1400b7c9ade4fffd340a64dc96597e4c729 100644 (file)
@@ -326,6 +326,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_RETRY)) {
                flags |= FAULT_FLAG_TRIED;
 
index e173b6187ad5631f22f5821f27f7e9c134c2f6b4..973dcd05c2935256156e871bd7dd08946f9f8f65 100644 (file)
@@ -433,6 +433,17 @@ retry:
                        goto out_up;
                goto out;
        }
+
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED) {
+               if (gmap) {
+                       mmap_read_lock(mm);
+                       goto out_gmap;
+               }
+               fault = 0;
+               goto out;
+       }
+
        if (unlikely(fault & VM_FAULT_ERROR))
                goto out_up;
 
@@ -452,6 +463,7 @@ retry:
                mmap_read_lock(mm);
                goto retry;
        }
+out_gmap:
        if (IS_ENABLED(CONFIG_PGSTE) && gmap) {
                address =  __gmap_link(gmap, current->thread.gmap_addr,
                                       address);
index e175667b13637907d2b4825237d800a20c5d5f18..acd2f5e50bfcd08ccb71fc71a8f33a1c9cb84c06 100644 (file)
@@ -485,6 +485,10 @@ good_area:
                if (mm_fault_error(regs, error_code, address, fault))
                        return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (fault & VM_FAULT_RETRY) {
                flags |= FAULT_FLAG_TRIED;
 
index ad569d9bd1242cde74ea530aafffb48cd95d72db..91259f291c54078541d59e39e89de7c7933c99ad 100644 (file)
@@ -190,6 +190,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                return;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index 253e07043298b73a1fd5d99119f05a3eeec160af..4acc12eafbf54da90987b7e9d7445fa69576386a 100644 (file)
@@ -427,6 +427,10 @@ good_area:
        if (fault_signal_pending(fault, regs))
                goto exit_exception;
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               goto lock_released;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
@@ -449,6 +453,7 @@ good_area:
        }
        mmap_read_unlock(mm);
 
+lock_released:
        mm_rss = get_mm_rss(mm);
 #if defined(CONFIG_TRANSPARENT_HUGEPAGE)
        mm_rss -= (mm->context.thp_pte_count * (HPAGE_SIZE / PAGE_SIZE));
index d1d5d0be0308561d13e5d6a9a0379140a77d9490..d3ce21c4ca32a8391de6b5372960ff461be1fe87 100644 (file)
@@ -76,6 +76,10 @@ good_area:
                if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current))
                        goto out_nosemaphore;
 
+               /* The fault is fully completed (including releasing mmap lock) */
+               if (fault & VM_FAULT_COMPLETED)
+                       return 0;
+
                if (unlikely(fault & VM_FAULT_ERROR)) {
                        if (fault & VM_FAULT_OOM) {
                                goto out_of_memory;
index fad8faa29d042d59ab9ae0f6d89d7aaee5b8a041..fe10c6d76bac789d4a8462e23455cb618a270ed4 100644 (file)
@@ -1408,6 +1408,10 @@ good_area:
                return;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        /*
         * If we need to retry the mmap_lock has already been released,
         * and if there is a fatal signal pending there is no guarantee
index 16f0a5ff57991c1135382594de7bb2ce5197cd70..8c781b05c0bdd9d55d62ff456c8150b94cf9d493 100644 (file)
@@ -172,6 +172,10 @@ good_area:
                return;
        }
 
+       /* The fault is fully completed (including releasing mmap lock) */
+       if (fault & VM_FAULT_COMPLETED)
+               return;
+
        if (unlikely(fault & VM_FAULT_ERROR)) {
                if (fault & VM_FAULT_OOM)
                        goto out_of_memory;
index c29ab4c0cd5c661d3b7d13a377c1c36dd72c07ce..6b961a29bf26f7808fbcfa57f520a1387dd07d9d 100644 (file)
@@ -729,6 +729,7 @@ typedef __bitwise unsigned int vm_fault_t;
  * @VM_FAULT_NEEDDSYNC:                ->fault did not modify page tables and needs
  *                             fsync() to complete (for synchronous page faults
  *                             in DAX)
+ * @VM_FAULT_COMPLETED:                ->fault completed, meanwhile mmap lock released
  * @VM_FAULT_HINDEX_MASK:      mask HINDEX value
  *
  */
@@ -746,6 +747,7 @@ enum vm_fault_reason {
        VM_FAULT_FALLBACK       = (__force vm_fault_t)0x000800,
        VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000,
        VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000,
+       VM_FAULT_COMPLETED      = (__force vm_fault_t)0x004000,
        VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
 };
 
index 5512644076246d9baaa4873c9019cde4e17a2e37..407a81d5ca030506d25efbea98902c1179344cca 100644 (file)
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -951,6 +951,25 @@ static int faultin_page(struct vm_area_struct *vma,
        }
 
        ret = handle_mm_fault(vma, address, fault_flags, NULL);
+
+       if (ret & VM_FAULT_COMPLETED) {
+               /*
+                * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
+                * mmap lock in the page fault handler. Sanity check this.
+                */
+               WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
+               if (locked)
+                       *locked = 0;
+               /*
+                * We should do the same as VM_FAULT_RETRY, but let's not
+                * return -EBUSY since that's not reflecting the reality of
+                * what has happened - we've just fully completed a page
+                * fault, with the mmap lock released.  Use -EAGAIN to show
+                * that we want to take the mmap lock _again_.
+                */
+               return -EAGAIN;
+       }
+
        if (ret & VM_FAULT_ERROR) {
                int err = vm_fault_to_errno(ret, *flags);
 
@@ -1177,6 +1196,7 @@ retry:
                        case 0:
                                goto retry;
                        case -EBUSY:
+                       case -EAGAIN:
                                ret = 0;
                                fallthrough;
                        case -EFAULT:
@@ -1303,6 +1323,18 @@ retry:
                return -EINTR;
 
        ret = handle_mm_fault(vma, address, fault_flags, NULL);
+
+       if (ret & VM_FAULT_COMPLETED) {
+               /*
+                * NOTE: it's a pity that we need to retake the lock here
+                * to pair with the unlock() in the callers. Ideally we
+                * could tell the callers so they do not need to unlock.
+                */
+               mmap_read_lock(mm);
+               *unlocked = true;
+               return 0;
+       }
+
        if (ret & VM_FAULT_ERROR) {
                int err = vm_fault_to_errno(ret, 0);
 
@@ -1368,7 +1400,7 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm,
                        /* VM_FAULT_RETRY couldn't trigger, bypass */
                        return ret;
 
-               /* VM_FAULT_RETRY cannot return errors */
+               /* VM_FAULT_RETRY or VM_FAULT_COMPLETED cannot return errors */
                if (!*locked) {
                        BUG_ON(ret < 0);
                        BUG_ON(ret >= nr_pages);
index 7a089145cad4b2b2fd32b47390277760eb15cc18..580c62febe42eb8ea06b35b7de27de68c92dec89 100644 (file)
@@ -3020,7 +3020,7 @@ static vm_fault_t fault_dirty_shared_page(struct vm_fault *vmf)
                balance_dirty_pages_ratelimited(mapping);
                if (fpin) {
                        fput(fpin);
-                       return VM_FAULT_RETRY;
+                       return VM_FAULT_COMPLETED;
                }
        }