From: David Wysochanski <dwysocha@redhat.com>
To: Daire Byrne <daire@dneg.com>
Cc: Anna Schumaker <anna.schumaker@netapp.com>,
Trond Myklebust <trond.myklebust@hammerspace.com>,
David Howells <dhowells@redhat.com>,
Benjamin Maynard <benmaynard@google.com>,
Daire Byrne <daire.byrne@gmail.com>,
linux-nfs@vger.kernel.org, linux-cachefs@redhat.com
Subject: Re: [PATCH v11 0/5] Convert NFS with fscache to the netfs API
Date: Wed, 22 Feb 2023 10:57:11 -0500 [thread overview]
Message-ID: <CALF+zOmcROD6tzSCixYQN-+hw8bpboTrF-Ff-hEZOMDAPe7BOA@mail.gmail.com> (raw)
In-Reply-To: <CAPt2mGPJxPWfFGtEacBw-AN5nMZfP_pvL6=wEM+QbrPf1brAFg@mail.gmail.com>
Thanks Daire! Did this also resolve the issue with
/proc/PID/io/read_bytes you reported (link below)
since netfs should increment that count now?
https://lore.kernel.org/linux-nfs/CAPt2mGNEYUk5u8V4abe=5MM5msZqmvzCVrtCP4Qw1n=gCHCnww@mail.gmail.com/
On Wed, Feb 22, 2023 at 7:51 AM Daire Byrne <daire@dneg.com> wrote:
>
> 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@dneg.com>
>
>
>
> On Mon, 20 Feb 2023 at 13:44, Dave Wysochanski <dwysocha@redhat.com> 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@kernel.org/
> > [2] https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@hammerspace.com/
> > [3] https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@hammerspace.com/
> > [4] https://lore.kernel.org/linux-nfs/0A640C47-5F51-47E8-864D-E0E980F8B310@oracle.com/
> > [5] https://lore.kernel.org/linux-nfs/CA+QRt4tPqH87NVkoETLjxieGjZ_f7XxRj+xS3NVxcJ+b8AAKQg@mail.gmail.com/
> > [6] https://lore.kernel.org/linux-nfs/20221103161637.1725471-1-dwysocha@redhat.com/
> > [7] https://lore.kernel.org/linux-nfs/4d60636f62df4f5c200666ed2d1a5f2414c18e1f.camel@kernel.org/
> > [8] https://lore.kernel.org/linux-nfs/20230216150701.3654894-1-dhowells@redhat.com/T/#mf3807fa68fb6d495b87dde0d76b5237833a0cc81
> > [9] https://lore.kernel.org/linux-kernel/217595.1662033775@warthog.procyon.org.uk/
> >
> > 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
> >
>
next prev parent reply other threads:[~2023-02-22 15:58 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-20 13:43 [PATCH v11 0/5] Convert NFS with fscache to the netfs API Dave Wysochanski
2023-02-20 13:43 ` [PATCH v11 1/5] NFS: Rename readpage_async_filler to nfs_read_add_folio Dave Wysochanski
2023-02-20 13:43 ` [PATCH v11 2/5] NFS: Configure support for netfs when NFS fscache is configured Dave Wysochanski
2023-02-20 13:43 ` [PATCH v11 3/5] NFS: Convert buffered read paths to use netfs when fscache is enabled Dave Wysochanski
2023-02-20 13:43 ` [PATCH v11 4/5] NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API Dave Wysochanski
2023-02-20 13:43 ` [PATCH v11 5/5] NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit Dave Wysochanski
2023-02-22 12:50 ` [PATCH v11 0/5] Convert NFS with fscache to the netfs API Daire Byrne
2023-02-22 15:57 ` David Wysochanski [this message]
2023-02-23 15:47 ` Daire Byrne
2023-03-09 23:50 ` David Wysochanski
2023-03-23 17:49 ` [Linux-cachefs] " David Wysochanski
2023-04-25 17:32 ` Chris Chilvers
2023-04-25 17:58 ` David Wysochanski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALF+zOmcROD6tzSCixYQN-+hw8bpboTrF-Ff-hEZOMDAPe7BOA@mail.gmail.com \
--to=dwysocha@redhat.com \
--cc=anna.schumaker@netapp.com \
--cc=benmaynard@google.com \
--cc=daire.byrne@gmail.com \
--cc=daire@dneg.com \
--cc=dhowells@redhat.com \
--cc=linux-cachefs@redhat.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@hammerspace.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).