Since UVM has been imported, we zap mappings associated to anon pages
before deactivating or freeing them.  Sadly, with the introduction of
locking for amaps & anons, I added new code paths that do not respect
this behavior.
The diff below restores it by moving the call to pmap_page_protect()
inside uvm_anon_release().  With it the 3 code paths using the function
are now coherent with the rest of UVM.

I remember a discussion we had questioning the need for zapping such
mappings.  I'm interested in hearing more arguments for or against this
change. However, right now, I'm more concerned about coherency, so I'd
like to commit the change below before we try something different.

ok?

Index: uvm/uvm_anon.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_anon.c,v
retrieving revision 1.55
diff -u -p -u -7 -r1.55 uvm_anon.c
--- uvm/uvm_anon.c      11 Apr 2023 00:45:09 -0000      1.55
+++ uvm/uvm_anon.c      15 May 2023 13:55:28 -0000
@@ -251,14 +251,15 @@ uvm_anon_release(struct vm_anon *anon)
        KASSERT((pg->pg_flags & PG_RELEASED) != 0);
        KASSERT((pg->pg_flags & PG_BUSY) != 0);
        KASSERT(pg->uobject == NULL);
        KASSERT(pg->uanon == anon);
        KASSERT(anon->an_ref == 0);
 
        uvm_lock_pageq();
+       pmap_page_protect(pg, PROT_NONE);
        uvm_pagefree(pg);
        uvm_unlock_pageq();
        KASSERT(anon->an_page == NULL);
        lock = anon->an_lock;
        uvm_anfree(anon);
        rw_exit(lock);
        /* Note: extra reference is held for PG_RELEASED case. */
Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.133
diff -u -p -u -7 -r1.133 uvm_fault.c
--- uvm/uvm_fault.c     4 Nov 2022 09:36:44 -0000       1.133
+++ uvm/uvm_fault.c     15 May 2023 13:55:28 -0000
@@ -392,15 +392,14 @@ uvmfault_anonget(struct uvm_faultinfo *u
 
                        /*
                         * if we were RELEASED during I/O, then our anon is
                         * no longer part of an amap.   we need to free the
                         * anon and try again.
                         */
                        if (pg->pg_flags & PG_RELEASED) {
-                               pmap_page_protect(pg, PROT_NONE);
                                KASSERT(anon->an_ref == 0);
                                /*
                                 * Released while we had unlocked amap.
                                 */
                                if (locked)
                                        uvmfault_unlockall(ufi, NULL, NULL);
                                uvm_anon_release(anon); /* frees page for us */

Reply via email to