The branch main has been updated by jah:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=6d3e78ad6c11849ad3d36feeb10c412c93b56073

commit 6d3e78ad6c11849ad3d36feeb10c412c93b56073
Author:     Jason A. Harmening <j...@freebsd.org>
AuthorDate: 2021-05-11 15:54:58 +0000
Commit:     Jason A. Harmening <j...@freebsd.org>
CommitDate: 2021-05-29 21:05:39 +0000

    VFS_QUOTACTL(9): allow implementation to indicate busy state changes
    
    Instead of requiring all implementations of vfs_quotactl to unbusy
    the mount for Q_QUOTAON and Q_QUOTAOFF, add an "mp_busy" in/out param
    to VFS_QUOTACTL(9).  The implementation may then indicate to the caller
    whether it needed to unbusy the mount.
    
    Reviewed By:    kib, markj
    Differential Revision: https://reviews.freebsd.org/D30218
---
 share/man/man9/VFS_QUOTACTL.9                      | 32 ++++++++++++++++++++--
 .../openzfs/module/os/freebsd/zfs/zfs_vfsops.c     | 15 ++++++++++
 sys/fs/nullfs/null_vfsops.c                        | 30 ++++++++++++++++++--
 sys/fs/smbfs/smbfs_vfsops.c                        |  3 +-
 sys/fs/unionfs/union_vfsops.c                      | 28 +++++++++++++++++--
 sys/kern/vfs_default.c                             |  4 +--
 sys/kern/vfs_init.c                                |  6 ++--
 sys/kern/vfs_syscalls.c                            | 19 ++++++-------
 sys/sys/mount.h                                    |  7 +++--
 sys/sys/param.h                                    |  2 +-
 sys/ufs/ufs/quota.h                                |  2 +-
 sys/ufs/ufs/ufs_quota.c                            | 21 ++++++--------
 sys/ufs/ufs/ufs_vfsops.c                           | 19 +++++--------
 13 files changed, 135 insertions(+), 53 deletions(-)

diff --git a/share/man/man9/VFS_QUOTACTL.9 b/share/man/man9/VFS_QUOTACTL.9
index 8d0cb113ce1e..210f71631353 100644
--- a/share/man/man9/VFS_QUOTACTL.9
+++ b/share/man/man9/VFS_QUOTACTL.9
@@ -28,7 +28,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd December 17, 2020
+.Dd May 29, 2021
 .Dt VFS_QUOTACTL 9
 .Os
 .Sh NAME
@@ -39,12 +39,38 @@
 .In sys/mount.h
 .In sys/vnode.h
 .Ft int
-.Fn VFS_QUOTACTL "struct mount *mp" "int cmds" "uid_t uid" "void *arg"
+.Fn VFS_QUOTACTL "struct mount *mp" "int cmds" "uid_t uid" "void *arg" "bool 
*mp_busy"
 .Sh DESCRIPTION
 Implement file system quotas.
+.Pp
+The
+.Fa mp_busy
+argument is an input/output parameter.
+.Fn VFS_QUOTACTL
+must be called with
+.Fa mp
+marked busy through
+.Xr vfs_busy 9
+and
+.Fa *mp_busy
+set to true.
+The filesystem implementation of
+.Fn VFS_QUOTACTL
+may then unbusy
+.Fa mp
+using
+.Xr vfs_unbusy 9
+prior to performing quota file I/O.
+In this case the implementation must set
+.Fa *mp_busy
+to false to indicate that the caller must not unbusy
+.Fa mp
+upon completion of
+.Fn VFS_QUOTACTL .
+.Pp
 See
 .Xr quotactl 2
-for a description of the arguments.
+for a description of the remaining arguments.
 .Sh SEE ALSO
 .Xr quotactl 2 ,
 .Xr vnode 9
diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c 
b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c
index a537342f9678..4f2d7df87fc0 100644
--- a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c
+++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c
@@ -102,7 +102,12 @@ SYSCTL_INT(_vfs_zfs_version, OID_AUTO, zpl, CTLFLAG_RD, 
&zfs_version_zpl, 0,
     "ZPL_VERSION");
 /* END CSTYLED */
 
+#if __FreeBSD_version >= 1400018
+static int zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg,
+    bool *mp_busy);
+#else
 static int zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg);
+#endif
 static int zfs_mount(vfs_t *vfsp);
 static int zfs_umount(vfs_t *vfsp, int fflag);
 static int zfs_root(vfs_t *vfsp, int flags, vnode_t **vpp);
@@ -267,7 +272,11 @@ done:
 }
 
 static int
+#if __FreeBSD_version >= 1400018
+zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg, bool *mp_busy)
+#else
 zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg)
+#endif
 {
        zfsvfs_t *zfsvfs = vfsp->vfs_data;
        struct thread *td;
@@ -291,8 +300,10 @@ zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg)
                        break;
                default:
                        error = EINVAL;
+#if __FreeBSD_version < 1400018
                        if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF)
                                vfs_unbusy(vfsp);
+#endif
                        goto done;
                }
        }
@@ -351,11 +362,15 @@ zfs_quotactl(vfs_t *vfsp, int cmds, uid_t id, void *arg)
        case Q_QUOTAON:
                // As far as I can tell, you can't turn quotas on or off on zfs
                error = 0;
+#if __FreeBSD_version < 1400018
                vfs_unbusy(vfsp);
+#endif
                break;
        case Q_QUOTAOFF:
                error = ENOTSUP;
+#if __FreeBSD_version < 1400018
                vfs_unbusy(vfsp);
+#endif
                break;
        case Q_SETQUOTA:
                error = copyin(arg, &dqblk, sizeof (dqblk));
diff --git a/sys/fs/nullfs/null_vfsops.c b/sys/fs/nullfs/null_vfsops.c
index 0bb98072edf4..0ad2385116a9 100644
--- a/sys/fs/nullfs/null_vfsops.c
+++ b/sys/fs/nullfs/null_vfsops.c
@@ -294,13 +294,39 @@ nullfs_root(mp, flags, vpp)
 }
 
 static int
-nullfs_quotactl(mp, cmd, uid, arg)
+nullfs_quotactl(mp, cmd, uid, arg, mp_busy)
        struct mount *mp;
        int cmd;
        uid_t uid;
        void *arg;
+       bool *mp_busy;
 {
-       return VFS_QUOTACTL(MOUNTTONULLMOUNT(mp)->nullm_vfs, cmd, uid, arg);
+       struct mount *lowermp;
+       struct null_mount *mntdata;
+       int error;
+       bool unbusy;
+
+       mntdata = MOUNTTONULLMOUNT(mp);
+       lowermp = atomic_load_ptr(&mntdata->nullm_vfs);
+       KASSERT(*mp_busy == true, ("upper mount not busy"));
+       /*
+        * See comment in sys_quotactl() for an explanation of why the
+        * lower mount needs to be busied by the caller of VFS_QUOTACTL()
+        * but may be unbusied by the implementation.  We must unbusy
+        * the upper mount for the same reason; otherwise a namei lookup
+        * issued by the VFS_QUOTACTL() implementation could traverse the
+        * upper mount and deadlock.
+        */
+       vfs_unbusy(mp);
+       *mp_busy = false;
+       unbusy = true;
+       error = vfs_busy(lowermp, 0);
+       if (error == 0)
+               error = VFS_QUOTACTL(lowermp, cmd, uid, arg, &unbusy);
+       if (unbusy)
+               vfs_unbusy(lowermp);
+
+       return (error);
 }
 
 static int
diff --git a/sys/fs/smbfs/smbfs_vfsops.c b/sys/fs/smbfs/smbfs_vfsops.c
index d19816a7869c..a1ae565c6341 100644
--- a/sys/fs/smbfs/smbfs_vfsops.c
+++ b/sys/fs/smbfs/smbfs_vfsops.c
@@ -352,11 +352,12 @@ out:
  */
 /* ARGSUSED */
 static int
-smbfs_quotactl(mp, cmd, uid, arg)
+smbfs_quotactl(mp, cmd, uid, arg, mp_busy)
        struct mount *mp;
        int cmd;
        uid_t uid;
        void *arg;
+       bool *mp_busy;
 {
        SMBVDEBUG("return EOPNOTSUPP\n");
        return EOPNOTSUPP;
diff --git a/sys/fs/unionfs/union_vfsops.c b/sys/fs/unionfs/union_vfsops.c
index ae95bd9c005c..bd264c7bcdb5 100644
--- a/sys/fs/unionfs/union_vfsops.c
+++ b/sys/fs/unionfs/union_vfsops.c
@@ -371,16 +371,38 @@ unionfs_root(struct mount *mp, int flags, struct vnode 
**vpp)
 }
 
 static int
-unionfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg)
+unionfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg,
+    bool *mp_busy)
 {
+       struct mount *uppermp;
        struct unionfs_mount *ump;
+       int error;
+       bool unbusy;
 
        ump = MOUNTTOUNIONFSMOUNT(mp);
-
+       uppermp = atomic_load_ptr(&ump->um_uppervp->v_mount);
+       KASSERT(*mp_busy == true, ("upper mount not busy"));
+       /*
+        * See comment in sys_quotactl() for an explanation of why the
+        * lower mount needs to be busied by the caller of VFS_QUOTACTL()
+        * but may be unbusied by the implementation.  We must unbusy
+        * the upper mount for the same reason; otherwise a namei lookup
+        * issued by the VFS_QUOTACTL() implementation could traverse the
+        * upper mount and deadlock.
+        */
+       vfs_unbusy(mp);
+       *mp_busy = false;
+       unbusy = true;
+       error = vfs_busy(uppermp, 0);
        /*
         * Writing is always performed to upper vnode.
         */
-       return (VFS_QUOTACTL(ump->um_uppervp->v_mount, cmd, uid, arg));
+       if (error == 0)
+               error = VFS_QUOTACTL(uppermp, cmd, uid, arg, &unbusy);
+       if (unbusy)
+               vfs_unbusy(uppermp);
+
+       return (error);
 }
 
 static int
diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
index ace9ad1d37c3..63bca7810847 100644
--- a/sys/kern/vfs_default.c
+++ b/sys/kern/vfs_default.c
@@ -1350,13 +1350,13 @@ vfs_stdstatfs (mp, sbp)
 }
 
 int
-vfs_stdquotactl (mp, cmds, uid, arg)
+vfs_stdquotactl (mp, cmds, uid, arg, mp_busy)
        struct mount *mp;
        int cmds;
        uid_t uid;
        void *arg;
+       bool *mp_busy;
 {
-
        return (EOPNOTSUPP);
 }
 
diff --git a/sys/kern/vfs_init.c b/sys/kern/vfs_init.c
index 3365ddb11474..112b4c76e575 100644
--- a/sys/kern/vfs_init.c
+++ b/sys/kern/vfs_init.c
@@ -212,12 +212,14 @@ vfs_cachedroot_sigdefer(struct mount *mp, int flags, 
struct vnode **vpp)
 }
 
 static int
-vfs_quotactl_sigdefer(struct mount *mp, int cmd, uid_t uid, void *arg)
+vfs_quotactl_sigdefer(struct mount *mp, int cmd, uid_t uid, void *arg,
+    bool *mp_busy)
 {
        int prev_stops, rc;
 
        prev_stops = sigdeferstop(SIGDEFERSTOP_SILENT);
-       rc = (*mp->mnt_vfc->vfc_vfsops_sd->vfs_quotactl)(mp, cmd, uid, arg);
+       rc = (*mp->mnt_vfc->vfc_vfsops_sd->vfs_quotactl)(mp, cmd, uid, arg,
+           mp_busy);
        sigallowstop(prev_stops);
        return (rc);
 }
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index 55780b0474ee..2f4a6036ef88 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -89,8 +89,6 @@ __FBSDID("$FreeBSD$");
 
 #include <fs/devfs/devfs.h>
 
-#include <ufs/ufs/quota.h>
-
 MALLOC_DEFINE(M_FADVISE, "fadvise", "posix_fadvise(2) information");
 
 static int kern_chflagsat(struct thread *td, int fd, const char *path,
@@ -195,6 +193,7 @@ sys_quotactl(struct thread *td, struct quotactl_args *uap)
        struct mount *mp;
        struct nameidata nd;
        int error;
+       bool mp_busy;
 
        AUDIT_ARG_CMD(uap->cmd);
        AUDIT_ARG_UID(uap->uid);
@@ -213,21 +212,21 @@ sys_quotactl(struct thread *td, struct quotactl_args *uap)
                vfs_rel(mp);
                return (error);
        }
-       error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg);
+       mp_busy = true;
+       error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg, &mp_busy);
 
        /*
-        * Since quota on operation typically needs to open quota
-        * file, the Q_QUOTAON handler needs to unbusy the mount point
+        * Since quota on/off operations typically need to open quota
+        * files, the implementation may need to unbusy the mount point
         * before calling into namei.  Otherwise, unmount might be
-        * started between two vfs_busy() invocations (first is our,
+        * started between two vfs_busy() invocations (first is ours,
         * 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.
+        * Avoid unbusying mp if the implementation indicates it has
+        * already done so.
         */
