Author: markj
Date: Wed Nov  4 16:30:56 2020
New Revision: 367334
URL: https://svnweb.freebsd.org/changeset/base/367334

Log:
  vmspace: Convert to refcount(9)
  
  This is mostly mechanical except for vmspace_exit().  There, use the new
  refcount_release_if_last() to avoid switching to vmspace0 unless other
  processes are sharing the vmspace.  In that case, upon switching to
  vmspace0 we can unconditionally release the reference.
  
  Remove the volatile qualifier from vm_refcnt now that accesses are
  protected using refcount(9) KPIs.
  
  Reviewed by:  alc, kib, mmel
  MFC after:    1 month
  Sponsored by: The FreeBSD Foundation
  Differential Revision:        https://reviews.freebsd.org/D27057

Modified:
  head/sys/dev/cxgbe/tom/t4_ddp.c
  head/sys/kern/init_main.c
  head/sys/kern/kern_exec.c
  head/sys/kern/vfs_aio.c
  head/sys/vm/vm_glue.c
  head/sys/vm/vm_map.c
  head/sys/vm/vm_map.h

Modified: head/sys/dev/cxgbe/tom/t4_ddp.c
==============================================================================
--- head/sys/dev/cxgbe/tom/t4_ddp.c     Wed Nov  4 16:30:30 2020        
(r367333)
+++ head/sys/dev/cxgbe/tom/t4_ddp.c     Wed Nov  4 16:30:56 2020        
(r367334)
@@ -1347,7 +1347,7 @@ hold_aio(struct toepcb *toep, struct kaiocb *job, stru
 
        ps->offset = pgoff;
        ps->len = job->uaiocb.aio_nbytes;
-       atomic_add_int(&vm->vm_refcnt, 1);
+       refcount_acquire(&vm->vm_refcnt);
        ps->vm = vm;
        ps->start = start;
 

Modified: head/sys/kern/init_main.c
==============================================================================
--- head/sys/kern/init_main.c   Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/kern/init_main.c   Wed Nov  4 16:30:56 2020        (r367334)
@@ -591,7 +591,7 @@ proc0_init(void *dummy __unused)
 
        /* Allocate a prototype map so we have something to fork. */
        p->p_vmspace = &vmspace0;
-       vmspace0.vm_refcnt = 1;
+       refcount_init(&vmspace0.vm_refcnt, 1);
        pmap_pinit0(vmspace_pmap(&vmspace0));
 
        /*

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c   Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/kern/kern_exec.c   Wed Nov  4 16:30:56 2020        (r367334)
@@ -1060,7 +1060,8 @@ exec_new_vmspace(struct image_params *imgp, struct sys
                sv_minuser = sv->sv_minuser;
        else
                sv_minuser = MAX(sv->sv_minuser, PAGE_SIZE);
-       if (vmspace->vm_refcnt == 1 && vm_map_min(map) == sv_minuser &&
+       if (refcount_load(&vmspace->vm_refcnt) == 1 &&
+           vm_map_min(map) == sv_minuser &&
            vm_map_max(map) == sv->sv_maxuser &&
            cpu_exec_vmspace_reuse(p, map)) {
                shmexit(vmspace);

Modified: head/sys/kern/vfs_aio.c
==============================================================================
--- head/sys/kern/vfs_aio.c     Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/kern/vfs_aio.c     Wed Nov  4 16:30:56 2020        (r367334)
@@ -1159,8 +1159,9 @@ aio_daemon(void *_id)
 
        KASSERT(p->p_vmspace == myvm,
            ("AIOD: bad vmspace for exiting daemon"));
-       KASSERT(myvm->vm_refcnt > 1,
-           ("AIOD: bad vm refcnt for exiting daemon: %d", myvm->vm_refcnt));
+       KASSERT(refcount_load(&myvm->vm_refcnt) > 1,
+           ("AIOD: bad vm refcnt for exiting daemon: %d",
+           refcount_load(&myvm->vm_refcnt)));
        kproc_exit(0);
 }
 

Modified: head/sys/vm/vm_glue.c
==============================================================================
--- head/sys/vm/vm_glue.c       Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/vm/vm_glue.c       Wed Nov  4 16:30:56 2020        (r367334)
@@ -75,6 +75,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/mutex.h>
 #include <sys/proc.h>
 #include <sys/racct.h>
+#include <sys/refcount.h>
 #include <sys/resourcevar.h>
 #include <sys/rwlock.h>
 #include <sys/sched.h>
@@ -549,7 +550,7 @@ vm_forkproc(struct thread *td, struct proc *p2, struct
                 * COW locally.
                 */
                if ((flags & RFMEM) == 0) {
-                       if (p1->p_vmspace->vm_refcnt > 1) {
+                       if (refcount_load(&p1->p_vmspace->vm_refcnt) > 1) {
                                error = vmspace_unshare(p1);
                                if (error)
                                        return (error);
@@ -561,7 +562,7 @@ vm_forkproc(struct thread *td, struct proc *p2, struct
 
        if (flags & RFMEM) {
                p2->p_vmspace = p1->p_vmspace;
-               atomic_add_int(&p1->p_vmspace->vm_refcnt, 1);
+               refcount_acquire(&p1->p_vmspace->vm_refcnt);
        }
        dset = td2->td_domain.dr_policy;
        while (vm_page_count_severe_set(&dset->ds_mask)) {

Modified: head/sys/vm/vm_map.c
==============================================================================
--- head/sys/vm/vm_map.c        Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/vm/vm_map.c        Wed Nov  4 16:30:56 2020        (r367334)
@@ -257,7 +257,7 @@ vmspace_alloc(vm_offset_t min, vm_offset_t max, pmap_p
        }
        CTR1(KTR_VM, "vmspace_alloc: %p", vm);
        _vm_map_init(&vm->vm_map, vmspace_pmap(vm), min, max);
-       vm->vm_refcnt = 1;
+       refcount_init(&vm->vm_refcnt, 1);
        vm->vm_shm = NULL;
        vm->vm_swrss = 0;
        vm->vm_tsize = 0;
@@ -316,10 +316,7 @@ vmspace_free(struct vmspace *vm)
        WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL,
            "vmspace_free() called");
 
-       if (vm->vm_refcnt == 0)
-               panic("vmspace_free: attempt to free already freed vmspace");
-
-       if (atomic_fetchadd_int(&vm->vm_refcnt, -1) == 1)
+       if (refcount_release(&vm->vm_refcnt))
                vmspace_dofree(vm);
 }
 
@@ -339,45 +336,42 @@ vmspace_exitfree(struct proc *p)
 void
 vmspace_exit(struct thread *td)
 {
-       int refcnt;
        struct vmspace *vm;
        struct proc *p;
+       bool released;
 
-       /*
-        * Release user portion of address space.
-        * This releases references to vnodes,
-        * which could cause I/O if the file has been unlinked.
-        * Need to do this early enough that we can still sleep.
-        *
-        * The last exiting process to reach this point releases as
-        * much of the environment as it can. vmspace_dofree() is the
-        * slower fallback in case another process had a temporary
-        * reference to the vmspace.
-        */
-
        p = td->td_proc;
        vm = p->p_vmspace;
-       atomic_add_int(&vmspace0.vm_refcnt, 1);
-       refcnt = vm->vm_refcnt;
-       do {
-               if (refcnt > 1 && p->p_vmspace != &vmspace0) {
-                       /* Switch now since other proc might free vmspace */
+
+       /*
+        * Prepare to release the vmspace reference.  The thread that releases
+        * the last reference is responsible for tearing down the vmspace.
+        * However, threads not releasing the final reference must switch to the
+        * kernel's vmspace0 before the decrement so that the subsequent pmap
+        * deactivation does not modify a freed vmspace.
+        */
+       refcount_acquire(&vmspace0.vm_refcnt);
+       if (!(released = refcount_release_if_last(&vm->vm_refcnt))) {
+               if (p->p_vmspace != &vmspace0) {
                        PROC_VMSPACE_LOCK(p);
                        p->p_vmspace = &vmspace0;
                        PROC_VMSPACE_UNLOCK(p);
                        pmap_activate(td);
                }
-       } while (!atomic_fcmpset_int(&vm->vm_refcnt, &refcnt, refcnt - 1));
-       if (refcnt == 1) {
+               released = refcount_release(&vm->vm_refcnt);
+       }
+       if (released) {
+               /*
+                * pmap_remove_pages() expects the pmap to be active, so switch
+                * back first if necessary.
+                */
                if (p->p_vmspace != vm) {
-                       /* vmspace not yet freed, switch back */
                        PROC_VMSPACE_LOCK(p);
                        p->p_vmspace = vm;
                        PROC_VMSPACE_UNLOCK(p);
                        pmap_activate(td);
                }
                pmap_remove_pages(vmspace_pmap(vm));
-               /* Switch now since this proc will free vmspace */
                PROC_VMSPACE_LOCK(p);
                p->p_vmspace = &vmspace0;
                PROC_VMSPACE_UNLOCK(p);
@@ -396,21 +390,13 @@ struct vmspace *
 vmspace_acquire_ref(struct proc *p)
 {
        struct vmspace *vm;
-       int refcnt;
 
        PROC_VMSPACE_LOCK(p);
        vm = p->p_vmspace;
-       if (vm == NULL) {
+       if (vm == NULL || !refcount_acquire_if_not_zero(&vm->vm_refcnt)) {
                PROC_VMSPACE_UNLOCK(p);
                return (NULL);
        }
-       refcnt = vm->vm_refcnt;
-       do {
-               if (refcnt <= 0) {      /* Avoid 0->1 transition */
-                       PROC_VMSPACE_UNLOCK(p);
-                       return (NULL);
-               }
-       } while (!atomic_fcmpset_int(&vm->vm_refcnt, &refcnt, refcnt + 1));
        if (vm != p->p_vmspace) {
                PROC_VMSPACE_UNLOCK(p);
                vmspace_free(vm);
@@ -442,7 +428,7 @@ vmspace_switch_aio(struct vmspace *newvm)
 
        /* XXX: Need some way to assert that this is an aio daemon. */
 
-       KASSERT(newvm->vm_refcnt > 0,
+       KASSERT(refcount_load(&newvm->vm_refcnt) > 0,
            ("vmspace_switch_aio: newvm unreferenced"));
 
        oldvm = curproc->p_vmspace;
@@ -453,7 +439,7 @@ vmspace_switch_aio(struct vmspace *newvm)
         * Point to the new address space and refer to it.
         */
        curproc->p_vmspace = newvm;
-       atomic_add_int(&newvm->vm_refcnt, 1);
+       refcount_acquire(&newvm->vm_refcnt);
 
        /* Activate the new mapping. */
        pmap_activate(curthread);
@@ -4777,7 +4763,7 @@ vmspace_unshare(struct proc *p)
        struct vmspace *newvmspace;
        vm_ooffset_t fork_charge;
 
-       if (oldvmspace->vm_refcnt == 1)
+       if (refcount_load(&oldvmspace->vm_refcnt) == 1)
                return (0);
        fork_charge = 0;
        newvmspace = vmspace_fork(oldvmspace, &fork_charge);

Modified: head/sys/vm/vm_map.h
==============================================================================
--- head/sys/vm/vm_map.h        Wed Nov  4 16:30:30 2020        (r367333)
+++ head/sys/vm/vm_map.h        Wed Nov  4 16:30:56 2020        (r367334)
@@ -291,7 +291,7 @@ struct vmspace {
        caddr_t vm_taddr;       /* (c) user virtual address of text */
        caddr_t vm_daddr;       /* (c) user virtual address of data */
        caddr_t vm_maxsaddr;    /* user VA at max stack growth */
-       volatile int vm_refcnt; /* number of references */
+       u_int vm_refcnt;        /* number of references */
        /*
         * Keep the PMAP last, so that CPU-specific variations of that
         * structure on a single architecture don't result in offset
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to