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: <aef89de72767d582ad1726becfd8c7169769b8b2.camel@kernel.org>
Subject: Re: [PATCH 4/8] NFSD: only call fh_unlock() once in nfsd_link()
From:   Jeff Layton <jlayton@kernel.org>
To:     NeilBrown <neilb@suse.de>, Chuck Lever <chuck.lever@oracle.com>
Cc:     linux-nfs@vger.kernel.org
Date:   Wed, 06 Jul 2022 09:31:48 -0400
In-Reply-To: <165708109258.1940.6581517569232462503.stgit@noble.brown>
References: <165708033167.1940.3364591321728458949.stgit@noble.brown>
         <165708109258.1940.6581517569232462503.stgit@noble.brown>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-07-06 at 14:18 +1000, NeilBrown wrote:
> On non-error paths, nfsd_link() calls fh_unlock() twice.  This is safe
> because fh_unlock() records that the unlock has been done and doesn't
> repeat it.
> However it makes the code a little confusing and interferes with changes
> that are planned for directory locking.
>=20
> So rearrange the code to ensure fh_unlock() is called exactly once if
> fh_lock() was called.
>=20
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/vfs.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>=20
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 3f4579f5775c..4916c29af0fa 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1551,8 +1551,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *f=
fhp,
> =20
>  	dnew =3D lookup_one_len(name, ddir, len);
>  	host_err =3D PTR_ERR(dnew);
> -	if (IS_ERR(dnew))
> -		goto out_nfserr;
> +	if (IS_ERR(dnew)) {
> +		err =3D nfserrno(host_err);
> +		goto out_unlock;
> +	}
> =20
>  	dold =3D tfhp->fh_dentry;
> =20
> @@ -1571,17 +1573,17 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *=
ffhp,
>  		else
>  			err =3D nfserrno(host_err);
>  	}
> -out_dput:
>  	dput(dnew);
> -out_unlock:
> -	fh_unlock(ffhp);
> +out_drop_write:
>  	fh_drop_write(tfhp);
>  out:
>  	return err;
> =20
> -out_nfserr:
> -	err =3D nfserrno(host_err);
> -	goto out_unlock;
> +out_dput:
> +	dput(dnew);
> +out_unlock:
> +	fh_unlock(ffhp);
> +	goto out_drop_write;
>  }
> =20
>  static void
>=20
>=20

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