Date: Sun, 30 May 2021 00:47:39 GMT From: "Jason A. Harmening" <jah@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 271fcf1c28ef - main - Revert commits 6d3e78ad6c11 and 54256e7954d7 Message-ID: <202105300047.14U0ldeD038999@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=271fcf1c28efd22342212b1ccaa3dba4fc059627 commit 271fcf1c28efd22342212b1ccaa3dba4fc059627 Author: Jason A. Harmening <jah@FreeBSD.org> AuthorDate: 2021-05-30 00:46:46 +0000 Commit: Jason A. Harmening <jah@FreeBSD.org> CommitDate: 2021-05-30 00:48:02 +0000 Revert commits 6d3e78ad6c11 and 54256e7954d7 Parts of libprocstat like to pretend they're kernel components for the sake of including mount.h, and including sys/types.h in the _KERNEL case doesn't fix the build for some reason. Revert both the VFS_QUOTACTL() change and the follow-up "fix" for now. --- 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 | 9 ++---- 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, 53 insertions(+), 137 deletions(-) diff --git a/share/man/man9/VFS_QUOTACTL.9 b/share/man/man9/VFS_QUOTACTL.9 index 210f71631353..8d0cb113ce1e 100644 --- a/share/man/man9/VFS_QUOTACTL.9 +++ b/share/man/man9/VFS_QUOTACTL.9 @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd May 29, 2021 +.Dd December 17, 2020 .Dt VFS_QUOTACTL 9 .Os .Sh NAME @@ -39,38 +39,12 @@ .In sys/mount.h .In sys/vnode.h .Ft int -.Fn VFS_QUOTACTL "struct mount *mp" "int cmds" "uid_t uid" "void *arg" "bool *mp_busy" +.Fn VFS_QUOTACTL "struct mount *mp" "int cmds" "uid_t uid" "void *arg" .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 remaining arguments. +for a description of the 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 4f2d7df87fc0..a537342f9678 100644 --- a/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c +++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vfsops.c @@ -102,12 +102,7 @@ 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); @@ -272,11 +267,7 @@ 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; @@ -300,10 +291,8 @@ 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; } } @@ -362,15 +351,11 @@ 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 0ad2385116a9..0bb98072edf4 100644 --- a/sys/fs/nullfs/null_vfsops.c +++ b/sys/fs/nullfs/null_vfsops.c @@ -294,39 +294,13 @@ nullfs_root(mp, flags, vpp) } static int -nullfs_quotactl(mp, cmd, uid, arg, mp_busy) +nullfs_quotactl(mp, cmd, uid, arg) struct mount *mp; int cmd; uid_t uid; void *arg; - bool *mp_busy; { - 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); + return VFS_QUOTACTL(MOUNTTONULLMOUNT(mp)->nullm_vfs, cmd, uid, arg); } static int diff --git a/sys/fs/smbfs/smbfs_vfsops.c b/sys/fs/smbfs/smbfs_vfsops.c index a1ae565c6341..d19816a7869c 100644 --- a/sys/fs/smbfs/smbfs_vfsops.c +++ b/sys/fs/smbfs/smbfs_vfsops.c @@ -352,12 +352,11 @@ out: */ /* ARGSUSED */ static int -smbfs_quotactl(mp, cmd, uid, arg, mp_busy) +smbfs_quotactl(mp, cmd, uid, arg) 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 bd264c7bcdb5..ae95bd9c005c 100644 --- a/sys/fs/unionfs/union_vfsops.c +++ b/sys/fs/unionfs/union_vfsops.c @@ -371,38 +371,16 @@ unionfs_root(struct mount *mp, int flags, struct vnode **vpp) } static int -unionfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg, - bool *mp_busy) +unionfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg) { - 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. */ - if (error == 0) - error = VFS_QUOTACTL(uppermp, cmd, uid, arg, &unbusy); - if (unbusy) - vfs_unbusy(uppermp); - - return (error); + return (VFS_QUOTACTL(ump->um_uppervp->v_mount, cmd, uid, arg)); } static int diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c index 63bca7810847..ace9ad1d37c3 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, mp_busy) +vfs_stdquotactl (mp, cmds, uid, arg) 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 112b4c76e575..3365ddb11474 100644 --- a/sys/kern/vfs_init.c +++ b/sys/kern/vfs_init.c @@ -212,14 +212,12 @@ 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, - bool *mp_busy) +vfs_quotactl_sigdefer(struct mount *mp, int cmd, uid_t uid, void *arg) { int prev_stops, rc; prev_stops = sigdeferstop(SIGDEFERSTOP_SILENT); - rc = (*mp->mnt_vfc->vfc_vfsops_sd->vfs_quotactl)(mp, cmd, uid, arg, - mp_busy); + rc = (*mp->mnt_vfc->vfc_vfsops_sd->vfs_quotactl)(mp, cmd, uid, arg); sigallowstop(prev_stops); return (rc); } diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 2f4a6036ef88..55780b0474ee 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -89,6 +89,8 @@ __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, @@ -193,7 +195,6 @@ 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); @@ -212,21 +213,21 @@ sys_quotactl(struct thread *td, struct quotactl_args *uap) vfs_rel(mp); return (error); } - mp_busy = true; - error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg, &mp_busy); + error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg); /* - * Since quota on/off operations typically need to open quota - * files, the implementation may need to unbusy the mount point + * 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 ours, + * started between two vfs_busy() invocations (first is our, * second is from mount point cross-walk code in lookup()), * causing deadlock. * - * Avoid unbusying mp if the implementation indicates it has - * already done so. + * Require that Q_QUOTAON handles the vfs_busy() reference on + * its own, always returning with ubusied mount point. */ - if (mp_busy) + if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON && + (uap->cmd >> SUBCMDSHIFT) != Q_QUOTAOFF) vfs_unbusy(mp); vfs_rel(mp); return (error); diff --git a/sys/sys/mount.h b/sys/sys/mount.h index 6c1cd82ee84f..a1d4bfd15ddb 100644 --- a/sys/sys/mount.h +++ b/sys/sys/mount.h @@ -43,8 +43,6 @@ #include <sys/tslog.h> #include <sys/_mutex.h> #include <sys/_sx.h> -#else -#include <stdbool.h> #endif /* @@ -761,8 +759,7 @@ 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, - bool *mp_busy); +typedef int vfs_quotactl_t(struct mount *mp, int cmds, uid_t uid, void *arg); 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, @@ -835,10 +832,10 @@ vfs_statfs_t __vfs_statfs; _rc = (*(MP)->mnt_op->vfs_cachedroot)(MP, FLAGS, VPP); \ _rc; }) -#define VFS_QUOTACTL(MP, C, U, A, MP_BUSY) ({ \ +#define VFS_QUOTACTL(MP, C, U, A) ({ \ int _rc; \ \ - _rc = (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A, MP_BUSY); \ + _rc = (*(MP)->mnt_op->vfs_quotactl)(MP, C, U, A); \ _rc; }) #define VFS_STATFS(MP, SBP) ({ \ diff --git a/sys/sys/param.h b/sys/sys/param.h index c63452973daf..959f0b94ca70 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 1400018 +#define __FreeBSD_version 1400017 /* * __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 eb3db9c300d0..e154f8234705 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 *, bool *); +int quotaon(struct thread *, struct mount *, int, void *); 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 143e0afbf1e3..4dff74f75945 100644 --- a/sys/ufs/ufs/ufs_quota.c +++ b/sys/ufs/ufs/ufs_quota.c @@ -492,8 +492,7 @@ 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, - bool *mp_busy) +quotaon(struct thread *td, struct mount *mp, int type, void *fname) { struct ufsmount *ump; struct vnode *vp, **vpp; @@ -503,11 +502,15 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname, struct nameidata nd; error = priv_check(td, PRIV_UFS_QUOTAON); - if (error != 0) + if (error != 0) { + vfs_unbusy(mp); return (error); + } - if ((mp->mnt_flag & MNT_RDONLY) != 0) + if ((mp->mnt_flag & MNT_RDONLY) != 0) { + vfs_unbusy(mp); return (EROFS); + } ump = VFSTOUFS(mp); dq = NODQUOT; @@ -515,9 +518,7 @@ 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); @@ -528,9 +529,10 @@ quotaon(struct thread *td, struct mount *mp, int type, void *fname, error = vfs_busy(mp, MBF_NOWAIT); vfs_rel(mp); if (error == 0) { - *mp_busy = true; - if (vp->v_type != VREG) + if (vp->v_type != VREG) { error = EACCES; + vfs_unbusy(mp); + } } if (error != 0) { VOP_UNLOCK(vp); @@ -543,6 +545,7 @@ 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; @@ -553,6 +556,7 @@ 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); @@ -636,6 +640,7 @@ 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 33ef7bc2c3d1..0f45baed634f 100644 --- a/sys/ufs/ufs/ufs_vfsops.c +++ b/sys/ufs/ufs/ufs_vfsops.c @@ -87,14 +87,17 @@ ufs_root(mp, flags, vpp) * Do operations associated with quotas */ int -ufs_quotactl(mp, cmds, id, arg, mp_busy) +ufs_quotactl(mp, cmds, id, arg) 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; @@ -114,23 +117,25 @@ ufs_quotactl(mp, cmds, id, arg, mp_busy) break; default: + if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF) + vfs_unbusy(mp); return (EINVAL); } } - if ((u_int)type >= MAXQUOTAS) + if ((u_int)type >= MAXQUOTAS) { + if (cmd == Q_QUOTAON || cmd == Q_QUOTAOFF) + vfs_unbusy(mp); return (EINVAL); + } switch (cmd) { case Q_QUOTAON: - error = quotaon(td, mp, type, arg, mp_busy); + error = quotaon(td, mp, type, arg); 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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105300047.14U0ldeD038999>