-       if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON &&
-           (uap->cmd >> SUBCMDSHIFT) != Q_QUOTAOFF)
+       if (mp_busy)
                vfs_unbusy(mp);
        vfs_rel(mp);
        return (error);
diff --git a/sys/sys/mount.h b/sys/sys/mount.h
index a1d4bfd15ddb..684d8c3eb780 100644
--- a/sys/sys/mount.h
+++ b/sys/sys/mount.h
@@ -759,7 +759,8 @@ struct mntarg;
 typedef int vfs_cmount_t(struct mntarg *ma, void *data, uint64_t flags);
 typedef int vfs_unmount_t(struct mount *mp, int mntflags);
 typedef int vfs_root_t(struct mount *mp, int flags, struct vnode **vpp);
-typedef        int vfs_quotactl_t(struct mount *mp, int cmds, uid_t uid, void 
*arg);
+typedef        int vfs_quotactl_t(struct mount *mp, int cmds, uid_t uid, void 
*arg,
+                   bool *mp_busy);
 typedef        int vfs_statfs_t(struct mount *mp, struct statfs *sbp);
 typedef        int vfs_sync_t(struct mount *mp, int waitfor);
 typedef        int vfs_vget_t(struct mount *mp, ino_t ino, int flags,
@@ -832,10 +833,10 @@ vfs_statfs_t      __vfs_statfs;
        _rc = (*(MP)->mnt_op->vfs_cachedroot)(MP, FLAGS, VPP);          \
        _rc; })
 
-#define        VFS_QUOTACTL(MP, C, U, A) ({                                    
\
+#define        VFS_QUOTACTL(MP, C, U, A, MP_BUSY) ({                           
\
        int _rc;                                                        \
                                                                        \
-       _rc = (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A);               \
+       _rc = (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A, MP_BUSY);      \
        _rc; })
 
 #define        VFS_STATFS(MP, SBP) ({                                          
\
diff --git a/sys/sys/param.h b/sys/sys/param.h
index 959f0b94ca70..c63452973daf 100644
--- a/sys/sys/param.h
+++ b/sys/sys/param.h
@@ -76,7 +76,7 @@
  * cannot include sys/param.h and should only be updated here.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 1400017
+#define __FreeBSD_version 1400018
 
 /*
  * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,
diff --git a/sys/ufs/ufs/quota.h b/sys/ufs/ufs/quota.h
index e154f8234705..eb3db9c300d0 100644
--- a/sys/ufs/ufs/quota.h
+++ b/sys/ufs/ufs/quota.h
@@ -232,7 +232,7 @@ int getinoquota(struct inode *);
 int    qsync(struct mount *);
 int    qsyncvp(struct vnode *);
 int    quotaoff(struct thread *, struct mount *, int);
-int    quotaon(struct thread *, struct mount *, int, void *);
+int    quotaon(struct thread *, struct mount *, int, void *, bool *);
 int    getquota32(struct thread *, struct mount *, u_long, int, void *);
 int    setquota32(struct thread *, struct mount *, u_long, int, void *);
 int    setuse32(struct thread *, struct mount *, u_long, int, void *);
diff --git a/sys/ufs/ufs/ufs_quota.c b/sys/ufs/ufs/ufs_quota.c
index 4dff74f75945..143e0afbf1e3 100644
--- a/sys/ufs/ufs/ufs_quota.c
+++ b/sys/ufs/ufs/ufs_quota.c
@@ -492,7 +492,8 @@ chkdquot(struct inode *ip)
  * Q_QUOTAON - set up a quota file for a particular filesystem.
  */
 int
