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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Neil Brown <neilb@suse.de>
CC:     Jeff Layton <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 4/8] NFSD: only call fh_unlock() once in nfsd_link()
Thread-Topic: [PATCH 4/8] NFSD: only call fh_unlock() once in nfsd_link()
Thread-Index: AQHYkO/GCNr2yaRUtkmc1q0pviopdq1xiUGA
Date:   Wed, 6 Jul 2022 16:29:29 +0000
Message-ID: <956CFBD3-EAB7-461C-81CD-F0C4A4479807@oracle.com>
References: <165708033167.1940.3364591321728458949.stgit@noble.brown>
 <165708109258.1940.6581517569232462503.stgit@noble.brown>
In-Reply-To: <165708109258.1940.6581517569232462503.stgit@noble.brown>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?inL7NYYdh/Afp+Fe84nfw7YSvf7B632MHfakrhX+1CFmI1Wk+zwPdbDmMB2S?=
 =?us-ascii?Q?Q2j2c7Y3UbLzEEMkJxlsTyGRQKiN4UmkTvnjqEblffLstJgbz/puQM80oaXd?=
 =?us-ascii?Q?H+SlKhOYQ1cStX8hH6q1/D0uqA2fecEkUBtW/rpoR8RDWto2EJxX5jkxTH0z?=
 =?us-ascii?Q?XnQruvaibeoKPAKdJ/2R+3HlGOUxzglV8djjCjhGLWNN5xpkjPSTVsS50ih3?=
 =?us-ascii?Q?W25t+KGh1psLxokAWTveYX4rSsaTVaXszjvIj4Ggjfzp0ju2vRtrXUPHxpvl?=
 =?us-ascii?Q?crn4Ogc0fKaRdKDGbgJ+Bo9l5FHeTo7nL76PzOvicOmo6k7hVTOSgde/YZi1?=
 =?us-ascii?Q?DZRxOV4Kid+0JKtEzgQnevmIrO8ubiRAxJAT2WuEOTULChAHc3KGLig8i6sU?=
 =?us-ascii?Q?x6zTXFOjD/LkTHKmrYqrZRchQyToMlr/TWJ/maFsw321Wkrykj6l3iGro1xj?=
 =?us-ascii?Q?oNVoNVNqGG7re+abemzve9tu736cQ4Ei8SclUnUtgA+2AsABhfWdX4t6sc6m?=
 =?us-ascii?Q?wIengaizbt2sKTwg3SfHdEA7WygAQVjiL/EcE6txAFWZFkMHbHff/2cXQFZL?=
 =?us-ascii?Q?fHFH4xkY82BVKVOZ9ZZaPKy6HjSA+14wU/4u1X07F16Gg0iK11OBXzy8qt43?=
 =?us-ascii?Q?kUxtemqKXAqdo+JTv3wbEMkc0/p/3DJLT+DHY4Egw8UJX4yjEMNRgc95mw0O?=
 =?us-ascii?Q?K0e1thqEEaaefmv8Io4YXQDMR93RIEe3nO6pKmGwYIrq8In5Dnpi35EvbuIs?=
 =?us-ascii?Q?IXpQCc5jN5ukNuqfJCaAnqMv0jAoaucjCzYQ7R3U4YtiVjZWo+/fxhS7deus?=
 =?us-ascii?Q?ZL9OXvttfjFouurIsRjmOPhwZNHJxfjoDLHt1lUlHLfIMJGnzU5KCpK1YCiu?=
 =?us-ascii?Q?sh65532cWAijEU/nNCxuu5wkxIq/+qD6Lw4/khwNRWsdOu4wUNPRg9lx2qXP?=
 =?us-ascii?Q?uUqA1YJ6aSAV6QXea44xHPxuf4OnvbJ5moOeDzLTiQLgPKbR0J0bS7O4so7O?=
 =?us-ascii?Q?6NnZyBkjQSCJgiBOT6FE0jOCYaxE7pJJOl1owwdahB7PslTEkRk2glg3yLJG?=
 =?us-ascii?Q?kBUnMY08eXLRKTFLZCJkDah5vTQYXiTycN/loKrW2P5fRSzwYQgCchAk5vaI?=
 =?us-ascii?Q?GtvCM1z7BN7yimdLt6oMiGcuN15d7rffEp78KBozMFxWvHqq//6565qOtVbS?=
 =?us-ascii?Q?UQ2sN70XPNPnynVPlUXrNjwCI4RQgB4fxK+J4sLr0HM5WidRrmEsjzq/EuSW?=
 =?us-ascii?Q?nYlW+LDDmgLN0sx8ebnBLtmim9Y+NSaE8dwNvuuYtz9PUiIpD/RFu5hp72Ju?=
 =?us-ascii?Q?9JQBHbv62lCKA912P7CIboU/J/5yHNRn1H87dyQGrEXzH1MqoYwkLjAXOR+I?=
 =?us-ascii?Q?m1cdN5Y30AStClAfV4r4lLLz9hetxnCPeVQ7E451U6cz4duPRbJV/YfoEDSG?=
 =?us-ascii?Q?vKFKBALcSKDUmzmIXLdkKszQx4rlkjipDvWnXI5YJXyRcX8mmuOqkJwHwvLa?=
 =?us-ascii?Q?hkpZskDCSNgzLiOMiPcHJ5qKpqqG1w3TVTtcOjKfPBzQZm1Dm0+LsSjFuFKF?=
 =?us-ascii?Q?vJhfvpnxF9Md9O3R9y0JXF4f3J4iPdKG8Z14vSx1yVBK0G1d4/Xa6DN12ufp?=
 =?us-ascii?Q?eQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <BB38F26DBFE4B347986CF71A802E0989@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4548d363-5381-4544-fe20-08da5f6cb36a
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jul 2022 16:29:29.6961
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ozg/j7/FE/8EJsVWYB7uJcc4tXCzlAlHom3fUXjoFbwtpJpgJqc+9eWFwh3TzUko1IvzRRJC1YO04K6K5KfhAA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR10MB5478
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-06_09:2022-06-28,2022-07-06 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 spamscore=0 suspectscore=0 adultscore=0 mlxlogscore=737
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207060065
X-Proofpoint-GUID: Ri6sOhVV1Gmv6lbBwnwd8OUCVWfifTzq
X-Proofpoint-ORIG-GUID: Ri6sOhVV1Gmv6lbBwnwd8OUCVWfifTzq
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jul 6, 2022, at 12:18 AM, NeilBrown <neilb@suse.de> wrote:
>=20
> 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;
> +	}

Nit: Let's do it this way:

	dnew =3D lookup_one_len(name, ddir, len);
	if (IS_ERR(dnew)) {
		err =3D nfserrno(PTR_ERR(dnew);
		goto out_unlock;
	}

> 	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

--
Chuck Lever