Date: Wed, 19 Sep 2018 14:36:58 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r338798 - in head/sys: kern ufs/ufs Message-ID: <201809191436.w8JEawx4000600@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Wed Sep 19 14:36:57 2018 New Revision: 338798 URL: https://svnweb.freebsd.org/changeset/base/338798 Log: Fix state of dquot-less vnodes after failed quotaoff. UFS quotaoff iterates over all mp vnodes, and derefences and clears the pointers to corresponding dquots. If SU work items transiently reference some of dquots,quotaoff() would eventually fail, but all processed vnodes are already stripped from dquots. The state is problematic, since quotas are left enabled, but there is no dquots where blocks and inodes can be accounted. The result is assertion failures and NULL pointer dereferences. Fix it by suspending writes around quotaoff() call. Since the filesystem is synced, no dandling references to dquots from SU workitems can left behind, which means that quotaoff succeeds. The complication there is that quotaoff VFS op is performed with the mount point busied, while to suspend, we need to start write on the mp. If vn_start_write() is called on busied mp, system might deadlock against parallel unmount request. Handle this by unbusy-ing mp before starting write, which in turn requires changing the quotaoff() interface to return with the mount point not busied, same as was done for quotaon(). Reviewed by: mckusick Reported and tested by: pho Sponsored by: The FreeBSD Foundation Approved by: re (gjb) MFC after: 1 week Differential revision: https://reviews.freebsd.org/D17208 Modified: head/sys/kern/vfs_syscalls.c head/sys/ufs/ufs/ufs_quota.c head/sys/ufs/ufs/ufs_vfsops.c Modified: head/sys/kern/vfs_syscalls.c ============================================================================== --- head/sys/kern/vfs_syscalls.c Wed Sep 19 10:26:45 2018 (r338797) +++ head/sys/kern/vfs_syscalls.c Wed Sep 19 14:36:57 2018 (r338798) @@ -190,7 +190,8 @@ sys_quotactl(struct thread *td, struct quotactl_args * * Require that Q_QUOTAON handles the vfs_busy() reference on * its own, always returning with ubusied mount point. */ - if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON) + if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON && + (uap->cmd >> SUBCMDSHIFT) != Q_QUOTAOFF) vfs_unbusy(mp); return (error); } Modified: head/sys/ufs/ufs/ufs_quota.c ============================================================================== --- head/sys/ufs/ufs/ufs_quota.c Wed Sep 19 10:26:45 2018 (r338797) +++ head/sys/ufs/ufs/ufs_quota.c Wed Sep 19 14:36:57 2018 (r338798) @@ -712,6 +712,34 @@ again: return (error); } +static int +quotaoff_inchange1(struct thread *td, struct mount *mp, int type) +{ + int error; + bool need_resume; + + /* + * mp is already suspended on unmount. If not, suspend it, to + * avoid the situation where quotaoff operation eventually + * failing due to SU structures still keeping references on + * dquots, but vnode's references are already clean. This + * would cause quota accounting leak and asserts otherwise. + * Note that the thread has already called vn_start_write(). + */ + if (mp->mnt_susp_owner == td) { + need_resume = false; + } else { + error = vfs_write_suspend_umnt(mp); + if (error != 0) + return (error); + need_resume = true; + } + error = quotaoff1(td, mp, type); + if (need_resume) + vfs_write_resume(mp, VR_START_WRITE); + return (error); +} + /* * Turns off quotas, assumes that ump->um_qflags are already checked * and QTF_CLOSING is set to indicate operation in progress. Fixes @@ -721,10 +749,9 @@ int quotaoff_inchange(struct thread *td, struct mount *mp, int type) { struct ufsmount *ump; - int i; - int error; + int error, i; - error = quotaoff1(td, mp, type); + error = quotaoff_inchange1(td, mp, type); ump = VFSTOUFS(mp); UFS_LOCK(ump); Modified: head/sys/ufs/ufs/ufs_vfsops.c ============================================================================== --- head/sys/ufs/ufs/ufs_vfsops.c Wed Sep 19 10:26:45 2018 (r338797) +++ head/sys/ufs/ufs/ufs_vfsops.c Wed Sep 19 14:36:57 2018 (r338798) @@ -94,7 +94,8 @@ ufs_quotactl(mp, cmds, id, arg) void *arg; { #ifndef QUOTA - if ((cmds >> SUBCMDSHIFT) == Q_QUOTAON) + if ((cmds >> SUBCMDSHIFT) == Q_QUOTAON || + (cmds >> SUBCMDSHIFT) == Q_QUOTAOFF) vfs_unbusy(mp); return (EOPNOTSUPP); @@ -117,13 +118,13 @@ ufs_quotactl(mp, cmds, id, arg) break; default: - if (cmd == Q_QUOTAON) + if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF) vfs_unbusy(mp); return (EINVAL); } } if ((u_int)type >= MAXQUOTAS) { - if (cmd == Q_QUOTAON) + if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF) vfs_unbusy(mp); return (EINVAL); } @@ -134,7 +135,11 @@ ufs_quotactl(mp, cmds, id, arg) break; case Q_QUOTAOFF: + vfs_ref(mp); + vfs_unbusy(mp); + vn_start_write(NULL, &mp, V_WAIT | V_MNTREF); error = quotaoff(td, mp, type); + vn_finished_write(mp); break; case Q_SETQUOTA32:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201809191436.w8JEawx4000600>