d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix state of dquot-less vnodes after failed quotaoff.

Browse Source 
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
This commit is contained in:
Konstantin Belousov 2018-09-19 14:36:57 +00:00
parent f803ec1ef5
commit bf94d6c78b
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=338798
3 changed files with 40 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
sys/kern/vfs_syscalls.c
View File

@ -190,7 +190,8 @@ sys_quotactl(struct thread *td, struct quotactl_args *uap)
* 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);
}

33
sys/ufs/ufs/ufs_quota.c
View File

@ -712,6 +712,34 @@ quotaoff1(struct thread *td, struct mount *mp, int type)
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);

11
sys/ufs/ufs/ufs_vfsops.c
View File

@ -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: