Dave, Thanks for this! I have been testing it with our production (render farm) loads for the last couple of days and have not run into any issues so far. It seems to be performing on par with your previous version of the patchset (based on v6.0). I am also running with both the "known issues" dhowells patches [8] & [9] mentioned in your email (as I was with your previous version). Tested-by: Daire Byrne <daire@xxxxxxxx> On Mon, 20 Feb 2023 at 13:44, Dave Wysochanski <dwysocha@xxxxxxxxxx> wrote: > > Trond, this v11 patchset addresses your latest feedback on patch #2, > and I did a little bit of cleanup to patch 3 (see Changes notes below). > I'm not sure of further changes to patch #3 without a more in-depth > review with specifics, if you feel the current approach is unacceptable [1]. > > Ben and Daire, if you could test this set and provide you feedback > and a Tested-By: that would be appreciated. This set addresses > the existing NFS + fscache performance concerns seen by a few > users [5], which is due to utilization use of the deprecated > single-page function, fscache_fallback_read_page(). However, > until "known issue #1" below is also resolved, even with these > patches, performance of NFS+fscache will still be a problem > in some scenarios. > > This patchset converts NFS with fscache buffered read IO paths to > use the netfs API with a non-invasive approach. The existing NFS pgio > layer does not need extensive changes, and is the best way so far I've > found to address Trond's previous concerns about modifying the IO > path [2] as well as only enabling netfs when fscache is configured > and enabled [3]. I have not attempted performance comparisions to > address Chuck Lever's concern [4] because we are not converting the > non-fscache enabled NFS IO paths to netfs. > > The patchset is based on Trond's latest 'testing' branch which includes > his folio patchset, and is based on 6.2-rc5. It has been pushed to > github at: > https://github.com/DaveWysochanskiRH/kernel/commits/nfs-fscache-netfs > https://github.com/DaveWysochanskiRH/kernel/commit/6424e4f139652b7552eff26eb5da1f2282d35616 > > Changes since v10 [6] > ===================== > PATCH6: Dropped > PATCH1: Rename nfs_pageio_add_page to nfs_read_add_folio > PATCH2: Use anonymous union to add struct netfs_inode to nfs_inode (Trond) [7] > PATCH3: Change nfs_netfs_readpage_release() to nfs_netfs_folio_unlock() > > Testing > ======= > I did a full round of testing on this because it was rebased on top of > Trond's testing branch which included his folio series. > All of my unit tests pass except the one with the known issue #1 below. > Multiple runs of xfstests generic tests (applicable to NFS) were run with > with various servers, both with and without fscache enabled, and > compared to baseline (Trond's testing branch). No new failures were > observed with these patches, and in some xfstest instances, this > patchset improves the results (some tests that were failing now pass). > - hammerspace(pNFS flexfiles): NFS4.1, NFS4.2 > - NetApp(pNFS filelayout): NFS4.1, NFS4.0, NFS3 > - RHEL9: NFS4.2, NFS4.1, NFS4.0, NFS3 > > Known issues > ============ > 1. Unit test setting rsize < readahead does not properly read from > fscache but re-reads data from the NFS server > * This will be fixed with another dhowells patch [8]: > "[PATCH v6 2/2] mm, netfs, fscache: Stop read optimisation when folio removed from pagecache" > * Daire Byrne verified the patch fixes his issue as well > > 2. "Cache volume key already in use" after xfstest runs involving multiple mounts > * Simple reproducer requires just two mounts as follows: > mount -overs=4.1,fsc,nosharecache -o context=system_u:object_r:root_t:s0 nfs-server:/exp1 /mnt1 > mount -overs=4.1,fsc,nosharecache -o context=system_u:object_r:root_t:s0 nfs-server:/exp2 /mnt2 > * This should be fixed with dhowells patch [9]: > "[PATCH v5] vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing" > > > References > ========== > [1] https://lore.kernel.org/linux-nfs/0676ecb2bb708e6fc29dbbe6b44551d6a0d021dc.camel@xxxxxxxxxx/ > [2] https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@xxxxxxxxxxxxxxx/ > [3] https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@xxxxxxxxxxxxxxx/ > [4] https://lore.kernel.org/linux-nfs/0A640C47-5F51-47E8-864D-E0E980F8B310@xxxxxxxxxx/ > [5] https://lore.kernel.org/linux-nfs/CA+QRt4tPqH87NVkoETLjxieGjZ_f7XxRj+xS3NVxcJ+b8AAKQg@xxxxxxxxxxxxxx/ > [6] https://lore.kernel.org/linux-nfs/20221103161637.1725471-1-dwysocha@xxxxxxxxxx/ > [7] https://lore.kernel.org/linux-nfs/4d60636f62df4f5c200666ed2d1a5f2414c18e1f.camel@xxxxxxxxxx/ > [8] https://lore.kernel.org/linux-nfs/20230216150701.3654894-1-dhowells@xxxxxxxxxx/T/#mf3807fa68fb6d495b87dde0d76b5237833a0cc81 > [9] https://lore.kernel.org/linux-kernel/217595.1662033775@xxxxxxxxxxxxxxxxxxxxxx/ > > Dave Wysochanski (5): > NFS: Rename readpage_async_filler to nfs_read_add_folio > NFS: Configure support for netfs when NFS fscache is configured > NFS: Convert buffered read paths to use netfs when fscache is enabled > NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API > NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit > > fs/nfs/Kconfig | 1 + > fs/nfs/fscache.c | 242 ++++++++++++++++++++++--------------- > fs/nfs/fscache.h | 131 ++++++++++++++------ > fs/nfs/inode.c | 2 + > fs/nfs/internal.h | 9 ++ > fs/nfs/iostat.h | 17 --- > fs/nfs/nfstrace.h | 91 -------------- > fs/nfs/pagelist.c | 4 + > fs/nfs/read.c | 105 ++++++++-------- > fs/nfs/super.c | 11 -- > include/linux/nfs_fs.h | 25 ++-- > include/linux/nfs_iostat.h | 12 -- > include/linux/nfs_page.h | 3 + > include/linux/nfs_xdr.h | 3 + > 14 files changed, 317 insertions(+), 339 deletions(-) > > -- > 2.31.1 >
