Date: Thu, 10 Mar 2016 18:29:16 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Chris Torek <torek@mail.torek.net> Cc: freebsd-fs@freebsd.org Subject: Re: quotactl bug: vfs_busy never unbusy-es Message-ID: <20160310162916.GB1741@kib.kiev.ua> In-Reply-To: <201603101138.u2ABcihi048011@elf.torek.net> References: <20160310091433.GS67250@kib.kiev.ua> <201603101138.u2ABcihi048011@elf.torek.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Mar 10, 2016 at 03:38:44AM -0800, Chris Torek wrote:
> That, in fact, is what I started with, before I investigated
> further and found the nullfs and unionfs pattern violation.
>
> It's certainly worth doing as a start, though.
Lets fix nullfs too. The uncomfortable issue with the interface is that
it is not always possible to busy mount point after unbusy. I propose
to assume that for ENOENT error the busy ref is lost (this is the only
possible error from vfs_busy()). I also verified that UFS quota code
does not return this error for !quotaon case.
diff --git a/sys/fs/nullfs/null_vfsops.c b/sys/fs/nullfs/null_vfsops.c
index 49bae28..36e44cf 100644
--- a/sys/fs/nullfs/null_vfsops.c
+++ b/sys/fs/nullfs/null_vfsops.c
@@ -53,6 +53,7 @@
#include <sys/jail.h>
#include <fs/nullfs/null.h>
+#include <ufs/ufs/quota.h>
static MALLOC_DEFINE(M_NULLFSMNT, "nullfs_mount", "NULLFS mount structure");
@@ -285,13 +286,33 @@ nullfs_root(mp, flags, vpp)
}
static int
-nullfs_quotactl(mp, cmd, uid, arg)
- struct mount *mp;
- int cmd;
- uid_t uid;
- void *arg;
+nullfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg)
{
- return VFS_QUOTACTL(MOUNTTONULLMOUNT(mp)->nullm_vfs, cmd, uid, arg);
+ struct mount *lmp;
+ int error, error1;
+
+ /*
+ * Nullfs mp is busy, which ensures that the lower mount is
+ * valid.
+ */
+ lmp = MOUNTTONULLMOUNT(mp)->nullm_vfs;
+ vfs_ref(mp);
+ vfs_ref(lmp);
+ vfs_unbusy(mp);
+ error = vfs_busy(lmp, 0);
+ if (error == 0) {
+ error = VFS_QUOTACTL(lmp, cmd, uid, arg);
+ if ((cmd >> SUBCMDSHIFT) != Q_QUOTAON && error != ENOENT) {
+ vfs_unbusy(lmp);
+ error1 = vfs_busy(mp, 0);
+ if (error1 != 0)
+ error = error1;
+ }
+ }
+ vfs_rel(mp);
+ vfs_rel(lmp);
+ return (error);
+
}
static int
diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
index a7977bf..26e45f4 100644
--- a/sys/kern/vfs_default.c
+++ b/sys/kern/vfs_default.c
@@ -66,6 +66,8 @@ __FBSDID("$FreeBSD$");
#include <vm/vm_pager.h>
#include <vm/vnode_pager.h>
+#include <ufs/ufs/quota.h>
+
static int vop_nolookup(struct vop_lookup_args *);
static int vop_norename(struct vop_rename_args *);
static int vop_nostrategy(struct vop_strategy_args *);
@@ -1190,6 +1192,8 @@ vfs_stdquotactl (mp, cmds, uid, arg)
void *arg;
{
+ if ((cmds >> SUBCMDSHIFT) == Q_QUOTAON)
+ vfs_unbusy(mp);
return (EOPNOTSUPP);
}
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 11813fc..1ca9d97 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -198,7 +198,7 @@ sys_quotactl(td, 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 && error != ENOENT)
vfs_unbusy(mp);
return (error);
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160310162916.GB1741>