-quotaon(struct thread *td, struct mount *mp, int type, void *fname)
+quotaon(struct thread *td, struct mount *mp, int type, void *fname,
+    bool *mp_busy)
 {
        struct ufsmount *ump;
        struct vnode *vp, **vpp;
@@ -502,15 +503,11 @@ quotaon(struct thread *td, struct mount *mp, int type, 
void *fname)
        struct nameidata nd;
 
        error = priv_check(td, PRIV_UFS_QUOTAON);
-       if (error != 0) {
-               vfs_unbusy(mp);
+       if (error != 0)
                return (error);
-       }
 
-       if ((mp->mnt_flag & MNT_RDONLY) != 0) {
-               vfs_unbusy(mp);
+       if ((mp->mnt_flag & MNT_RDONLY) != 0)
                return (EROFS);
-       }
 
        ump = VFSTOUFS(mp);
        dq = NODQUOT;
@@ -518,7 +515,9 @@ quotaon(struct thread *td, struct mount *mp, int type, void 
*fname)
        NDINIT(&nd, LOOKUP, FOLLOW, UIO_USERSPACE, fname, td);
        flags = FREAD | FWRITE;
        vfs_ref(mp);
+       KASSERT(*mp_busy, ("%s called without busied mount", __func__));
        vfs_unbusy(mp);
+       *mp_busy = false;
        error = vn_open(&nd, &flags, 0, NULL);
        if (error != 0) {
                vfs_rel(mp);
@@ -529,10 +528,9 @@ quotaon(struct thread *td, struct mount *mp, int type, 
void *fname)
        error = vfs_busy(mp, MBF_NOWAIT);
        vfs_rel(mp);
        if (error == 0) {
-               if (vp->v_type != VREG) {
+               *mp_busy = true;
+               if (vp->v_type != VREG)
                        error = EACCES;
-                       vfs_unbusy(mp);
-               }
        }
        if (error != 0) {
                VOP_UNLOCK(vp);
@@ -545,7 +543,6 @@ quotaon(struct thread *td, struct mount *mp, int type, void 
*fname)
                UFS_UNLOCK(ump);
                VOP_UNLOCK(vp);
                (void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
-               vfs_unbusy(mp);
                return (EALREADY);
        }
        ump->um_qflags[type] |= QTF_OPENING|QTF_CLOSING;
@@ -556,7 +553,6 @@ quotaon(struct thread *td, struct mount *mp, int type, void 
*fname)
                ump->um_qflags[type] &= ~(QTF_OPENING|QTF_CLOSING);
                UFS_UNLOCK(ump);
                (void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
-               vfs_unbusy(mp);
                return (error);
        }
        VOP_UNLOCK(vp);
@@ -640,7 +636,6 @@ again:
                ("quotaon: leaking flags"));
        UFS_UNLOCK(ump);
 
-       vfs_unbusy(mp);
        return (error);
 }
 
diff --git a/sys/ufs/ufs/ufs_vfsops.c b/sys/ufs/ufs/ufs_vfsops.c
index 0f45baed634f..33ef7bc2c3d1 100644
--- a/sys/ufs/ufs/ufs_vfsops.c
+++ b/sys/ufs/ufs/ufs_vfsops.c
@@ -87,17 +87,14 @@ ufs_root(mp, flags, vpp)
  * Do operations associated with quotas
  */
 int
-ufs_quotactl(mp, cmds, id, arg)
+ufs_quotactl(mp, cmds, id, arg, mp_busy)
        struct mount *mp;
        int cmds;
        uid_t id;
        void *arg;
+       bool *mp_busy;
 {
 #ifndef QUOTA
-       if ((cmds >> SUBCMDSHIFT) == Q_QUOTAON ||
-           (cmds >> SUBCMDSHIFT) == Q_QUOTAOFF)
-               vfs_unbusy(mp);
-
        return (EOPNOTSUPP);
 #else
        struct thread *td;
@@ -117,25 +114,23 @@ ufs_quotactl(mp, cmds, id, arg)
                        break;
 
                default:
-                       if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF)
-                               vfs_unbusy(mp);
                        return (EINVAL);
                }
        }
-       if ((u_int)type >= MAXQUOTAS) {
-               if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF)
-                       vfs_unbusy(mp);
+       if ((u_int)type >= MAXQUOTAS)
                return (EINVAL);
-       }
 
        switch (cmd) {
        case Q_QUOTAON:
-               error = quotaon(td, mp, type, arg);
+               error = quotaon(td, mp, type, arg, mp_busy);
                break;
 
        case Q_QUOTAOFF:
                vfs_ref(mp);
+               KASSERT(*mp_busy,
+                   ("%s called without busied mount", __func__));
                vfs_unbusy(mp);
+               *mp_busy = false;
                vn_start_write(NULL, &mp, V_WAIT | V_MNTREF);
                error = quotaoff(td, mp, type);
                vn_finished_write(mp);
_______________________________________________
dev-commits-src-main@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"

Reply via email to