From owner-svn-src-all@FreeBSD.ORG Sun Jan 8 23:06:53 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7E934106566B; Sun, 8 Jan 2012 23:06:53 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 696228FC13; Sun, 8 Jan 2012 23:06:53 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q08N6r2u011771; Sun, 8 Jan 2012 23:06:53 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q08N6rDQ011768; Sun, 8 Jan 2012 23:06:53 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <201201082306.q08N6rDQ011768@svn.freebsd.org> From: Konstantin Belousov Date: Sun, 8 Jan 2012 23:06:53 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r229828 - in head/sys: kern ufs/ufs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Jan 2012 23:06:53 -0000 Author: kib Date: Sun Jan 8 23:06:53 2012 New Revision: 229828 URL: http://svn.freebsd.org/changeset/base/229828 Log: Avoid LOR between vfs_busy() lock and covered vnode lock on quotaon(). The vfs_busy() is after covered vnode lock in the global lock order, but since quotaon() does recursive VFS call to open quota file, we usually end up locking covered vnode after mp is busied in sys_quotactl(). Change the interface of VFS_QUOTACTL(), requiring that mp was unbusied by fs code, and do not try to pick up vfs_busy() reference in ufs quotaon, esp. if vfs_busy cannot succeed due to unmount being performed. Reported and tested by: pho MFC after: 1 week Modified: head/sys/kern/vfs_syscalls.c head/sys/ufs/ufs/ufs_quota.c Modified: head/sys/kern/vfs_syscalls.c ============================================================================== --- head/sys/kern/vfs_syscalls.c Sun Jan 8 23:05:36 2012 (r229827) +++ head/sys/kern/vfs_syscalls.c Sun Jan 8 23:06:53 2012 (r229828) @@ -86,6 +86,8 @@ __FBSDID("$FreeBSD$"); #include #include +#include + static MALLOC_DEFINE(M_FADVISE, "fadvise", "posix_fadvise(2) information"); SDT_PROVIDER_DEFINE(vfs); @@ -212,7 +214,20 @@ sys_quotactl(td, uap) return (error); } error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg); - vfs_unbusy(mp); + + /* + * Since quota on operation typically needs to open quota + * file, the Q_QUOTAON handler needs to unbusy the mount point + * before calling into namei. Otherwise, unmount might be + * started between two vfs_busy() invocations (first is our, + * second is from mount point cross-walk code in lookup()), + * causing deadlock. + * + * Require that Q_QUOTAON handles the vfs_busy() reference on + * its own, always returning with ubusied mount point. + */ + if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON) + vfs_unbusy(mp); VFS_UNLOCK_GIANT(vfslocked); return (error); } Modified: head/sys/ufs/ufs/ufs_quota.c ============================================================================== --- head/sys/ufs/ufs/ufs_quota.c Sun Jan 8 23:05:36 2012 (r229827) +++ head/sys/ufs/ufs/ufs_quota.c Sun Jan 8 23:06:53 2012 (r229828) @@ -512,17 +512,29 @@ quotaon(struct thread *td, struct mount NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE, UIO_USERSPACE, fname, td); flags = FREAD | FWRITE; + vfs_ref(mp); + vfs_unbusy(mp); error = vn_open(&nd, &flags, 0, NULL); - if (error) + if (error != 0) { + vfs_rel(mp); return (error); + } vfslocked = NDHASGIANT(&nd); NDFREE(&nd, NDF_ONLY_PNBUF); vp = nd.ni_vp; - if (vp->v_type != VREG) { + error = vfs_busy(mp, MBF_NOWAIT); + vfs_rel(mp); + if (error == 0) { + if (vp->v_type != VREG) { + error = EACCES; + vfs_unbusy(mp); + } + } + if (error != 0) { VOP_UNLOCK(vp, 0); (void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td); VFS_UNLOCK_GIANT(vfslocked); - return (EACCES); + return (error); } UFS_LOCK(ump); @@ -531,6 +543,7 @@ quotaon(struct thread *td, struct mount VOP_UNLOCK(vp, 0); (void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td); VFS_UNLOCK_GIANT(vfslocked); + vfs_unbusy(mp); return (EALREADY); } ump->um_qflags[type] |= QTF_OPENING|QTF_CLOSING; @@ -542,6 +555,7 @@ quotaon(struct thread *td, struct mount UFS_UNLOCK(ump); (void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td); VFS_UNLOCK_GIANT(vfslocked); + vfs_unbusy(mp); return (error); } VOP_UNLOCK(vp, 0); @@ -619,6 +633,7 @@ again: ("quotaon: leaking flags")); UFS_UNLOCK(ump); + vfs_unbusy(mp); return (error); }