uvm_map_inentry diff for testing

[Mark Kettenis]

While playing with dt(4)/btrace(4) flamegraphs on a 32-core arm64
machine, I noticed that the kernel was spending a lot of time (6.84%)
in uvm_map_inentry().  This is caused by kernel lock contention.
Pushing baack the kernel lock further into the slow path reduces the
time to 0.05%.

Now mpi@ tried this before in sys/uvm/uvm_map.c rev 1.249.  That was
backed out because it caused issues with golang.  So I had a look
again and identified a potential issue.  When we "fix" the cached
entry, we use the serial number that has been passed down to us.  But
since that serial number was recorded before we grapped the vm_map
lock, it may be stale.  Instead we should be using the current map
serial number.

This makes me wonder whether having two serial numbers is a case of
premature optimization.  But that is a different discussion.

I'd like to see this tested, especially by people running/building
golang.  Does anybody remember specific details of the issue?  If it
manifests itself, it should be pretty obvious as it results in a
kernel printf and a killed process.


Index: uvm/uvm_map.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.263
diff -u -p -r1.263 uvm_map.c
--- uvm/uvm_map.c       4 Mar 2020 21:15:38 -0000       1.263
+++ uvm/uvm_map.c       17 Mar 2020 18:57:19 -0000
@@ -161,7 +161,7 @@ void                         uvm_map_addr_augment(struct 
vm_m
 int                     uvm_map_inentry_recheck(u_long, vaddr_t,
                             struct p_inentry *);
 boolean_t               uvm_map_inentry_fix(struct proc *, struct p_inentry *,
-                            vaddr_t, int (*)(vm_map_entry_t), u_long);
+                            vaddr_t, int (*)(vm_map_entry_t));
 /*
  * Tree management functions.
  */
@@ -1848,10 +1848,11 @@ uvm_map_inentry_recheck(u_long serial, v
  */
 boolean_t
 uvm_map_inentry_fix(struct proc *p, struct p_inentry *ie, vaddr_t addr,
-    int (*fn)(vm_map_entry_t), u_long serial)
+    int (*fn)(vm_map_entry_t))
 {
        vm_map_t map = &p->p_vmspace->vm_map;
        vm_map_entry_t entry;
+       u_long serial;
        int ret;
 
        if (addr < map->min_offset || addr >= map->max_offset)
@@ -1866,6 +1867,11 @@ uvm_map_inentry_fix(struct proc *p, stru
                return (FALSE);
        }
 
+       if (ie == &p->p_spinentry)
+               serial = map->sserial;
+       else
+               serial = map->wserial;
+
        ret = (*fn)(entry);
        if (ret == 0) {
                vm_map_unlock_read(map);
@@ -1889,16 +1895,16 @@ uvm_map_inentry(struct proc *p, struct p
        boolean_t ok = TRUE;
 
        if (uvm_map_inentry_recheck(serial, addr, ie)) {
-               KERNEL_LOCK();
-               ok = uvm_map_inentry_fix(p, ie, addr, fn, serial);
+               ok = uvm_map_inentry_fix(p, ie, addr, fn);
                if (!ok) {
                        printf(fmt, p->p_p->ps_comm, p->p_p->ps_pid, p->p_tid,
                            addr, ie->ie_start, ie->ie_end);
+                       KERNEL_LOCK();
                        p->p_p->ps_acflag |= AMAP;
                        sv.sival_ptr = (void *)PROC_PC(p);
                        trapsignal(p, SIGSEGV, 0, SEGV_ACCERR, sv);
+                       KERNEL_UNLOCK();
                }
-               KERNEL_UNLOCK();
        }
        return (ok);
 }