Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/uvm Keep the lock around pmap_update() where required. ...



details:   https://anonhg.NetBSD.org/src/rev/d1a606395975
branches:  trunk
changeset: 755812:d1a606395975
user:      rmind <rmind%NetBSD.org@localhost>
date:      Tue Jun 22 18:34:50 2010 +0000

description:
Keep the lock around pmap_update() where required.  While fixing this
in ubc_fault(), rework logic to "remember" the last object of page and
reduce locking overhead, since in common case pages belong to one and
the same UVM object (but not always, therefore add a comment).

Unlocks before pmap_update(), on removal of mappings, might cause TLB
coherency issues, since on architectures like x86 and mips64 invalidation
IPIs are deferred to pmap_update().  Hence, VA space might be globally
visible before IPIs are sent or while they are still in-flight.

OK ad@.

diffstat:

 sys/uvm/uvm_bio.c   |  69 ++++++++++++++++++++++++++++++++++++----------------
 sys/uvm/uvm_fault.c |  12 +++++---
 sys/uvm/uvm_map.c   |  12 +++++++-
 sys/uvm/uvm_pager.c |   7 +++--
 4 files changed, 68 insertions(+), 32 deletions(-)

diffs (263 lines):

diff -r f883be421a04 -r d1a606395975 sys/uvm/uvm_bio.c
--- a/sys/uvm/uvm_bio.c Tue Jun 22 18:32:07 2010 +0000
+++ b/sys/uvm/uvm_bio.c Tue Jun 22 18:34:50 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_bio.c,v 1.69 2010/05/29 23:17:53 rmind Exp $       */
+/*     $NetBSD: uvm_bio.c,v 1.70 2010/06/22 18:34:50 rmind Exp $       */
 
 /*
  * Copyright (c) 1998 Chuck Silvers.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_bio.c,v 1.69 2010/05/29 23:17:53 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_bio.c,v 1.70 2010/06/22 18:34:50 rmind Exp $");
 
 #include "opt_uvmhist.h"
 #include "opt_ubc.h"
@@ -217,9 +217,11 @@
 
 /*
  * ubc_fault_page: helper of ubc_fault to handle a single page.
+ *
+ * => Caller has UVM object locked.
  */
 
-static inline void
+static inline int
 ubc_fault_page(const struct uvm_faultinfo *ufi, const struct ubc_map *umap,
     struct vm_page *pg, vm_prot_t prot, vm_prot_t access_type, vaddr_t va)
 {
@@ -229,7 +231,8 @@
        bool rdonly;
 
        uobj = pg->uobject;
-       mutex_enter(&uobj->vmobjlock);
+       KASSERT(mutex_owned(&uobj->vmobjlock));
+
        if (pg->flags & PG_WANTED) {
                wakeup(pg);
        }
@@ -238,8 +241,7 @@
                mutex_enter(&uvm_pageqlock);
                uvm_pagefree(pg);
                mutex_exit(&uvm_pageqlock);
-               mutex_exit(&uobj->vmobjlock);
-               return;
+               return 0;
        }
        if (pg->loan_count != 0) {
 
@@ -256,12 +258,7 @@
                        newpg = uvm_loanbreak(pg);
                        if (newpg == NULL) {
                                uvm_page_unbusy(&pg, 1);
-                               mutex_exit(&uobj->vmobjlock);
-                               uvm_wait("ubc_loanbrk");
-                               /*
-                                * Note: will re-fault.
-                                */
-                               return;
+                               return ENOMEM;
                        }
                        pg = newpg;
                }
@@ -290,14 +287,8 @@
        mutex_exit(&uvm_pageqlock);
        pg->flags &= ~(PG_BUSY|PG_WANTED);
        UVM_PAGE_OWN(pg, NULL);
-       mutex_exit(&uobj->vmobjlock);
 
-       if (error) {
-               /*
-                * Note: will re-fault.
-                */
-               uvm_wait("ubc_pmfail");
-       }
+       return error;
 }
 
 /*
@@ -386,7 +377,7 @@
            0);
 
        if (error == EAGAIN) {
-               kpause("ubc_fault", false, hz, NULL);
+               kpause("ubc_fault", false, hz >> 2, NULL);
                goto again;
        }
        if (error) {
@@ -405,6 +396,16 @@
 #else
        prot = VM_PROT_READ | VM_PROT_WRITE;
 #endif
+
+       /*
+        * Note: in the common case, all returned pages would have the same
+        * UVM object.  However, due to layered file-systems and e.g. tmpfs,
+        * returned pages may have different objects.  We "remember" the
+        * last object in the loop to reduce locking overhead and to perform
+        * pmap_update() before object unlock.
+        */
+       uobj = NULL;
+
        va = ufi->orig_rvaddr;
        eva = ufi->orig_rvaddr + (npages << PAGE_SHIFT);
 
@@ -418,9 +419,33 @@
                if (pg == NULL || pg == PGO_DONTCARE) {
                        continue;
                }
