From owner-dev-commits-src-all@freebsd.org Thu Feb 18 01:16:59 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id E268C5416D7; Thu, 18 Feb 2021 01:16:59 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Dgxd75tRhz4Xth; Thu, 18 Feb 2021 01:16:59 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x32a.google.com with SMTP id m1so443102wml.2; Wed, 17 Feb 2021 17:16:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=qhtvHO+deyRPjvZXBIfnP+dlgYrn+n5NJGIhVMJSDC0=; b=okd2O9mITdpEtFFPXsuhMxK8dw1hpCoFXtRE+7J54fuy4RMvMLYg/shi+lOMDoaC9N pwRZO9gEdzBRiMga5kHqZQtpwkHk2dit2JfohH6+3jhRFn+qibNy+ZniPReU8Tw1EsyS lVJP1e1+AHrH68RuZCmpb2ySVG+HagLoi2O9RmsACzAH9PYrP67sxDCSIkD4wAHa596U y6Gu+DHpCkFQzb6+ixabyEZ53c6zF0g87i+36aI0sW1E38fRSGdBFWwcQvALBzVIVtvf wqyye9zWUQhPDxlckn/HUfxwLQtLWdxDkBKhlr3cPptqJeWd/9jDGe5QeBkTStJYfJxK j/zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=qhtvHO+deyRPjvZXBIfnP+dlgYrn+n5NJGIhVMJSDC0=; b=NRE3Wo4X0Acas9kmQaQXHNVVIiWwwhpYZScwajVIWJGg2ryce1FAqp38MXfi5w7ACZ kQ0IQg+qzwRRZHocqiRUQn8vXYXOCQIZbSypgUNP2OzxGTLBw0zfYv6F47Uhvs2wbHUL o5oJEYLIw0fts80U8vwkt0VFOf8xET0JvPPqStHqkLgCBhclaitkIUuFSF55WUN1ZnS5 XAnzv8JUUfLil2OnDE2WuCf2Zf/SH+2u0e3uk8uJ9VTCi/yWAdFShnmhYbevK9B3EV/t XAGOW59mjp1rLt3podWooujFjw69Nw4AwFy7n9CXtiryLZfaZzWL42MSyIiIHYP17okH vTlQ== X-Gm-Message-State: AOAM53124xTZA5hgpVffjL2iaXKiceyF8G+xYRGBtvop5aEaypZ3NJS7 IvcxeE/reduFev/NSEI+nHAvxYmbrghEwTm+HCL7Jkoe X-Google-Smtp-Source: ABdhPJx4I7jR5MqBs1hmdrE33tX4b4I/0+qZFE7qxbhhiAYACjGo3uQpc66nHor0sHvafoKu0by0ripGdwKgHZeflfE= X-Received: by 2002:a05:600c:47c4:: with SMTP id l4mr1244769wmo.83.1613611017638; Wed, 17 Feb 2021 17:16:57 -0800 (PST) MIME-Version: 1.0 Received: by 2002:adf:e406:0:0:0:0:0 with HTTP; Wed, 17 Feb 2021 17:16:56 -0800 (PST) In-Reply-To: <202102172323.11HNN1xo077930@gitrepo.freebsd.org> References: <202102172323.11HNN1xo077930@gitrepo.freebsd.org> From: Mateusz Guzik Date: Thu, 18 Feb 2021 02:16:56 +0100 Message-ID: Subject: Re: git: fa3bd463cee5 - main - lockf: ensure atomicity of lockf for open(O_CREAT|O_EXCL|O_EXLOCK) To: Konstantin Belousov Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4Dgxd75tRhz4Xth X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Feb 2021 01:16:59 -0000 On 2/18/21, Konstantin Belousov wrote: > The branch main has been updated by kib: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=fa3bd463cee5c3abeac29a83dc86eb3abfa97b06 > > commit fa3bd463cee5c3abeac29a83dc86eb3abfa97b06 > Author: Konstantin Belousov > AuthorDate: 2021-01-29 23:48:55 +0000 > Commit: Konstantin Belousov > 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-all@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all > To unsubscribe, send any mail to > "dev-commits-src-all-unsubscribe@freebsd.org" > -- Mateusz Guzik