kernel - Remove mplock from KTRACE paths master
authorMatthew Dillon <dillon@apollo.backplane.com>
Tue, 27 Sep 2016 21:39:03 +0000 (14:39 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Tue, 27 Sep 2016 21:41:56 +0000 (14:41 -0700)
* The mplock is no longer needed for KTRACE, ktrace writes are serialized
  by the vnode lock and everything else is MPSAFE.  Note that this change
  means that even fast system calls may interleave in the ktrace output on
  a multi-threaded program.

* Fix ktrace bug related to vkernels.  The syscall2() code assumes that
  no tokens are held on entry (since we are coming from usermode), but
  a system call made from the vkernel may actually be nested inside
  another syscall2().  The mplock KTRACE held caused this to assert in
  the nested syscall2().  The removal of the mplock from the ktrace path
  also fixes this bug.

* Minor comment adjustment in vm_vmspace.c.

Reported-by: tuxillo
sys/kern/sys_generic.c
sys/platform/pc64/x86_64/trap.c
sys/vm/vm_vmspace.c

index f71229a..c894a81 100644 (file)
@@ -309,9 +309,7 @@ dofileread(int fd, struct file *fp, struct uio *auio, int flags, size_t *res)
                if (error == 0) {
                        ktruio.uio_iov = ktriov;
                        ktruio.uio_resid = len - auio->uio_resid;
-                       get_mplock();
                        ktrgenio(td->td_lwp, fd, UIO_READ, &ktruio, error);
-                       rel_mplock();
                }
                kfree(ktriov, M_TEMP);
        }
@@ -522,9 +520,7 @@ dofilewrite(int fd, struct file *fp, struct uio *auio, int flags, size_t *res)
                if (error == 0) {
                        ktruio.uio_iov = ktriov;
                        ktruio.uio_resid = len - auio->uio_resid;
-                       get_mplock();
                        ktrgenio(lp, fd, UIO_WRITE, &ktruio, error);
-                       rel_mplock();
                }
                kfree(ktriov, M_TEMP);
        }
index 82bd915..171d419 100644 (file)
@@ -1155,6 +1155,7 @@ syscall2(struct trapframe *frame)
        if (lp->lwp_vkernel && lp->lwp_vkernel->ve) {
                vkernel_trap(lp, frame);
                error = EJUSTRETURN;
+               callp = NULL;
                goto out;
        }
 
@@ -1212,8 +1213,6 @@ syscall2(struct trapframe *frame)
                if (error) {
 #ifdef KTRACE
                        if (KTRPOINT(td, KTR_SYSCALL)) {
-                               MAKEMPSAFE(have_mplock);
-
                                ktrsyscall(lp, code, narg,
                                        (void *)(&args.nosys.sysmsg + 1));
                        }
@@ -1224,7 +1223,6 @@ syscall2(struct trapframe *frame)
 
 #ifdef KTRACE
        if (KTRPOINT(td, KTR_SYSCALL)) {
-               MAKEMPSAFE(have_mplock);
                ktrsyscall(lp, code, narg, (void *)(&args.nosys.sysmsg + 1));
        }
 #endif
@@ -1308,10 +1306,9 @@ bad:
        }
 
        /*
-        * Traced syscall.  trapsignal() is not MP aware.
+        * Traced syscall.  trapsignal() should now be MP aware
         */
        if (orig_tf_rflags & PSL_T) {
-               MAKEMPSAFE(have_mplock);
                frame->tf_rflags &= ~PSL_T;
                trapsignal(lp, SIGTRAP, TRAP_TRACE);
        }
@@ -1323,7 +1320,6 @@ bad:
 
 #ifdef KTRACE
        if (KTRPOINT(td, KTR_SYSRET)) {
-               MAKEMPSAFE(have_mplock);
                ktrsysret(lp, code, error, args.sysmsg_result);
        }
 #endif
@@ -1347,8 +1343,9 @@ bad:
                ("syscall: critical section count mismatch! %d/%d",
                crit_count, td->td_pri));
        KASSERT(&td->td_toks_base == td->td_toks_stop,
-               ("syscall: extra tokens held after trap! %ld",
-               td->td_toks_stop - &td->td_toks_base));
+               ("syscall: %ld extra tokens held after trap! syscall %p",
+               td->td_toks_stop - &td->td_toks_base,
+               callp->sy_call));
 #endif
 }
 
index d4718da..58276b5 100644 (file)
@@ -243,9 +243,11 @@ sys_vmspace_ctl(struct vmspace_ctl_args *uap)
                        if (curthread->td_vmm == NULL)
                                atomic_subtract_int(&ve->refs, 1);
                } else {
-                       /* If it's a VMM thread just set the CR3. We also set the
-                        * vklp->ve to a key to be able to distinguish when a
-                        * vkernel user process runs and when not (when it's NULL)
+                       /*
+                        * If it's a VMM thread just set the CR3. We also set
+                        * the vklp->ve to a key to be able to distinguish
+                        * when a vkernel user process runs and when not
+                        * (when it's NULL)
                         */
                        if (curthread->td_vmm == NULL) {
                                vklp->ve = ve;