kernel - Fix cpu/token starvation, vfs_busy deadlocks. incls sysctl
authorMatthew Dillon <dillon@apollo.backplane.com>
Thu, 21 Feb 2013 02:38:33 +0000 (18:38 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Thu, 21 Feb 2013 02:38:33 +0000 (18:38 -0800)
* Remove the mplock around the userland sysctl system call, it should no
  longer be needed.

* Remove the mplock around getcwd(), it should no longer be needed.

* Change the vfs_busy(), sys_mount(), and related mount code to use the
  per-mount token instead of the mp lock.

* Fix a race in vfs_busy() which could cause it to never get woken up.

* Fix a deadlock in nlookup() when the lookup is racing an unmount.  When
  the mp is flagged MNTK_UNMOUNT, the unmount is in progress and the lookup
  must fail instead of loop.

* per-mount token now protects mp->mnt_kern_flag.

* unmount code now waits for final mnt_refs to return to the proper value,
  fixing races with other code that might temporarily ref the mount point.

* Add lwkt_yield()'s in nvtruncbuf*() and nvnode_pager_setsize(), reducing
  cpu stalls due to large file-extending I/O's.  Also in tmpfs.

* Use a marker in the vm_meter code and check for vmobj_token collisions.
  When a collision is detected, give other threads a chance to take the
  token.  This prevents hogging of this very important token.

Testing-by: dillon, vsrinivas, ftigeot
sys/kern/kern_sysctl.c
sys/kern/vfs_cache.c
sys/kern/vfs_mount.c
sys/kern/vfs_nlookup.c
sys/kern/vfs_syscalls.c
sys/kern/vfs_vm.c
sys/vfs/tmpfs/tmpfs_vfsops.c
sys/vm/vm_meter.c

index f348700..93c9473 100644 (file)
@@ -1218,11 +1218,9 @@ sys___sysctl(struct sysctl_args *uap)
        if (error)
                return (error);
 
-       get_mplock();
        error = userland_sysctl(name, uap->namelen,
                uap->old, uap->oldlenp, 0,
                uap->new, uap->newlen, &j);
-       rel_mplock();
        if (error && error != ENOMEM)
                return (error);
        if (uap->oldlenp) {
index 339c6d2..319d5f7 100644 (file)
@@ -3128,9 +3128,7 @@ sys___getcwd(struct __getcwd_args *uap)
                buflen = MAXPATHLEN;
 
        buf = kmalloc(buflen, M_TEMP, M_WAITOK);
-       get_mplock();
        bp = kern_getcwd(buf, buflen, &error);
-       rel_mplock();
        if (error == 0)
                error = copyout(bp, uap->buf, strlen(bp) + 1);
        kfree(buf, M_TEMP);
index c043ad0..126067c 100644 (file)
@@ -248,8 +248,10 @@ vfs_busy(struct mount *mp, int flags)
        int lkflags;
 
        atomic_add_int(&mp->mnt_refs, 1);
+       lwkt_gettoken(&mp->mnt_token);
        if (mp->mnt_kern_flag & MNTK_UNMOUNT) {
                if (flags & LK_NOWAIT) {
+                       lwkt_reltoken(&mp->mnt_token);
                        atomic_add_int(&mp->mnt_refs, -1);
                        return (ENOENT);
                }
@@ -262,12 +264,14 @@ vfs_busy(struct mount *mp, int flags)
                 * exclusive lock at the end of dounmount.
                 */
                tsleep((caddr_t)mp, 0, "vfs_busy", 0);
+               lwkt_reltoken(&mp->mnt_token);
                atomic_add_int(&mp->mnt_refs, -1);
                return (ENOENT);
        }
        lkflags = LK_SHARED;
        if (lockmgr(&mp->mnt_lock, lkflags))
                panic("vfs_busy: unexpected lock failure");
+       lwkt_reltoken(&mp->mnt_token);
        return (0);
 }
 
index 1bc9d6b..6944fe3 100644 (file)
@@ -745,8 +745,15 @@ nlookup(struct nlookupdata *nd)
 again:
            cache_put(&nch);
            if (vfs_do_busy) {
-               while (vfs_busy(mp, 0))
-                   ;
+               while (vfs_busy(mp, 0)) {
+                   if (mp->mnt_kern_flag & MNTK_UNMOUNT) {
+                       kprintf("nlookup: warning umount race avoided\n");
+                       cache_dropmount(mp);
+                       error = EBUSY;
+                       vfs_do_busy = 0;
+                       goto double_break;
+                   }
+               }
            }
            cache_get(&mp->mnt_ncmountpt, &nch);
 
@@ -769,6 +776,8 @@ again:
                vfs_unbusy(mp);
            cache_dropmount(mp);
        }
