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: <82827bf5e90d761e1eb263603d729199e3401cd7.camel@kernel.org>
Subject: Re: [PATCH RFC 8/9] NFSD: Clean up nfs4_preprocess_stateid_op()
 call sites
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Date:   Thu, 06 Oct 2022 12:05:29 -0400
In-Reply-To: <166498178682.1527.2423899374993202065.stgit@manet.1015granger.net>
References: <166497916751.1527.11190362197003358927.stgit@manet.1015granger.net>
         <166498178682.1527.2423899374993202065.stgit@manet.1015granger.net>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-10-05 at 10:56 -0400, Chuck Lever wrote:
> Remove the lame-duck dprintk()s around nfs4_preprocess_stateid_op()
> call sites.
>=20
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4proc.c |   31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
>=20
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index d0d976f847ca..42b81e88ea14 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -951,12 +951,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_comp=
ound_state *cstate,
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_f=
h,
>  					&read->rd_stateid, RD_STATE,
>  					&read->rd_nf, NULL);
> -	if (status) {
> -		dprintk("NFSD: nfsd4_read: couldn't process stateid!\n");
> -		goto out;
> -	}
> -	status =3D nfs_ok;
> -out:
> +
>  	read->rd_rqstp =3D rqstp;
>  	read->rd_fhp =3D &cstate->current_fh;
>  	return status;
> @@ -1125,10 +1120,8 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4=
_compound_state *cstate,
>  		status =3D nfs4_preprocess_stateid_op(rqstp, cstate,
>  				&cstate->current_fh, &setattr->sa_stateid,
>  				WR_STATE, NULL, NULL);
> -		if (status) {
> -			dprintk("NFSD: nfsd4_setattr: couldn't process stateid!\n");
> +		if (status)
>  			return status;
> -		}
>  	}
>  	err =3D fh_want_write(&cstate->current_fh);
>  	if (err)
> @@ -1176,10 +1169,8 @@ nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_c=
ompound_state *cstate,
>  			       write->wr_offset, cnt);
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_f=
h,
>  						stateid, WR_STATE, &nf, NULL);
> -	if (status) {
> -		dprintk("NFSD: nfsd4_write: couldn't process stateid!\n");
> +	if (status)
>  		return status;
> -	}
> =20
>  	write->wr_how_written =3D write->wr_stable_how;
> =20
> @@ -1210,17 +1201,13 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct =
nfsd4_compound_state *cstate,
> =20
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
>  					    src_stateid, RD_STATE, src, NULL);
> -	if (status) {
> -		dprintk("NFSD: %s: couldn't process src stateid!\n", __func__);
> +	if (status)
>  		goto out;
> -	}
> =20
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_f=
h,
>  					    dst_stateid, WR_STATE, dst, NULL);
> -	if (status) {
> -		dprintk("NFSD: %s: couldn't process dst stateid!\n", __func__);
> +	if (status)
>  		goto out_put_src;
> -	}
> =20
>  	/* fix up for NFS-specific error code */
>  	if (!S_ISREG(file_inode((*src)->nf_file)->i_mode) ||
> @@ -1955,10 +1942,8 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfs=
d4_compound_state *cstate,
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_f=
h,
>  					    &fallocate->falloc_stateid,
>  					    WR_STATE, &nf, NULL);
> -	if (status !=3D nfs_ok) {
> -		dprintk("NFSD: nfsd4_fallocate: couldn't process stateid!\n");
> +	if (status !=3D nfs_ok)
>  		return status;
> -	}
> =20
>  	status =3D nfsd4_vfs_fallocate(rqstp, &cstate->current_fh, nf->nf_file,
>  				     fallocate->falloc_offset,
> @@ -2014,10 +1999,8 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_co=
mpound_state *cstate,
>  	status =3D nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_f=
h,
>  					    &seek->seek_stateid,
>  					    RD_STATE, &nf, NULL);
> -	if (status) {
> -		dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n");
> +	if (status)
>  		return status;
> -	}
> =20
>  	switch (seek->seek_whence) {
>  	case NFS4_CONTENT_DATA:
>=20
>=20

I agree that these are not particularly helpful. It might be a good idea
to add a conditional tracepoint in nfs4_preprocess_stateid_op though,
for when it returns an error, but I don't care enough to stand in the
way of this decluttering.

Reviewed-by: Jeff Layton <jlayton@kernel.org>