From owner-svn-src-head@FreeBSD.ORG Wed Oct 16 00:58:48 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 7A3472F8; Wed, 16 Oct 2013 00:58:48 +0000 (UTC) (envelope-from neel@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 66515242D; Wed, 16 Oct 2013 00:58:48 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r9G0wmUb036035; Wed, 16 Oct 2013 00:58:48 GMT (envelope-from neel@svn.freebsd.org) Received: (from neel@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r9G0wmOJ036034; Wed, 16 Oct 2013 00:58:48 GMT (envelope-from neel@svn.freebsd.org) Message-Id: <201310160058.r9G0wmOJ036034@svn.freebsd.org> From: Neel Natu Date: Wed, 16 Oct 2013 00:58:48 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r256570 - head/sys/amd64/vmm X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Oct 2013 00:58:48 -0000 Author: neel Date: Wed Oct 16 00:58:47 2013 New Revision: 256570 URL: http://svnweb.freebsd.org/changeset/base/256570 Log: Fix the witness warning that warned against calling uiomove() while holding the 'vmmdev_mtx' in vmmdev_rw(). Rely on the 'si_threadcount' accounting to ensure that we never destroy the VM device node while it has operations in progress (e.g. ioctl, mmap etc). Reported by: grehan Reviewed by: grehan Modified: head/sys/amd64/vmm/vmm_dev.c Modified: head/sys/amd64/vmm/vmm_dev.c ============================================================================== --- head/sys/amd64/vmm/vmm_dev.c Tue Oct 15 23:56:31 2013 (r256569) +++ head/sys/amd64/vmm/vmm_dev.c Wed Oct 16 00:58:47 2013 (r256570) @@ -60,7 +60,10 @@ struct vmmdev_softc { struct vm *vm; /* vm instance cookie */ struct cdev *cdev; SLIST_ENTRY(vmmdev_softc) link; + int flags; }; +#define VSC_LINKED 0x01 + static SLIST_HEAD(, vmmdev_softc) head; static struct mtx vmmdev_mtx; @@ -104,7 +107,6 @@ vmmdev_rw(struct cdev *cdev, struct uio static char zerobuf[PAGE_SIZE]; error = 0; - mtx_lock(&vmmdev_mtx); sc = vmmdev_lookup2(cdev); if (sc == NULL) error = ENXIO; @@ -134,8 +136,6 @@ vmmdev_rw(struct cdev *cdev, struct uio vm_gpa_release(cookie); } } - - mtx_unlock(&vmmdev_mtx); return (error); } @@ -379,34 +379,28 @@ vmmdev_mmap_single(struct cdev *cdev, vm int error; struct vmmdev_softc *sc; - mtx_lock(&vmmdev_mtx); - sc = vmmdev_lookup2(cdev); if (sc != NULL && (nprot & PROT_EXEC) == 0) error = vm_get_memobj(sc->vm, *offset, size, offset, object); else error = EINVAL; - mtx_unlock(&vmmdev_mtx); - return (error); } static void -vmmdev_destroy(struct vmmdev_softc *sc, boolean_t unlink) +vmmdev_destroy(void *arg) { - /* - * XXX must stop virtual machine instances that may be still - * running and cleanup their state. - */ - if (sc->cdev) + struct vmmdev_softc *sc = arg; + + if (sc->cdev != NULL) destroy_dev(sc->cdev); - if (sc->vm) + if (sc->vm != NULL) vm_destroy(sc->vm); - if (unlink) { + if ((sc->flags & VSC_LINKED) != 0) { mtx_lock(&vmmdev_mtx); SLIST_REMOVE(&head, sc, vmmdev_softc, link); mtx_unlock(&vmmdev_mtx); @@ -421,27 +415,38 @@ sysctl_vmm_destroy(SYSCTL_HANDLER_ARGS) int error; char buf[VM_MAX_NAMELEN]; struct vmmdev_softc *sc; + struct cdev *cdev; strlcpy(buf, "beavis", sizeof(buf)); error = sysctl_handle_string(oidp, buf, sizeof(buf), req); if (error != 0 || req->newptr == NULL) return (error); - /* - * XXX TODO if any process has this device open then fail - */ - mtx_lock(&vmmdev_mtx); sc = vmmdev_lookup(buf); - if (sc == NULL) { + if (sc == NULL || sc->cdev == NULL) { mtx_unlock(&vmmdev_mtx); return (EINVAL); } - sc->cdev->si_drv1 = NULL; + /* + * The 'cdev' will be destroyed asynchronously when 'si_threadcount' + * goes down to 0 so we should not do it again in the callback. + */ + cdev = sc->cdev; + sc->cdev = NULL; mtx_unlock(&vmmdev_mtx); - vmmdev_destroy(sc, TRUE); + /* + * Schedule the 'cdev' to be destroyed: + * + * - any new operations on this 'cdev' will return an error (ENXIO). + * + * - when the 'si_threadcount' dwindles down to zero the 'cdev' will + * be destroyed and the callback will be invoked in a taskqueue + * context. + */ + destroy_dev_sched_cb(cdev, vmmdev_destroy, sc); return (0); } @@ -462,6 +467,7 @@ sysctl_vmm_create(SYSCTL_HANDLER_ARGS) { int error; struct vm *vm; + struct cdev *cdev; struct vmmdev_softc *sc, *sc2; char buf[VM_MAX_NAMELEN]; @@ -489,22 +495,28 @@ sysctl_vmm_create(SYSCTL_HANDLER_ARGS) */ mtx_lock(&vmmdev_mtx); sc2 = vmmdev_lookup(buf); - if (sc2 == NULL) + if (sc2 == NULL) { SLIST_INSERT_HEAD(&head, sc, link); + sc->flags |= VSC_LINKED; + } mtx_unlock(&vmmdev_mtx); if (sc2 != NULL) { - vmmdev_destroy(sc, FALSE); + vmmdev_destroy(sc); return (EEXIST); } - error = make_dev_p(MAKEDEV_CHECKNAME, &sc->cdev, &vmmdevsw, NULL, + error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmdevsw, NULL, UID_ROOT, GID_WHEEL, 0600, "vmm/%s", buf); if (error != 0) { - vmmdev_destroy(sc, TRUE); + vmmdev_destroy(sc); return (error); } + + mtx_lock(&vmmdev_mtx); + sc->cdev = cdev; sc->cdev->si_drv1 = sc; + mtx_unlock(&vmmdev_mtx); return (0); }