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:     Al Viro <viro@zeniv.linux.org.uk>, Daire Byrne <daire@dneg.com>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 09/12] nfsd: support concurrent renames.
Thread-Topic: [PATCH 09/12] nfsd: support concurrent renames.
Thread-Index: AQHYf3xXp6NQ/4zQBEydVnDMQfaA961esooA
Date:   Fri, 24 Jun 2022 14:43:24 +0000
Message-ID: <7FDF6A0F-CD8A-4813-A5CA-FC2D69E84F29@oracle.com>
References: <165516173293.21248.14587048046993234326.stgit@noble.brown>
 <165516230201.21248.13160043266041158437.stgit@noble.brown>
In-Reply-To: <165516230201.21248.13160043266041158437.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?w8n5q7Sq3bow+aqdPvi9Xad1t71XBrse32BAWmn3jqpF3aHTN07S3HVKK0cB?=
 =?us-ascii?Q?hPG7J0QWbZoIv1TqzjKfEPEQiEaum/ZmFFTnCDJqq6qmQknqMTPKlYgrWvms?=
 =?us-ascii?Q?64U2Q5TK6bol2WHlZSyg7gDO6xYRAJ2BeDDxdQ5vkkZ0sostIHJHETp3cpTm?=
 =?us-ascii?Q?hAayJUud35yYy8dYvC2KFC6gfsmFYiMJZTQxR2ziblsaAkkdX4tyUFazYlpq?=
 =?us-ascii?Q?WOXG4PNCm+1UvUUD1xgOoNX3PlVtysbPFaF+XY5icJJLTnPtR/tHz2Qz4u/D?=
 =?us-ascii?Q?EfV0uRsWH8p4M+mrWQSHmdFz2zLSaM1SgYvtXGRthUrsJxlHbFqH+tbu2x7w?=
 =?us-ascii?Q?DxRoXArRnN3LfH1z3WhekN4ecVcJblS/+/ry3L3WaN7w2YlqLQN+ZoDGk77C?=
 =?us-ascii?Q?Fkc9NwL70ytQO9w3ufMWR5hGoOHugVUQStcq+GWwt34DkhZ4+9mflLbW45CF?=
 =?us-ascii?Q?2Bgyv/yRov/RaDKxXrJZPqnm0Ncff1Km3q7zjlM4zLGNrYGKhYPEvPfGgDqx?=
 =?us-ascii?Q?CuUROnLnO8oPqRJwWtLraNvgL2EMGxzvQh0tN0W+4+cuCbBH4Ok+bDVD2hus?=
 =?us-ascii?Q?rYwYFrRYXsvt/jQJCO/RuySO5HHhrYGkiSA9DuP+MH4avtwhRx5MvBHgBZ//?=
 =?us-ascii?Q?y7xiEmbGJnK4jB1OBraqRpAoX5FoFxeWOKILxEsdh96txstZrvXO+EykgleO?=
 =?us-ascii?Q?2m+ZTCT6ewF9O5dptYJErd9p10WfhMhA0ykOYfrXUD1awigBeibPLNahgxet?=
 =?us-ascii?Q?e6acXY7Do1IjC1T9tE0QlR5+Y1PsnViDHzD7pDF6/mr5X8X2uNEdXB4BkNSA?=
 =?us-ascii?Q?FgRDBIrrpSu229xEiuco0v5Srovp6YOnoesb7r6nvXMu/DFBQsGWf0TEnlhp?=
 =?us-ascii?Q?yZ2uc9aJARQ/YcBMRk1IJMpISDNqxJmA8yCII0ArhYm1f6LAfujpFRcxFeEG?=
 =?us-ascii?Q?tSf3RcJKEfRpwB+N2iv2v5K+FuNT+GZx26RJG0SJXpaIPcO0QIvo/VB6EaMv?=
 =?us-ascii?Q?XWAuZZxeYc8iTnQMKpq+dCoLHZXbFywwEtq8jYYuGH+AcsbSzSzTK3JM7RYx?=
 =?us-ascii?Q?qCw3xe5nXUcgQlcIyEjwnt6+PHSFUObR/L2n2L5/oc/xcoIe3oq4rzalCrbZ?=
 =?us-ascii?Q?fYyifQPr0MJHRoCPwcjR53w1QIrRtyWRbwOvwQ4PWJ4YktNUE2lESmdVhEf9?=
 =?us-ascii?Q?okI6lDefk15U3D65ZKUIMb5ENuGYHbBysvfi0NhDBw2gBGx5fPXXaJ5SPrPM?=
 =?us-ascii?Q?kyBQ0L4MzD63m4m0SYgrMRwBSmYC9M1r1tFnSNhz5v+1qLMCCYdSrnOO4/5o?=
 =?us-ascii?Q?o581vVYDr6nrZcVL7dZsm4iJi33dr7v8lK7GvQnNHqauFqlpiZ1C4TI/ezpg?=
 =?us-ascii?Q?7uBS4qPA/Jx6A3JsjAC7Py08JVdJvwAmDFewyCpVcF6oYadetJaPdc1kCNAn?=
 =?us-ascii?Q?dX7R6XbTkZOhVSOTKws79hL6bM2lBzCkB5KvNaey+QR810xaIADkHWN/pFOT?=
 =?us-ascii?Q?I6iSyuOk4+MRr8IVBV0wFpfx9KeKbEgtV6+Yg2WspPELQul/1L61U/hm5zZP?=
 =?us-ascii?Q?z+STU/nEoWkYsFQWrQ9AzoDLyLAZEImDiE7KNzLBNUlhwl29h9GcWzSdqTKJ?=
 =?us-ascii?Q?Qg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <D8DAABF01A5B6649B28E455E4F5BAAD1@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: 676b190a-8607-41e4-b1cc-08da55efe4c3
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jun 2022 14:43:24.9452
 (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: RN8dUr4DrgXB+3UcDuDQX6UqniWYZsHN7ezOgDjTwshjfbOJHq8bUvcUch1cAjoBiMxXxf6Qfbj/Bzfsv6AwAw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4236
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-06-24_07:2022-06-23,2022-06-24 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxlogscore=999
 malwarescore=0 adultscore=0 phishscore=0 bulkscore=0 mlxscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2204290000 definitions=main-2206240057
X-Proofpoint-GUID: a2f12PDYdL3nNOb7QEwcleMX3UgvIhgK
X-Proofpoint-ORIG-GUID: a2f12PDYdL3nNOb7QEwcleMX3UgvIhgK
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 Jun 13, 2022, at 7:18 PM, NeilBrown <neilb@suse.de> wrote:
>=20
> If the filesystem supports it, renames can now be concurrent with other
> updates.
> We use lock_rename_lookup_one() to do the appropriate locking in the
> right order and to look up the names.
>=20
> Signed-off-by: NeilBrown <neilb@suse.de>

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


> ---
> fs/nfsd/vfs.c |   49 +++++++++++++++++++------------------------------
> 1 file changed, 19 insertions(+), 30 deletions(-)
>=20
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6cdd5e407600..b0df216ab3e4 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1584,6 +1584,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *=
ffhp, char *fname, int flen,
> 	__be32		err;
> 	int		host_err;
> 	bool		close_cached =3D false;
> +	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);