-               ubc_fault_page(ufi, umap, pg, prot, access_type, va);
+               if (__predict_false(pg->uobject != uobj)) {
+                       /* Check for the first iteration and error cases. */
+                       if (uobj != NULL) {
+                               /* Must make VA visible before the unlock. */
+                               pmap_update(ufi->orig_map->pmap);
+                               mutex_exit(&uobj->vmobjlock);
+                       }
+                       uobj = pg->uobject;
+                       mutex_enter(&uobj->vmobjlock);
+               }
+               error = ubc_fault_page(ufi, umap, pg, prot, access_type, va);
+               if (error) {
+                       /*
+                        * Flush (there might be pages entered), drop the lock,
+                        * "forget" the object and perform uvm_wait().
+                        * Note: page will re-fault.
+                        */
+                       pmap_update(ufi->orig_map->pmap);
+                       mutex_exit(&uobj->vmobjlock);
+                       uobj = NULL;
+                       uvm_wait("ubc_fault");
+               }
        }
-       pmap_update(ufi->orig_map->pmap);
+       if (__predict_true(uobj != NULL)) {
+               pmap_update(ufi->orig_map->pmap);
+               mutex_exit(&uobj->vmobjlock);
+       }
        return 0;
 }
 
diff -r f883be421a04 -r d1a606395975 sys/uvm/uvm_fault.c
--- a/sys/uvm/uvm_fault.c       Tue Jun 22 18:32:07 2010 +0000
+++ b/sys/uvm/uvm_fault.c       Tue Jun 22 18:34:50 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_fault.c,v 1.174 2010/05/28 23:41:14 rmind Exp $    */
+/*     $NetBSD: uvm_fault.c,v 1.175 2010/06/22 18:34:50 rmind Exp $    */
 
 /*
  *
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_fault.c,v 1.174 2010/05/28 23:41:14 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_fault.c,v 1.175 2010/06/22 18:34:50 rmind Exp $");
 
 #include "opt_uvmhist.h"
 
@@ -1496,10 +1496,11 @@
         * done case 1!  finish up by unlocking everything and returning success
         */
 
-       if (anon != oanon)
+       if (anon != oanon) {
                mutex_exit(&anon->an_lock);
+       }
+       pmap_update(ufi->orig_map->pmap);
        uvmfault_unlockall(ufi, amap, uobj, oanon);
-       pmap_update(ufi->orig_map->pmap);
        return 0;
 }
 
@@ -2197,8 +2198,9 @@
 
        uvm_fault_lower_done(ufi, flt, uobj, anon, pg);
 
+       pmap_update(ufi->orig_map->pmap);
        uvmfault_unlockall(ufi, amap, uobj, anon);
-       pmap_update(ufi->orig_map->pmap);
+
        UVMHIST_LOG(maphist, "<- done (SUCCESS!)",0,0,0,0);
        return 0;
 }
diff -r f883be421a04 -r d1a606395975 sys/uvm/uvm_map.c
--- a/sys/uvm/uvm_map.c Tue Jun 22 18:32:07 2010 +0000
+++ b/sys/uvm/uvm_map.c Tue Jun 22 18:34:50 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_map.c,v 1.291 2010/05/14 05:32:06 cegger Exp $     */
+/*     $NetBSD: uvm_map.c,v 1.292 2010/06/22 18:34:50 rmind Exp $      */
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -71,7 +71,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.291 2010/05/14 05:32:06 cegger Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.292 2010/06/22 18:34:50 rmind Exp $");
 
 #include "opt_ddb.h"
 #include "opt_uvmhist.h"
@@ -2356,6 +2356,7 @@
                         * and we should free any such pages immediately.
                         * this is mostly used for kmem_map.
                         */
+                       KASSERT(vm_map_pmap(map) == pmap_kernel());
 
                        if ((entry->flags & UVM_MAP_KMAPENT) == 0) {
                                uvm_km_pgremove_intrsafe(map, entry->start,
@@ -2460,8 +2461,15 @@
                first_entry = entry;
                entry = next;
        }
+
+       /*
+        * Note: if map is dying, leave pmap_update() for pmap_destroy(),
+        * which will be called later.
+        */
        if ((map->flags & VM_MAP_DYING) == 0) {
                pmap_update(vm_map_pmap(map));
+       } else {
+               KASSERT(vm_map_pmap(map) != pmap_kernel());
        }
 
        uvm_map_check(map, "unmap_remove leave");
diff -r f883be421a04 -r d1a606395975 sys/uvm/uvm_pager.c
--- a/sys/uvm/uvm_pager.c       Tue Jun 22 18:32:07 2010 +0000
+++ b/sys/uvm/uvm_pager.c       Tue Jun 22 18:34:50 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_pager.c,v 1.97 2009/11/07 07:27:50 cegger Exp $    */
+/*     $NetBSD: uvm_pager.c,v 1.98 2010/06/22 18:34:50 rmind Exp $     */
 
 /*
  *
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_pager.c,v 1.97 2009/11/07 07:27:50 cegger Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_pager.c,v 1.98 2010/06/22 18:34:50 rmind Exp $");
 
 #include "opt_uvmhist.h"
 #include "opt_readahead.h"
@@ -230,6 +230,8 @@
         */
 
        pmap_kremove(kva, npages << PAGE_SHIFT);
+       pmap_update(pmap_kernel());
+
        if (kva == emergva) {
                mutex_enter(&pager_map_wanted_lock);
                emerginuse = false;
@@ -249,7 +251,6 @@
        vm_map_unlock(pager_map);
        if (entries)
                uvm_unmap_detach(entries, 0);
-       pmap_update(pmap_kernel());
        UVMHIST_LOG(maphist,"<- done",0,0,0,0);
 }
 



Home | Main Index | Thread Index | Old Index