+
+double_break:
        if (error) {
            cache_put(&nch);
            break;
index eec6025..f051ae0 100644 (file)
@@ -128,7 +128,6 @@ sys_mount(struct mount_args *uap)
        char fstypename[MFSNAMELEN];
        struct ucred *cred;
 
-       get_mplock();
        cred = td->td_ucred;
        if (jailed(cred)) {
                error = EPERM;
@@ -267,6 +266,7 @@ sys_mount(struct mount_args *uap)
                vsetflags(vp, VMOUNT);
                mp->mnt_flag |=
                    uap->flags & (MNT_RELOAD | MNT_FORCE | MNT_UPDATE);
+               lwkt_gettoken(&mp->mnt_token);
                vn_unlock(vp);
                goto update;
        }
@@ -348,9 +348,12 @@ sys_mount(struct mount_args *uap)
        mp->mnt_flag |= vfsp->vfc_flags & MNT_VISFLAGMASK;
        strncpy(mp->mnt_stat.f_fstypename, vfsp->vfc_name, MFSNAMELEN);
        mp->mnt_stat.f_owner = cred->cr_uid;
+       lwkt_gettoken(&mp->mnt_token);
        vn_unlock(vp);
 update:
        /*
+        * (per-mount token acquired at this point)
+        *
         * Set the mount level flags.
         */
        if (uap->flags & MNT_RDONLY)
@@ -380,6 +383,7 @@ update:
                        mp->mnt_flag = flag;
                        mp->mnt_kern_flag = flag2;
                }
+               lwkt_reltoken(&mp->mnt_token);
                vfs_unbusy(mp);
                vclrflags(vp, VMOUNT);
                vrele(vp);
@@ -415,6 +419,7 @@ update:
                vn_unlock(vp);
                checkdirs(&mp->mnt_ncmounton, &mp->mnt_ncmountpt);
                error = vfs_allocate_syncvnode(mp);
+               lwkt_reltoken(&mp->mnt_token);
                vfs_unbusy(mp);
                error = VFS_START(mp, 0);
                vrele(vp);
@@ -426,13 +431,13 @@ update:
                vfs_rm_vnodeops(mp, NULL, &mp->mnt_vn_fifo_ops);
                vclrflags(vp, VMOUNT);
                mp->mnt_vfc->vfc_refcount--;
+               lwkt_reltoken(&mp->mnt_token);
                vfs_unbusy(mp);
                kfree(mp, M_MOUNT);
                cache_drop(&nch);
                vput(vp);
        }
 done:
-       rel_mplock();
        return (error);
 }
 
@@ -664,6 +669,7 @@ dounmount(struct mount *mp, int flags)
        int freeok = 1;
 
        lwkt_gettoken(&mntvnode_token);
+       lwkt_gettoken(&mp->mnt_token);
        /*
         * Exclusive access for unmounting purposes
         */
@@ -679,8 +685,10 @@ dounmount(struct mount *mp, int flags)
        error = lockmgr(&mp->mnt_lock, lflags);
        if (error) {
                mp->mnt_kern_flag &= ~(MNTK_UNMOUNT | MNTK_UNMOUNTF);
-               if (mp->mnt_kern_flag & MNTK_MWAIT)
+               if (mp->mnt_kern_flag & MNTK_MWAIT) {
+                       mp->mnt_kern_flag &= ~MNTK_MWAIT;
                        wakeup(mp);
+               }
                goto out;
        }
 
@@ -767,8 +775,10 @@ dounmount(struct mount *mp, int flags)
                mp->mnt_kern_flag &= ~(MNTK_UNMOUNT | MNTK_UNMOUNTF);
                mp->mnt_flag |= async_flag;
                lockmgr(&mp->mnt_lock, LK_RELEASE);
-               if (mp->mnt_kern_flag & MNTK_MWAIT)
+               if (mp->mnt_kern_flag & MNTK_MWAIT) {
+                       mp->mnt_kern_flag &= ~MNTK_MWAIT;
                        wakeup(mp);
+               }
                goto out;
        }
        /*
@@ -807,12 +817,29 @@ dounmount(struct mount *mp, int flags)
        if (!TAILQ_EMPTY(&mp->mnt_nvnodelist))
                panic("unmount: dangling vnode");
        lockmgr(&mp->mnt_lock, LK_RELEASE);
-       if (mp->mnt_kern_flag & MNTK_MWAIT)
+       if (mp->mnt_kern_flag & MNTK_MWAIT) {
+               mp->mnt_kern_flag &= ~MNTK_MWAIT;
                wakeup(mp);
-       if (freeok)
+       }
+
+       /*
+        * If we reach here and freeok != 0 we must free the mount.
+        * If refs > 1 cycle and wait, just in case someone tried
+        * to busy the mount after we decided to do the unmount.
+        */
+       if (freeok) {
+               while (mp->mnt_refs > 1) {
+                       wakeup(mp);
+                       tsleep(&mp->mnt_refs, 0, "umntrwait", hz / 10 + 1);
+               }
+               lwkt_reltoken(&mp->mnt_token);
                kfree(mp, M_MOUNT);
+               mp = NULL;
+       }
        error = 0;
 out:
+       if (mp)
+               lwkt_reltoken(&mp->mnt_token);
        lwkt_reltoken(&mntvnode_token);
        return (error);
 }
index 2c5dae5..99e51d4 100644 (file)
@@ -293,6 +293,7 @@ nvtruncbuf_bp_trunc(struct buf *bp, void *data)
                bp->b_flags |= (B_INVAL | B_RELBUF | B_NOCACHE);
                brelse(bp);
        }
+       lwkt_yield();
        return(1);
 }
 
@@ -306,6 +307,7 @@ nvtruncbuf_bp_metasync_cmp(struct buf *bp, void *data __unused)
 {
        if (bp->b_loffset < 0)
                return(0);
+       lwkt_yield();
        return(1);
 }
 
@@ -332,6 +334,7 @@ nvtruncbuf_bp_metasync(struct buf *bp, void *data)
                bremfree(bp);
                bawrite(bp);
        }
+       lwkt_yield();
        return(1);
 }
 
@@ -470,6 +473,7 @@ nvnode_pager_setsize(struct vnode *vp, off_t length, int blksize, int boff)
                                vm_page_wakeup(m);
                        }
                        ++pi;
+                       lwkt_yield();
                }
        } else {
                /*
index 0f67881..b2050e7 100644 (file)
@@ -326,6 +326,7 @@ tmpfs_unmount(struct mount *mp, int mntflags)
        LIST_FOREACH(node, &tmp->tm_nodes_used, tn_entries) {
                if (node->tn_type == VREG && node->tn_vnode) {
                        ++node->tn_links;
+                       lwkt_yield();
                        TMPFS_NODE_LOCK(node);
                        vx_get(node->tn_vnode);
                        tmpfs_truncate(node->tn_vnode, 0);
@@ -348,6 +349,7 @@ tmpfs_unmount(struct mount *mp, int mntflags)
         */
        LIST_FOREACH(node, &tmp->tm_nodes_used, tn_entries) {
                ++node->tn_links;
+               lwkt_yield();
                TMPFS_NODE_LOCK(node);
                if (node->tn_type == VDIR) {
                        struct tmpfs_dirent *de;
@@ -387,6 +389,7 @@ tmpfs_unmount(struct mount *mp, int mntflags)
                TMPFS_NODE_LOCK(node);
                tmpfs_free_node(tmp, node);
                /* eats lock */
+               lwkt_yield();
        }
        KKASSERT(tmp->tm_root == NULL);
 
index 672998d..bd09a92 100644 (file)
@@ -95,11 +95,17 @@ do_vmtotal(SYSCTL_HANDLER_ARGS)
 {
        struct vmtotal total;
        struct vmtotal *totalp;
+       struct vm_object marker;
        vm_object_t object;
+       long collisions;
 
        bzero(&total, sizeof(total));
        totalp = &total;
+       bzero(&marker, sizeof(marker));
+       marker.type = OBJT_MARKER;
+       collisions = vmobj_token.t_collisions;
 
+#if 0
        /*
         * Mark all objects as inactive.
         */
@@ -112,6 +118,7 @@ do_vmtotal(SYSCTL_HANDLER_ARGS)
                vm_object_clear_flag(object, OBJ_ACTIVE);
        }
        lwkt_reltoken(&vmobj_token);
+#endif
 
        /*
         * Calculate process statistics.
@@ -122,6 +129,8 @@ do_vmtotal(SYSCTL_HANDLER_ARGS)
         * Calculate object memory usage statistics.
         */
        lwkt_gettoken(&vmobj_token);
+       TAILQ_INSERT_HEAD(&vm_object_list, &marker, object_list);
+
        for (object = TAILQ_FIRST(&vm_object_list);
            object != NULL;
            object = TAILQ_NEXT(object, object_list)) {
@@ -160,8 +169,25 @@ do_vmtotal(SYSCTL_HANDLER_ARGS)
                                totalp->t_armshr += object->resident_page_count;
                        }
                }
+
+               /*
+                * Don't hog the vmobj_token if someone else wants it.
+                */
+               if (collisions != vmobj_token.t_collisions) {
+                       TAILQ_REMOVE(&vm_object_list, &marker, object_list);
+                       TAILQ_INSERT_AFTER(&vm_object_list, object,
+                                          &marker, object_list);
+                       tsleep(&vm_object_list, 0, "breath", 1);
+                       object = &marker;
+                       collisions = vmobj_token.t_collisions;
+               } else {
+                       lwkt_yield();
+               }
        }
+
+       TAILQ_REMOVE(&vm_object_list, &marker, object_list);
        lwkt_reltoken(&vmobj_token);
+
        totalp->t_free = vmstats.v_free_count + vmstats.v_cache_count;
 
        return (sysctl_handle_opaque(oidp, totalp, sizeof total, req));