On 2/18/21, Konstantin Belousov <k...@freebsd.org> wrote:
> The branch main has been updated by kib:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=fa3bd463cee5c3abeac29a83dc86eb3abfa97b06
>
> commit fa3bd463cee5c3abeac29a83dc86eb3abfa97b06
> Author: Konstantin Belousov <k...@freebsd.org>
> AuthorDate: 2021-01-29 23:48:55 +0000
> Commit: Konstantin Belousov <k...@freebsd.org>
> CommitDate: 2021-02-17 23:22:05 +0000
>
> lockf: ensure atomicity of lockf for open(O_CREAT|O_EXCL|O_EXLOCK)
>
> or EX_SHLOCK. Do it by setting a vnode iflag indicating that the
> locking
> exclusive open is in progress, and not allowing F_LOCK request to make
> a progress until the first open finishes.
>
> Requested by: mckusick
> Reviewed by: markj, mckusick
> Tested by: pho
> Sponsored by: The FreeBSD Foundation
> MFC after: 1 week
> Differential Revision: https://reviews.freebsd.org/D28697
> ---
> sys/kern/vfs_default.c | 15 +++++++++++++++
> sys/kern/vfs_vnops.c | 23 ++++++++++++++++++++---
> sys/sys/fcntl.h | 1 +
> sys/sys/vnode.h | 2 ++
> 4 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
> index 382fbb2d9ace..3c428d7b7511 100644
> --- a/sys/kern/vfs_default.c
> +++ b/sys/kern/vfs_default.c
> @@ -423,10 +423,25 @@ int
> vop_stdadvlock(struct vop_advlock_args *ap)
> {
> struct vnode *vp;
> + struct mount *mp;
> struct vattr vattr;
> int error;
>
> vp = ap->a_vp;
> +
> + /*
> + * Provide atomicity of open(O_CREAT | O_EXCL | O_EXLOCK) for
> + * local filesystems. See vn_open_cred() for reciprocal part.
> + */
> + mp = vp->v_mount;
> + if (mp != NULL && (mp->mnt_flag & MNT_LOCAL) != 0 &&
> + ap->a_op == F_SETLK && (ap->a_flags & F_FIRSTOPEN) == 0) {
This should check for FIRSTOPEN first, which will be least likely of
the entire bunch.
> + VI_LOCK(vp);
> + while ((vp->v_iflag & VI_FOPENING) != 0)
> + msleep(vp, VI_MTX(vp), PLOCK, "lockfo", 0);
> + VI_UNLOCK(vp);
> + }
> +
> if (ap->a_fl->l_whence == SEEK_END) {
> /*
> * The NFSv4 server must avoid doing a vn_lock() here, since it
> diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
> index 71dd379558cb..781968f2db53 100644
> --- a/sys/kern/vfs_vnops.c
> +++ b/sys/kern/vfs_vnops.c
> @@ -228,8 +228,10 @@ vn_open_cred(struct nameidata *ndp, int *flagp, int
> cmode, u_int vn_open_flags,
> struct vattr vat;
> struct vattr *vap = &vat;
> int fmode, error;
> + bool first_open;
>
> restart:
> + first_open = false;
> fmode = *flagp;
> if ((fmode & (O_CREAT | O_EXCL | O_DIRECTORY)) == (O_CREAT |
> O_EXCL | O_DIRECTORY))
> @@ -275,8 +277,16 @@ restart:
> #endif
> error = VOP_CREATE(ndp->ni_dvp, &ndp->ni_vp,
> &ndp->ni_cnd, vap);
> - VOP_VPUT_PAIR(ndp->ni_dvp, error == 0 ? &ndp->ni_vp :
> - NULL, false);
> + vp = ndp->ni_vp;
> + if (error == 0 && (fmode & O_EXCL) != 0 &&
> + (fmode & (O_EXLOCK | O_SHLOCK)) != 0) {
> + VI_LOCK(vp);
> + vp->v_iflag |= VI_FOPENING;
> + VI_UNLOCK(vp);
> + first_open = true;
> + }
error tends to be 0, so this can inspect fmode first
> + VOP_VPUT_PAIR(ndp->ni_dvp, error == 0 ? &vp : NULL,
> + false);
> vn_finished_write(mp);
> if (error) {
> NDFREE(ndp, NDF_ONLY_PNBUF);
> @@ -287,7 +297,6 @@ restart:
> return (error);
> }
> fmode &= ~O_TRUNC;
> - vp = ndp->ni_vp;
> } else {
> if (ndp->ni_dvp == ndp->ni_vp)
> vrele(ndp->ni_dvp);
> @@ -317,6 +326,12 @@ restart:
> vp = ndp->ni_vp;
> }
> error = vn_open_vnode(vp, fmode, cred, td, fp);
> + if (first_open) {
> + VI_LOCK(vp);
> + vp->v_iflag &= ~VI_FOPENING;
> + wakeup(vp);
> + VI_UNLOCK(vp);
> + }
> if (error)
> goto bad;
> *flagp = fmode;
> @@ -352,6 +367,8 @@ vn_open_vnode_advlock(struct vnode *vp, int fmode,
> struct file *fp)
> type = F_FLOCK;
> if ((fmode & FNONBLOCK) == 0)
> type |= F_WAIT;
> + if ((fmode & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
> + type |= F_FIRSTOPEN;
> error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type);
> if (error == 0)
> fp->f_flag |= FHASLOCK;
> diff --git a/sys/sys/fcntl.h b/sys/sys/fcntl.h
> index 3c29c04e46db..70e68246be13 100644
> --- a/sys/sys/fcntl.h
> +++ b/sys/sys/fcntl.h
> @@ -287,6 +287,7 @@ typedef __pid_t pid_t;
> #define F_POSIX 0x040 /* Use POSIX semantics for lock
> */
> #define F_REMOTE 0x080 /* Lock owner is remote NFS
> client */
> #define F_NOINTR 0x100 /* Ignore signals when waiting
> */
> +#define F_FIRSTOPEN 0x200 /* First right to advlock file
> */
> #endif
>
> /*
> diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
> index 639a16881e09..9d68f9e236f6 100644
> --- a/sys/sys/vnode.h
> +++ b/sys/sys/vnode.h
> @@ -254,6 +254,8 @@ struct xvnode {
> #define VI_DOINGINACT 0x0004 /* VOP_INACTIVE is in progress */
> #define VI_OWEINACT 0x0008 /* Need to call inactive */
> #define VI_DEFINACT 0x0010 /* deferred inactive */
> +#define VI_FOPENING 0x0020 /* In open, with opening process having
> the
> + first right to advlock file */
The flag was not added to vn_printf
>
> #define VV_ROOT 0x0001 /* root of its filesystem */
> #define VV_ISTTY 0x0002 /* vnode represents a tty */
> _______________________________________________
> dev-commits-src-...@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all
> To unsubscribe, send any mail to
> "dev-commits-src-all-unsubscr...@freebsd.org"
>
--
Mateusz Guzik <mjguzik gmail.com>
_______________________________________________
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"
