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 v2 26/27] NFSD: Add tracepoints in the NFS dispatcher
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <20200925141716.GC1096@fieldses.org>
Date:   Fri, 25 Sep 2020 10:21:40 -0400
Cc:     Bill Baker <Bill.Baker@oracle.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <39D76A18-F134-4A3C-9B50-FEE46564E945@oracle.com>
References: <160071167664.1468.1365570508917640511.stgit@klimt.1015granger.net>
 <160071198717.1468.14262284967190973528.stgit@klimt.1015granger.net>
 <20200924234526.GB12407@fieldses.org>
 <801F3A94-4668-4DF6-9CAF-27171EEBA17A@oracle.com>
 <20200925141716.GC1096@fieldses.org>
To:     Bruce Fields <bfields@fieldses.org>
Precedence: bulk


> On Sep 25, 2020, at 10:17 AM, Bruce Fields <bfields@fieldses.org> =
wrote:
>=20
> On Fri, Sep 25, 2020 at 09:59:54AM -0400, Chuck Lever wrote:
>>=20
>>=20
>>> On Sep 24, 2020, at 7:45 PM, J. Bruce Fields <bfields@fieldses.org> =
wrote:
>>>=20
>>> On Mon, Sep 21, 2020 at 02:13:07PM -0400, Chuck Lever wrote:
>>>> This is follow-on work to the tracepoints added in the NFS server's
>>>> duplicate reply cache. Here, tracepoints are introduced that report
>>>> replies from cache as well as encoding and decoding errors.
>>>>=20
>>>> The NFSv2, v3, and v4 dispatcher requirements have diverged over
>>>> time, leaving us with a little bit of technical debt. In addition,
>>>> I wanted to add a tracepoint for NFSv2 and NFSv3 similar to the
>>>> nfsd4_compound/compoundstatus tracepoints. Lastly, removing some
>>>> conditional branches from this hot path helps optimize CPU
>>>> utilization. So, I've duplicated the nfsd_dispatcher function.
>>>=20
>>> Comparing current nfsd_dispatch to the nfsv2/v3 nfsd_dispatch: the =
only
>>> thing I spotted removed from the v2/v3-specific dispatch is the
>>> rq_lease_breaker =3D NULL.  (I think that's not correct, actually.  =
We
>>> could remove the need for that to be set in the v2/v3 case, but with =
the
>>> current code it does need to be set.)
>>=20
>> Noted with thanks.
>=20
> Maybe just do this?

I'll try it out. Thanks!


> --b.
>=20
> commit c7265a111269
> 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..a498278ba96b 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4597,6 +4597,9 @@ static bool nfsd_breaker_owns_lease(struct =
file_lock *fl)
>=20
> 	if (!i_am_nfsd())
> 		return NULL;
> +	/* Note rq_prog =3D=3D NFS_ACL_PROGRAM is also possible: */
> +	if (rqst->rq_prog !=3D NFS_PROGRAM || rqst->rq_vers < 4)
> +		return NULL;
> 	rqst =3D kthread_data(current);
> 	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