On Wed, 2022-03-30 at 12:34 +0200, Jan Kara wrote: > Hello, > > 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: > > fio --direct=0 --ioengine=sync --thread --directory=/mnt/mnt1 -- > invalidate=1 --group_reporting=1 --runtime=300 --fallocate=posix -- > ramp_time=10 --name=RandomReads-128000-4k-4 --new_group -- > rw=randwrite --size=4000m --numjobs=4 --bs=4k -- > filename_format=FioWorkloads.\$jobnum --end_fsync=1 > > 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. > > 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? > > Thanks in advance for ideas. > > Honza 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). -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx