Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Trond Myklebust <trondmy@hammerspace.com>,
        "jack@suse.cz" <jack@suse.cz>
CC:     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/mRhCvEKbVzqr4c88IqzYBl4AgAAJ0IA=
Date:   Wed, 30 Mar 2022 15:38:38 +0000
Message-ID: <FF014DFC-EC48-4CB7-A3F4-04FBB82E4A27@oracle.com>
References: <20220330103457.r4xrhy2d6nhtouzk@quack3.lan>
 <64a4832afd830d7c831ab687bc7a72cc791c2f0c.camel@hammerspace.com>
In-Reply-To: <64a4832afd830d7c831ab687bc7a72cc791c2f0c.camel@hammerspace.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Z2kLBnpsDfT3pgukwGAO55nP5R27OzoH5VV1uTxfa4IetHYW5D7qy2O/ZuR4?=
 =?us-ascii?Q?I5y59TK6O7h2BRXUwyu12RboH6tFb9S866vzgWubg10+NstJluG7LTmGpFeC?=
 =?us-ascii?Q?0jwDa2GUS0aAPxm067kktXFKIwb/amxkShrCKz8P6Z1O9Nfllp+MlVDk9+Jr?=
 =?us-ascii?Q?QkrDbB3YCsibBStq1xKROF5x5xSdoxY+QIuqLgLwQnfYciodIUEdsW/pSFUf?=
 =?us-ascii?Q?qrSufaneDDVv8yjs+HB0eOZ+ninEyjWOFe6ImyBL7V/dZ2UeRA+B5oQdedWx?=
 =?us-ascii?Q?xuuNXRZMAgrmYQ7HGfzVL1OG31hRLfXdYRs+oC9wGmOHNGY7qUTB1qAjh5Pp?=
 =?us-ascii?Q?HL5LV2gOpm+jV5E2p1e2/Y0ozRLDTpqBZv+x+858bnDyfJY0JqZlYWDymuNr?=
 =?us-ascii?Q?+Eb2rhHKwZr/EYF3pDgHz63mtuMTo/BYwGhiCLCSBR+VKTiqppSNf20dVFL2?=
 =?us-ascii?Q?xwA5Q0ID1f4SeSBPDvczDEWsnn/HqU0eYm8ql3Iw8BnUGEvgfhOdRdKzPNnd?=
 =?us-ascii?Q?KP1WMTU2GlGtdU6/v21xFjFuzyxCT5ghs3EmeUX3kkSQPnMsV30gL6T3zSzX?=
 =?us-ascii?Q?gT/l/k2gQz57igjCvTOmqb/WBdoP3JDKT8RUZR1Sr0bVo9ZRMYATgovfSSGD?=
 =?us-ascii?Q?kqH3lG5MqiXm8VASxffgVpQeGnPc1sL7aILWnAB4IlJXpNmCNa6gm7lHcxzy?=
 =?us-ascii?Q?GAxtwG40j4zWaf85a6A/Ldenq68VK5TgV4V0GwdfP5Y75BdDLfnD2pR1Ks76?=
 =?us-ascii?Q?dzSFn8yew39zTG8iRdiRmLAXJGj2et/BGLcG+0BVp45o17Yw4rg1yKBTAo70?=
 =?us-ascii?Q?ZY7VeSa0V34FiK1KwHa4BaTQ3QFmi2TjSM2CN6PeKJnp0O9mpdAYR6Tej4Ne?=
 =?us-ascii?Q?bOYOCfxxmsBBCCruY2usQbNee4pH0BrLrOIcWJLbQy6BzFfytf9aUF5jk3Pq?=
 =?us-ascii?Q?kc2T4DYEwr7nKjbYVB4HqTkkm3IavPJLXDKcJVo18iNgzh5PO/BWzD+nl+67?=
 =?us-ascii?Q?0xJqlozX7bhFJxgbxdyJslETE7vBFREZa299XDk3eT79hcnpvD2B4q0/ZLQM?=
 =?us-ascii?Q?+tSbT3wuTImaB+I4jRDhisIXq9lRUxd86u10Sed3QWLepM+7s/Q2rgkIIF0F?=
 =?us-ascii?Q?foTBoi+n/ujzkkbYZzfKrHMJKpBPTd4a7Xb6GzFeQxx/EF2Z8ifA4VnY7S6G?=
 =?us-ascii?Q?jqzK6/LZs80OeLz+wl7xhv1yN1ku7EKVohCu5mouNhdl7dpc4eoZhOYd5dpi?=
 =?us-ascii?Q?KifZ5NHBGPhveJ5RJqiKgrq7pshoTWD74QhVuBCIeL5pcPjPBQEmAaMTaYN2?=
 =?us-ascii?Q?5jm91u71VRuE5nZb90UBPRIpifi9UBhKpZZBX4v+LMBZyE3Zcjnqq5OJTpgO?=
 =?us-ascii?Q?8aVUx6SJZKSmJUncirWI5kV4c6jDBvKxTRcgjspwQYB5CjQXy/Yw9z8zp5S+?=
 =?us-ascii?Q?qx2l7barV3xNJMnMyrMkwINECU9WPW6Z9NoD4FTBKL5JQRcj7YrbTsHT/+Sg?=
 =?us-ascii?Q?lLSVt9SwTich67tYng9lN8TpMCBWhkSL0HUzVWW95iXZnPMdWr0FVbTslJFd?=
 =?us-ascii?Q?zuh1r3t7k4eWTG3xIs6N2EfvtN8j0T7XH1M1taJq87QRIRfe3Dvoq2BYOCuj?=
 =?us-ascii?Q?IGIixALNS1k/TAwzETOOMH/wyomR2IxeYGhjBDmMgDC8RgPXSeiI1VrzCn+D?=
 =?us-ascii?Q?icjntrpQvuzZHG7PGeVUp3KcVA+tjUEPLqysLaIFTXheKVOaYCkK+U7vslXq?=
 =?us-ascii?Q?FV+ZFrNjGxqB4agW1LryMrG509n/GtU=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9F1D3F699C253646988759B4AFB722B4@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: 29961c13-c18a-49b7-16e2-08da12635c0b
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Mar 2022 15:38:38.0702
 (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: fqfZUsLSzGQta7Hk4NjoC+rRKNA21k0nbx0YEvpRAOts54YKWcAC0VCjwfNyI8rq3MFMhP757gItuMj+21jBwA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR10MB2450
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.850
 definitions=2022-03-30_04: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-2203300075
X-Proofpoint-GUID: UGRvAzTvDNPyzUXhcuPLw3ADjUEZXbCG
X-Proofpoint-ORIG-GUID: UGRvAzTvDNPyzUXhcuPLw3ADjUEZXbCG
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 11:03 AM, Trond Myklebust <trondmy@hammerspace.com> w=
rote:
>=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=3Dposix=
 --
>> 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).

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.

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.

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.


--
Chuck Lever