Ditto.


>=20
> 	err =3D fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_REMOVE);
> 	if (err)
> @@ -1611,41 +1612,37 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh=
 *ffhp, char *fname, int flen,
>=20
> 	/* cannot use fh_lock as we need deadlock protective ordering
> 	 * so do it by hand */
> -	trap =3D lock_rename(tdentry, fdentry);
> -	ffhp->fh_locked =3D tfhp->fh_locked =3D true;
> -	fh_fill_pre_attrs(ffhp, true);
> -	fh_fill_pre_attrs(tfhp, true);
> -
> -	odentry =3D lookup_one_len(fname, fdentry, flen);
> -	host_err =3D PTR_ERR(odentry);
> -	if (IS_ERR(odentry))
> +	trap =3D lock_rename_lookup_one(tdentry, fdentry, &ndentry, &odentry,
> +				      tname, tlen, fname, flen, 0, 0, &wq);
> +	host_err =3D PTR_ERR(trap);
> +	if (IS_ERR(trap))
> 		goto out_nfserr;
> +	ffhp->fh_locked =3D tfhp->fh_locked =3D true;
> +	fh_fill_pre_attrs(ffhp, (ndentry->d_flags & DCACHE_PAR_UPDATE) =3D=3D 0=
);
> +	fh_fill_pre_attrs(tfhp, (ndentry->d_flags & DCACHE_PAR_UPDATE) =3D=3D 0=
);
>=20
> 	host_err =3D -ENOENT;
> 	if (d_really_is_negative(odentry))
> -		goto out_dput_old;
> +		goto out_unlock;
> 	host_err =3D -EINVAL;
> 	if (odentry =3D=3D trap)
> -		goto out_dput_old;
> +		goto out_unlock;
>=20
> -	ndentry =3D lookup_one_len(tname, tdentry, tlen);
> -	host_err =3D PTR_ERR(ndentry);
> -	if (IS_ERR(ndentry))
> -		goto out_dput_old;
> 	host_err =3D -ENOTEMPTY;
> 	if (ndentry =3D=3D trap)
> -		goto out_dput_new;
> +		goto out_unlock;
>=20
> 	host_err =3D -EXDEV;
> 	if (ffhp->fh_export->ex_path.mnt !=3D tfhp->fh_export->ex_path.mnt)
> -		goto out_dput_new;
> +		goto out_unlock;
> 	if (ffhp->fh_export->ex_path.dentry !=3D tfhp->fh_export->ex_path.dentry=
)
> -		goto out_dput_new;
> +		goto out_unlock;
>=20
> 	if ((ndentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) =
&&
> 	    nfsd_has_cached_files(ndentry)) {
> 		close_cached =3D true;
> -		goto out_dput_old;
> +		dget(ndentry);
> +		goto out_unlock;
> 	} else {
> 		struct renamedata rd =3D {
> 			.old_mnt_userns	=3D &init_user_ns,
> @@ -1662,23 +1659,15 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh=
 *ffhp, char *fname, int flen,
> 				host_err =3D commit_metadata(ffhp);
> 		}
> 	}
> - out_dput_new:
> -	dput(ndentry);
> - out_dput_old:
> -	dput(odentry);
> - out_nfserr:
> -	err =3D nfserrno(host_err);
> -	/*
> -	 * We cannot rely on fh_unlock on the two filehandles,
> -	 * as that would do the wrong thing if the two directories
> -	 * were the same, so again we do it by hand.
> -	 */
> 	if (!close_cached) {
> 		fh_fill_post_attrs(ffhp);
> 		fh_fill_post_attrs(tfhp);
> 	}
> -	unlock_rename(tdentry, fdentry);
> + out_unlock:
> +	unlock_rename_lookup(tdentry, fdentry, ndentry, odentry);
> 	ffhp->fh_locked =3D tfhp->fh_locked =3D false;
> + out_nfserr:
> +	err =3D nfserrno(host_err);
> 	fh_drop_write(ffhp);
>=20
> 	/*
>=20
>=20

--
Chuck Lever