Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.1\))
Subject: Re: [PATCH] nfsd: rq_lease_breaker cleanup
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20200925204809.GI1096@fieldses.org>
Date:   Fri, 25 Sep 2020 16:49:57 -0400
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <5B93AC63-B81B-443B-BE28-D4F718978817@oracle.com>
References: <160106058240.10141.2317053300018495103.stgit@klimt.1015granger.net>
 <20200925204809.GI1096@fieldses.org>
To:     Bruce Fields <bfields@fieldses.org>
Precedence: bulk


> On Sep 25, 2020, at 4:48 PM, J. Bruce Fields <bfields@fieldses.org> =
wrote:
>=20
> On Fri, Sep 25, 2020 at 03:03:42PM -0400, Chuck Lever wrote:
>> From: J. Bruce Fields <bfields@redhat.com>
>>=20
>> Since only the v4 code cares about it, maybe it's better to leave
>> rq_lease_breaker out of the common dispatch code?
>>=20
>> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> fs/nfsd/nfs4state.c |    3 +++
>> fs/nfsd/nfs4xdr.c   |    1 +
>> fs/nfsd/nfssvc.c    |    2 --
>> 3 files changed, 4 insertions(+), 2 deletions(-)
>>=20
>> Hey Bruce-
>>=20
>> This seems to work a little better than the patch you sent me
>> this morning.
>=20
> Oops, right, I should have warned that was untested!  I don't know how
> it got past me that I was trying to read rqst before it was set....
>=20
> The other two lines aren't needed, though.
>=20
> (The only place we read rq_lease_breaker is in =
nfsd_breaker_owns_lease(),
> and only after we've checked that we're running as an nfsd thread
> processing an NFSv4 rpc.
>=20
> Such a thread shouldn't touch the filesystem and trigger this callback
> until it's in nfsd4_proc_compound.  Which sets rq_lease_breaker at the
> start.)
>=20
> --b.
>=20
> commit 4abef2c530f7
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Fri Sep 25 10:12:39 2020 -0400
>=20
>    nfsd: rq_lease_breaker cleanup
>=20
>    Since only the v4 code cares about it, maybe it's better to leave
>    rq_lease_breaker out of the common dispatch code?
>=20
>    Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>=20
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 62afcae18e17..c13b04718a3f 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4598,6 +4598,9 @@ static bool nfsd_breaker_owns_lease(struct =
file_lock *fl)
> 	if (!i_am_nfsd())
> 		return NULL;
> 	rqst =3D kthread_data(current);
> +	/* Note rq_prog =3D=3D NFS_ACL_PROGRAM is also possible: */
> +	if (rqst->rq_prog !=3D NFS_PROGRAM || rqst->rq_vers < 4)
> +		return NULL;

Am I missing a patch that removes

	if (!rqst->rq_lease_breaker)
		return NULL;

> 	clp =3D *(rqst->rq_lease_breaker);
> 	return dl->dl_stid.sc_client =3D=3D clp;
> }
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index b603dfcdd361..8d6f6f4c8b28 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -1016,7 +1016,6 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 =
*statp)
> 		*statp =3D rpc_garbage_args;
> 		return 1;
> 	}
> -	rqstp->rq_lease_breaker =3D NULL;
> 	/*
> 	 * Give the xdr decoder a chance to change this if it wants
> 	 * (necessary in the NFSv4.0 compound case)

--
Chuck Lever