Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jan Kara <jack@suse.cz>
CC:     Trond Myklebust <trondmy@hammerspace.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        "nfbrown@suse.com" <nfbrown@suse.com>,
        Bruce Fields <bfields@redhat.com>
Subject: Re: Performance regression with random IO pattern from the client
Thread-Topic: Performance regression with random IO pattern from the client
Thread-Index: AQHYRCHUc6q/mRhCvEKbVzqr4c88IqzYBl4AgAAJ0ICAAAuGAIAAGwaA
Date:   Wed, 30 Mar 2022 17:56:36 +0000
Message-ID: <FDDB9D43-A695-46A6-9C82-2205C9779957@oracle.com>
References: <20220330103457.r4xrhy2d6nhtouzk@quack3.lan>
 <64a4832afd830d7c831ab687bc7a72cc791c2f0c.camel@hammerspace.com>
 <FF014DFC-EC48-4CB7-A3F4-04FBB82E4A27@oracle.com>
 <20220330161952.haopqr342qlij5hg@quack3.lan>
In-Reply-To: <20220330161952.haopqr342qlij5hg@quack3.lan>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Gav4mxkNFEjZeetjBlJkxVO9PnXzclVn8D8HsyVrdU/tjalfBieiXBC8+lm7?=
 =?us-ascii?Q?y575H45oUWAYY5e6hYZC9Walnl+bTKSxBOzxG1Y1KzhLS5a0bKWN8Dm85/gi?=
 =?us-ascii?Q?Tyi3l/3l546NEUEc2Hz/VB9r54OenosNAteLlfEJYDuVeEsitHpZZgx6Lq1u?=
 =?us-ascii?Q?GsIGLXacePUU97+yHQsu7CwbOzQc/11ymLGMWGb9OxIOR2wP47LCVeYtQdfS?=
 =?us-ascii?Q?EaI9RWQRzY+dLbgfQNQ5if8wCJSv2AG3yVu8P6IxWh7cPDSBnR5Ww+Jaqq6r?=
 =?us-ascii?Q?yjzD4FgCxzwNSDDHMpSjFQmbg8gXs4ytqXUeRgoHyqEEHPe+YTNCXQF1vZcA?=
 =?us-ascii?Q?WXtXgzmXnK79STYj4Y44Bu5sYBTm3rCNraFiW4ldLSqPNnIjgOPGmhq+j8vQ?=
 =?us-ascii?Q?oIRWwSQ/Q6E2JRFPYARSkOFsHMwl9Xdn9oNvSxwnHaW4Wf4m1AbX14m8b7S1?=
 =?us-ascii?Q?Aj/my6jA0LKKY8a04OPG1s5CLYiuHhPJCypqRAiejJJ+SiPP2hHm57q2hdG9?=
 =?us-ascii?Q?f14lTIjzq2LknZ+RENcF4EG5CTUWKX+kl5eksSfChINybTDCnnzGypAmT9q2?=
 =?us-ascii?Q?2Z0VSU9S2ag7qDMHed7mPuzYsYdBQblpdTpjwE2BTe2z2DUUffeGC/3fTDs5?=
 =?us-ascii?Q?n5fqtZUchhr/IUBq06sAP5qBV3UD6+oq/LN5dGM2Ac//LoZifVcy75RcHx4m?=
 =?us-ascii?Q?3YHOGuYbj+tEGl3FM+PMPV3CBUvx151nSbCkf++QC+KUvctw4SKJVoei6agX?=
 =?us-ascii?Q?k7loVmmO70y0Y8/RuLg1e1KGXpVnYlEqceeWqkpfUl2fV1sz8eLrszuIdJpp?=
 =?us-ascii?Q?s/YHfYT3SzfjFt3KcPJQs9XrmAmFLh7OBRFSskB+3o7tvNiyxXTsO0GzKHeu?=
 =?us-ascii?Q?xklUE098nuXpBGTKDOgJyo9NTKe9q0pse+NkIsRIDa1KSA6R90ziJMpjOkCs?=
 =?us-ascii?Q?y6E7Yce8NkBfwu/zDZ77Rt/6sohDnFlXB9M7dFlGn8iSsR5ThtgJcwI4CMDq?=
 =?us-ascii?Q?+RuVbvITquJPIpxa+usDs6PpViU/wC+SlbskciBNHAL9En2l4/oVX3BjJH6D?=
 =?us-ascii?Q?5S7w8LXPnxd3NX+8RH8MrcPLW65vn7+zvw11jXszvedgoKP0EjddS0Adadlk?=
 =?us-ascii?Q?MwyD9fOcUujEq3iPU+BKu59mxHBepBCAcsGxdQ+K725CXpKi8RSDgeZhvQpf?=
 =?us-ascii?Q?LQLBJxggLFAOSMGj7r9BmVtm7duSqXkUFdFs2FEmZ/SbC249BjAm778h/wRb?=
 =?us-ascii?Q?a8p5jpPI2LAQMVI5Z5AEmqYZ14UhIDRIxmRNlNfV0FzvCeCJVeS+AlslsMvI?=
 =?us-ascii?Q?fi2wHeUD19YLyZydC9kQuzB+lYfsNxHnHyHdzKjThswNP1vojr1dPuP/zmm9?=
 =?us-ascii?Q?zTEGOeC+wM72gZh8m9hY5EcqCvH3WBvadbRFzLJsrc7u710Rj7bINkuNMLKW?=
 =?us-ascii?Q?kOWYdmph4bwapF34NROknI27zLtETgg5GxLyL5V6dhxhGJ8RM5zN40TZTRqt?=
 =?us-ascii?Q?84x2/kW6Zh1xpmlpIYPJj6vx4mUjMyGJ7fj1DDWBBYQcOvjurVsRfeuULDid?=
 =?us-ascii?Q?ZBM5OZ85beCOj5yrK1PZnU9BLjY1LsOw2T0mBkQzuirj5kXWPt1c+DWk5pkQ?=
 =?us-ascii?Q?jyCmd6Cs5c0jSrIJ4ERZf1IzHMU9aU27u2ByVFFyBgQYcuOsaqQ2dOunx1wY?=
 =?us-ascii?Q?tW3B3NbV+xQaj8rR7zB6YvvEsg5b11c+yaH9VLzBDMca5iVXsI5vEkxovDaf?=
 =?us-ascii?Q?jBkeAOj/HhYGOQjDbcIt85uhwNvPVLs=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <5BA192D044632C4D86827694E5334F2D@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: 0768c184-cd88-4443-f4c6-08da1276a227
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Mar 2022 17:56:36.1377
 (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: LwObGTox/I33A/PFC97wXyoQEBOHdUi/XVdXtpNEhvtA6+45QNRMXINR2f007gTlPYqqawt6rqeE/uBK5nYpoA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB3578
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-03-30_06:2022-03-29,2022-03-30 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 phishscore=0 suspectscore=0 malwarescore=0 adultscore=0 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2203300087
X-Proofpoint-ORIG-GUID: AjZ0UPWeqfVkS5RFt7TDrvUIsasggUSR
X-Proofpoint-GUID: AjZ0UPWeqfVkS5RFt7TDrvUIsasggUSR
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no 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 Mar 30, 2022, at 12:19 PM, Jan Kara <jack@suse.cz> wrote:
>=20
> On Wed 30-03-22 15:38:38, Chuck Lever III wrote:
>>> On Mar 30, 2022, at 11:03 AM, Trond Myklebust <trondmy@hammerspace.com>=
 wrote:
>>>=20
>>> On Wed, 2022-03-30 at 12:34 +0200, Jan Kara wrote:
>>>> Hello,
>>>>=20
>>>> during our performance testing we have noticed that commit
>>>> b6669305d35a
>>>> ("nfsd: Reduce the number of calls to nfsd_file_gc()") has introduced
>>>> a
>>>> performance regression when a client does random buffered writes. The
>>>> workload on NFS client is fio running 4 processed doing random
>>>> buffered writes to 4
>>>> different files and the files are large enough to hit dirty limits
>>>> and
>>>> force writeback from the client. In particular the invocation is
>>>> like:
>>>>=20
>>>> fio --direct=3D0 --ioengine=3Dsync --thread --directory=3D/mnt/mnt1 --
>>>> invalidate=3D1 --group_reporting=3D1 --runtime=3D300 --fallocate=3Dpos=
ix --
>>>> ramp_time=3D10 --name=3DRandomReads-128000-4k-4 --new_group --
>>>> rw=3Drandwrite --size=3D4000m --numjobs=3D4 --bs=3D4k --
>>>> filename_format=3DFioWorkloads.\$jobnum --end_fsync=3D1
>>>>=20
>>>> The reason why commit b6669305d35a regresses performance is the
>>>> filemap_flush() call it adds into nfsd_file_put(). Before this commit
>>>> writeback on the server happened from nfsd_commit() code resulting in
>>>> rather long semisequential streams of 4k writes. After commit
>>>> b6669305d35a
>>>> all the writeback happens from filemap_flush() calls resulting in
>>>> much
>>>> longer average seek distance (IO from different files is more
>>>> interleaved)
>>>> and about 16-20% regression in the achieved writeback throughput when
>>>> the
>>>> backing store is rotational storage.
>>>>=20
>>>> I think the filemap_flush() from nfsd_file_put() is indeed rather
>>>> aggressive and I think we'd be better off to just leave writeback to
>>>> either
>>>> nfsd_commit() or standard dirty page cleaning happening on the
>>>> system. I
>>>> assume the rationale for the filemap_flush() call was to make it more
>>>> likely the file can be evicted during the garbage collection run? Was
>>>> there
>>>> any particular problem leading to addition of this call or was it
>>>> just "it
>>>> seemed like a good idea" thing?
>>>>=20
>>>> Thanks in advance for ideas.
>>>>=20
>>>>                                                                Honza
>>>=20
>>> It was mainly introduced to reduce the amount of work that
>>> nfsd_file_free() needs to do. In particular when re-exporting NFS, the
>>> call to filp_close() can be expensive because it synchronously flushes
>>> out dirty pages. That again means that some of the calls to
>>> nfsd_file_dispose_list() can end up being very expensive (particularly
>>> the ones run by the garbage collector itself).
>>=20
>> The "no regressions" rule suggests that some kind of action needs
>> to be taken. I don't have a sense of whether Jan's workload or NFS
>> re-export is the more common use case, however.
>>=20
>> I can see that filp_close() on a file on an NFS mount could be
>> costly if that file has dirty pages, due to the NFS client's
>> "flush on close" semantic.
>>=20
>> Trond, are there alternatives to flushing in the nfsd_file_put()
>> path? I'm willing to entertain bug fix patches rather than a
>> mechanical revert of b6669305d35a.
>=20
> Yeah, I don't think we need to rush fixing this with a revert.

Sorry I wasn't clear: I would prefer to apply a bug fix over
sending a revert commit, and I do not have enough information
yet to make that choice. Waiting a bit is OK with me.


> Also because
> just removing the filemap_flush() from nfsd_file_put() would keep other
> benefits of that commit while fixing the regression AFAIU. But I think
> making flushing less aggressive is desirable because as I wrote in my oth=
er
> reply, currently it is preventing pagecache from accumulating enough dirt=
y
> data for a good IO pattern.

I might even go so far as to say that a small write workload
isn't especially good for solid state storage either.

I know Trond is trying to address NFS re-export performance, but
there appear to be some palpable effects outside of that narrow
use case that need to be considered. Thus a server-side fix,
rather than a fix in the NFS client used to do the re-export,
seems appropriate to consider.


--
Chuck Lever