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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     "trondmy@kernel.org" <trondmy@kernel.org>
CC:     Bruce Fields <bfields@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: Reduce contention for the nfsd_file nf_rwsem
Thread-Topic: [PATCH] nfsd: Reduce contention for the nfsd_file nf_rwsem
Thread-Index: AQHXY9BFZBFmWOpSekG1W92QMIYbjKsesyqA
Date:   Mon, 21 Jun 2021 16:49:28 +0000
Message-ID: <1669C849-D7DC-46C1-B6B6-F2C79C819710@oracle.com>
References: <20210617232652.264884-1-trondmy@kernel.org>
In-Reply-To: <20210617232652.264884-1-trondmy@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?EiGrdI1eAZ6wsowz2SfLUatXWiU4/3sBbbs2b0kGL+qEV830vqP1SD4cSfFW?=
 =?us-ascii?Q?9ve51ibrcYiwjSeT8uwgYW10C0KdB0r3J2HGTjOKGmkKT5VLgXj/Thq1vamP?=
 =?us-ascii?Q?m9L6HDEBSarieApC+5ILmi+sLgIxB58cpFoHUX2jAanMJf2rs7AvM97OiK8s?=
 =?us-ascii?Q?zEogtSHqLw66TlS1mbtgF0ZE2JrzFFwcbik8xNzk2PPOqLWe+1jRftspcoZj?=
 =?us-ascii?Q?GVU22UNj11sJAZc9Sea0Qf/BpZfVmk7OIyagHgYt36qGDr51iWjgxoMSM7DV?=
 =?us-ascii?Q?xOddcSPQiD7Gm1SwEovXUL/jdxE2cchJxo7GsQngyCKO6c0i5YRs0fe7YVPm?=
 =?us-ascii?Q?4Bo+KAOuSVjRlUjShRQFUpGLYqp3pL687SsYFiC+SRz3RM4PsK/NrOMAv7xU?=
 =?us-ascii?Q?dxaZjcuOHEDDU0Htm4ozdD9nrnJvQiN5A0rYgaPDL3y2fdX2eluIwSzK8I3N?=
 =?us-ascii?Q?UDtIfbTpYRn1IHBCZFdzgah7jmjuEICImEcEwYb3+TCv3vqhL7d/s4goUDYs?=
 =?us-ascii?Q?wSdBMl/OJnVY0AXORCZzx6WGhBtxf4iiigfPCuyNIoktKhTTnEkfPDGPBVEI?=
 =?us-ascii?Q?qgwkDyxzLKslvRHghOld1eyVknFwx8bA+Cf3dIyWzVoRqnHJeDRtziLVJw0o?=
 =?us-ascii?Q?7YMHYFQm4g0I6Al5U9z+6OPlYUW4DWvPScbGeKgKqPY0johMCE+IkGDzxGGu?=
 =?us-ascii?Q?oD1cZ3NKOu6p5UBqS96L2PIM7+tArAmWTxA8U8n1mZL3RKQGSDYkqbtL8O2W?=
 =?us-ascii?Q?WD6jHvkRzj+H8wM/Ff/HHguy+m4GsKR/+AXeiKir5DK8nGb67V8nx3cnQRcf?=
 =?us-ascii?Q?33NUspDHF3QeekLb0mQeXpssOg+bFYS5q7BEdcJc3isHmg+Ja0FGngi1z/cg?=
 =?us-ascii?Q?ej2NTdJ/HzmfW+W7M5dQI5Am3Md8+kS+Jb55kTVWsG2Svkp3G06nLD5RcPUe?=
 =?us-ascii?Q?KAyJ7OJWC+/IbaNfyXXbLLiakV3rBjHMyQsCYOb1bjJC+L761dFE49/USJ8g?=
 =?us-ascii?Q?AAY7SmeS3E4uzUT5ZIVYek0OO6GBNoAdeRBTfosTgjUAMkFSVR+IVtoKy2H3?=
 =?us-ascii?Q?cY7egsr8J/BUDfG79VyDMQsDoHPlGA7un3OThB3019H8AqxTDEErWBdvlWVH?=
 =?us-ascii?Q?FuwlG+2ipaghzid7fTjL+j+GkLq2A3tDcKRTShLaCZmMp7+wBMV5OTQ1VnOJ?=
 =?us-ascii?Q?9CxAjI8nWwExI2poTK278/xhNl/zuqkc8xjh6GhMfj8ma85gF8iFufki3Qod?=
 =?us-ascii?Q?QnNC/aSiye05VZTsNV4nnq82j/KGv33E+BAFa3ne1AsmhLEsn5P9RpVZMPG1?=
 =?us-ascii?Q?ddyevNpaeSduvx0OLQdvMh1V?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <CD4243EC856C7A4FB69F0935BD8221C4@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: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dbe33596-59d8-4ecb-e16d-08d934d488d2
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Jun 2021 16:49:28.2121
 (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: 3gWySM9QxdApi7z0kzxP0U95XjH5KpONFaIv59HtOMWQdquJBO8kGe9RiHTZPhCYVbbQr6XoiUGMdfdbbnYnnw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4259
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10022 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0 mlxscore=0
 spamscore=0 mlxlogscore=999 bulkscore=0 suspectscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2106210099
X-Proofpoint-GUID: FFcGULJ8RE6kzmwzMl8aGrRTpKsJN27C
X-Proofpoint-ORIG-GUID: FFcGULJ8RE6kzmwzMl8aGrRTpKsJN27C
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

Hi-

> On Jun 17, 2021, at 7:26 PM, trondmy@kernel.org wrote:
>=20
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>=20
> When flushing out the unstable file writes as part of a COMMIT call, try
> to perform most of of the data writes and waits outside the semaphore.
>=20
> This means that if the client is sending the COMMIT as part of a memory
> reclaim operation, then it can continue performing I/O, with contention
> for the lock occurring only once the data sync is finished.
>=20
> Fixes: 5011af4c698a ("nfsd: Fix stable writes")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>

The good news is I've found no functional regressions. The bad
news is I haven't seen any difference in performance. Is there
a particular test that I can run to observe improvement?

I wonder about adding a Fixes: tag for a change that the patch
description describes as an optimization.


> ---
> fs/nfsd/vfs.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>=20
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 15adf1f6ab21..46485c04740d 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1123,6 +1123,19 @@ nfsd_write(struct svc_rqst *rqstp, struct svc_fh *=
fhp, loff_t offset,
> }
>=20
> #ifdef CONFIG_NFSD_V3
> +static int
> +nfsd_filemap_write_and_wait_range(struct nfsd_file *nf, loff_t offset,
> +				  loff_t end)
> +{
> +	struct address_space *mapping =3D nf->nf_file->f_mapping;
> +	int ret =3D filemap_fdatawrite_range(mapping, offset, end);
> +
> +	if (ret)
> +		return ret;
> +	filemap_fdatawait_range_keep_errors(mapping, offset, end);
> +	return 0;
> +}
> +
> /*
>  * Commit all pending writes to stable storage.
>  *
> @@ -1153,10 +1166,11 @@ nfsd_commit(struct svc_rqst *rqstp, struct svc_fh=
 *fhp,
> 	if (err)
> 		goto out;
> 	if (EX_ISSYNC(fhp->fh_export)) {
> -		int err2;
> +		int err2 =3D nfsd_filemap_write_and_wait_range(nf, offset, end);
>=20
> 		down_write(&nf->nf_rwsem);
> -		err2 =3D vfs_fsync_range(nf->nf_file, offset, end, 0);
> +		if (!err2)
> +			err2 =3D vfs_fsync_range(nf->nf_file, offset, end, 0);
> 		switch (err2) {
> 		case 0:
> 			nfsd_copy_boot_verifier(verf, net_generic(nf->nf_net,
> --=20
> 2.31.1
>=20

--
Chuck Lever