Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bc960c7251781f912d2d0d4271702d15f19fb34a.camel@kernel.org>
Subject: Re: [PATCH v5 3/3] locks: allow support for write delegation
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Dai Ngo <dai.ngo@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Date:   Wed, 24 May 2023 12:55:41 -0400
In-Reply-To: <D8739068-BCAD-4E47-A2E2-1467F9DC32ED@oracle.com>
References: <1684799560-31663-1-git-send-email-dai.ngo@oracle.com>
         <1684799560-31663-4-git-send-email-dai.ngo@oracle.com>
         <32e880c5f66ce8a6f343c01416fcc8b791cc1302.camel@kernel.org>
         <D8739068-BCAD-4E47-A2E2-1467F9DC32ED@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.1 (3.48.1-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-05-24 at 15:09 +0000, Chuck Lever III wrote:
>=20
> > On May 24, 2023, at 11:08 AM, Jeff Layton <jlayton@kernel.org> wrote:
> >=20
> > On Mon, 2023-05-22 at 16:52 -0700, Dai Ngo wrote:
> > > Remove the check for F_WRLCK in generic_add_lease to allow file_lock
> > > to be used for write delegation.
> > >=20
> > > First consumer is NFSD.
> > >=20
> > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> > > ---
> > > fs/locks.c | 7 -------
> > > 1 file changed, 7 deletions(-)
> > >=20
> > > diff --git a/fs/locks.c b/fs/locks.c
> > > index df8b26a42524..08fb0b4fd4f8 100644
> > > --- a/fs/locks.c
> > > +++ b/fs/locks.c
> > > @@ -1729,13 +1729,6 @@ generic_add_lease(struct file *filp, long arg,=
 struct file_lock **flp, void **pr
> > > if (is_deleg && !inode_trylock(inode))
> > > return -EAGAIN;
> > >=20
> > > - if (is_deleg && arg =3D=3D F_WRLCK) {
> > > - /* Write delegations are not currently supported: */
> > > - inode_unlock(inode);
> > > - WARN_ON_ONCE(1);
> > > - return -EINVAL;
> > > - }
> > > -
> > > percpu_down_read(&file_rwsem);
> > > spin_lock(&ctx->flc_lock);
> > > time_out_leases(inode, &dispose);
> >=20
> > I'd probably move this back to the first patch in the series.
> >=20
> > Reviewed-by: Jeff Layton <jlayton@kernel.org>
>=20
> I asked him to move it to the end. Is it safe to take out this
> check before write delegation is actually implemented?
>=20

I think so, but it don't think it doesn't make much difference either
way. The only real downside of putting it at the end is that you might
have to contend with a WARN_ON_ONCE if you're bisecting.
--=20
Jeff Layton <jlayton@kernel.org>