| Message ID | 20220824093501.384755-3-dwysocha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert NFS to the new netfs API | expand |
On Wed, 2022-08-24 at 05:35 -0400, Dave Wysochanski wrote: > As first steps for support of the netfs library, add NETFS_SUPPORT > to Kconfig and add the required netfs_inode into struct nfs_inode. > The struct netfs_inode is now where the vfs_inode is stored as well > as the fscache_cookie. In addition, use the netfs_inode() and > netfs_i_cookie() helpers, and remove our own helper, nfs_i_fscache(). > > Later patches will enable netfs by defining NFS specific version > of struct netfs_request_ops and calling netfs_inode_init(). > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > --- > fs/nfs/Kconfig | 1 + > fs/nfs/delegation.c | 2 +- > fs/nfs/dir.c | 2 +- > fs/nfs/fscache.c | 20 +++++++++----------- > fs/nfs/fscache.h | 15 ++++++--------- > fs/nfs/inode.c | 6 +++--- > fs/nfs/internal.h | 2 +- > fs/nfs/pnfs.c | 12 ++++++------ > fs/nfs/write.c | 2 +- > include/linux/nfs_fs.h | 19 +++++-------------- > 10 files changed, 34 insertions(+), 47 deletions(-) > > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig > index 14a72224b657..79b241bed762 100644 > --- a/fs/nfs/Kconfig > +++ b/fs/nfs/Kconfig > @@ -5,6 +5,7 @@ config NFS_FS > select LOCKD > select SUNRPC > select NFS_ACL_SUPPORT if NFS_V3_ACL > + select NETFS_SUPPORT > NACK. I'm not at all OK with making netfs mandatory.
On Wed, Aug 24, 2022 at 8:42 AM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Wed, 2022-08-24 at 05:35 -0400, Dave Wysochanski wrote: > > As first steps for support of the netfs library, add NETFS_SUPPORT > > to Kconfig and add the required netfs_inode into struct nfs_inode. > > The struct netfs_inode is now where the vfs_inode is stored as well > > as the fscache_cookie. In addition, use the netfs_inode() and > > netfs_i_cookie() helpers, and remove our own helper, nfs_i_fscache(). > > > > Later patches will enable netfs by defining NFS specific version > > of struct netfs_request_ops and calling netfs_inode_init(). > > > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > > --- > > fs/nfs/Kconfig | 1 + > > fs/nfs/delegation.c | 2 +- > > fs/nfs/dir.c | 2 +- > > fs/nfs/fscache.c | 20 +++++++++----------- > > fs/nfs/fscache.h | 15 ++++++--------- > > fs/nfs/inode.c | 6 +++--- > > fs/nfs/internal.h | 2 +- > > fs/nfs/pnfs.c | 12 ++++++------ > > fs/nfs/write.c | 2 +- > > include/linux/nfs_fs.h | 19 +++++-------------- > > 10 files changed, 34 insertions(+), 47 deletions(-) > > > > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig > > index 14a72224b657..79b241bed762 100644 > > --- a/fs/nfs/Kconfig > > +++ b/fs/nfs/Kconfig > > @@ -5,6 +5,7 @@ config NFS_FS > > select LOCKD > > select SUNRPC > > select NFS_ACL_SUPPORT if NFS_V3_ACL > > + select NETFS_SUPPORT > > > > NACK. I'm not at all OK with making netfs mandatory. > Just so we're on the same page, are you ok with netfs being enabled if fscache is enabled like today?
On Wed, 2022-08-24 at 09:00 -0400, David Wysochanski wrote: > On Wed, Aug 24, 2022 at 8:42 AM Trond Myklebust > <trondmy@hammerspace.com> wrote: > > > > On Wed, 2022-08-24 at 05:35 -0400, Dave Wysochanski wrote: > > > As first steps for support of the netfs library, add > > > NETFS_SUPPORT > > > to Kconfig and add the required netfs_inode into struct > > > nfs_inode. > > > The struct netfs_inode is now where the vfs_inode is stored as > > > well > > > as the fscache_cookie. In addition, use the netfs_inode() and > > > netfs_i_cookie() helpers, and remove our own helper, > > > nfs_i_fscache(). > > > > > > Later patches will enable netfs by defining NFS specific version > > > of struct netfs_request_ops and calling netfs_inode_init(). > > > > > > Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> > > > --- > > > fs/nfs/Kconfig | 1 + > > > fs/nfs/delegation.c | 2 +- > > > fs/nfs/dir.c | 2 +- > > > fs/nfs/fscache.c | 20 +++++++++----------- > > > fs/nfs/fscache.h | 15 ++++++--------- > > > fs/nfs/inode.c | 6 +++--- > > > fs/nfs/internal.h | 2 +- > > > fs/nfs/pnfs.c | 12 ++++++------ > > > fs/nfs/write.c | 2 +- > > > include/linux/nfs_fs.h | 19 +++++-------------- > > > 10 files changed, 34 insertions(+), 47 deletions(-) > > > > > > diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig > > > index 14a72224b657..79b241bed762 100644 > > > --- a/fs/nfs/Kconfig > > > +++ b/fs/nfs/Kconfig > > > @@ -5,6 +5,7 @@ config NFS_FS > > > select LOCKD > > > select SUNRPC > > > select NFS_ACL_SUPPORT if NFS_V3_ACL > > > + select NETFS_SUPPORT > > > > > > > NACK. I'm not at all OK with making netfs mandatory. > > > > Just so we're on the same page, are you ok with netfs being enabled > if > fscache is enabled like today? > As long as it is an opt-in feature, I'm OK. I don't want to have to compile it in by default. A cachefs should never become a mandatory feature of networked filesystems.
Trond Myklebust <trondmy@hammerspace.com> wrote: > As long as it is an opt-in feature, I'm OK. I don't want to have to > compile it in by default. > A cachefs should never become a mandatory feature of networked > filesystems. netfslib is intended to be used even if fsache is not enabled. It is intended to make the underlying network filesystem maintainer's life easier by: - Moving the implementation of all the VM ops from the network filesystems as much as possible into one place. The network filesystem then just has to provide a read op and a write op. - Making it such that the filesystem doesn't have to deal with the difference between DIO and buffered I/O - Handling VM features on behalf of all filesystems. This gives the VM folk one place to change instead of 5+. mpage and iomap are similar things but for blockdev filesystems. - Providing features to those filesystems that can support them, eg.: - fscrypt - compression - bounce buffering - local caching - disconnected operation Currently nfs interacts with fscache on a page-by-page basis, but this needs to change: (1) Multipage folios are now a thing. You need to roll folios out into nfs if you're going to take advantage of this. Also, you may have noticed that all the VM interfaces are being recast in terms of folios. (2) I need to fix the cache so that it no longer uses the backing filesystem's metadata to track content. To make this use less diskspace, I want to increase the cache block size to, say, 256K or 2M. This means that the cache needs to have a say in how big a read the network filesystem does - and also that a single cache request might need to be stitched together from multiple read ops. (3) More pagecache changes are lurking in the future, possibly including getting rid of the concept of pages entirely from the pagecache. There are users of nfs + fscache and we'd like to continue to support them as best as possible but the current code noticably degrades performance here. Unfortunately, I'm also going to need to drop the fallback interface which nfs currently uses in the next couple versions, we have to at least get the fscache enabled conversion done. I've been dealing with the VM, 9p, ceph and cifs people over the direction that netfslib might need to go in, but for nfs, it's typically been a flat "no". I would like to work out how to make netfslib work for nfs also, if you're willing to discuss it. I would be open to having a look at importing nfs page handling into netfslib and working from that - but it still needs to deal with (1) and (2) above, and I would like to make it pass iterators down to the lower layers as buffer descriptions. It's also very complicated stuff. Also: - I've noted the nfs_page structs that nfs uses and I'm looking at a way of having something similar, but held separately so that one struct can span and store information about multiple folios. - I'm looking at punting write-to-the-cache to writepages() or something like that so that the VM folks can reclaim the PG_private_2 flag bit, so that won't be available to nfs either in the future. - aops->write_begin() and ->write_end() are going to go away. In netfslib what I'm trying to do is make a "netfs_perform_write" as a parallel to generic_perform_write(). David
On Wed, 2022-08-24 at 15:12 +0100, David Howells wrote: > Trond Myklebust <trondmy@hammerspace.com> wrote: > > > As long as it is an opt-in feature, I'm OK. I don't want to have to > > compile it in by default. > > A cachefs should never become a mandatory feature of networked > > filesystems. > > netfslib is intended to be used even if fsache is not enabled. It is > intended > to make the underlying network filesystem maintainer's life easier > by: > > - Moving the implementation of all the VM ops from the network > filesystems as > much as possible into one place. The network filesystem then just > has to > provide a read op and a write op. > > - Making it such that the filesystem doesn't have to deal with the > difference > between DIO and buffered I/O > > - Handling VM features on behalf of all filesystems. This gives the > VM folk > one place to change instead of 5+. mpage and iomap are similar > things but > for blockdev filesystems. > > - Providing features to those filesystems that can support them, > eg.: > > - fscrypt > - compression > - bounce buffering > - local caching > - disconnected operation > > > Currently nfs interacts with fscache on a page-by-page basis, but > this needs > to change: > > (1) Multipage folios are now a thing. You need to roll folios out > into nfs > if you're going to take advantage of this. Also, you may have > noticed > that all the VM interfaces are being recast in terms of folios. Right now, I see limited value in adding multipage folios to NFS. While basic NFSv4 does allow you to pretend there is a fundamental underlying block size, pNFS has changed all that, and we have had to engineer support for determining the I/O block size on the fly, and building the RPC requests accordingly. Client side mirroring just adds to the fun. As I see it, the only value that multipage folios might bring to NFS would be smaller page cache management overhead when dealing with large files. > > (2) I need to fix the cache so that it no longer uses the backing > filesystem's metadata to track content. To make this use less > diskspace, > I want to increase the cache block size to, say, 256K or 2M. > > This means that the cache needs to have a say in how big a read > the > network filesystem does - and also that a single cache request > might need > to be stitched together from multiple read ops. > > (3) More pagecache changes are lurking in the future, possibly > including > getting rid of the concept of pages entirely from the pagecache. > > There are users of nfs + fscache and we'd like to continue to support > them as > best as possible but the current code noticably degrades performance > here. > > Unfortunately, I'm also going to need to drop the fallback interface > which nfs > currently uses in the next couple versions, we have to at least get > the > fscache enabled conversion done. > > I've been dealing with the VM, 9p, ceph and cifs people over the > direction > that netfslib might need to go in, but for nfs, it's typically been a > flat > "no". I would like to work out how to make netfslib work for nfs > also, if > you're willing to discuss it. > > I would be open to having a look at importing nfs page handling into > netfslib > and working from that - but it still needs to deal with (1) and (2) > above, and > I would like to make it pass iterators down to the lower layers as > buffer > descriptions. It's also very complicated stuff. > > Also: > > - I've noted the nfs_page structs that nfs uses and I'm looking at a > way of > having something similar, but held separately so that one struct > can span > and store information about multiple folios. > > - I'm looking at punting write-to-the-cache to writepages() or > something like > that so that the VM folks can reclaim the PG_private_2 flag bit, > so that > won't be available to nfs either in the future. > > - aops->write_begin() and ->write_end() are going to go away. In > netfslib > what I'm trying to do is make a "netfs_perform_write" as a > parallel to > generic_perform_write(). > What problems would any of this solve for NFS? I'm worried about the cost of all this proposed code churn as well; as you said 'it is complicated stuff', mainly for the good reason that we've been optimising a lot of code over the last 25-30 years. However let's start with the "why?" question first. Why do I need an extra layer of abstraction between NFS and the VM, when one of my primary concerns right now is that the stack depth keeps growing?
On Wed, Aug 24, 2022 at 04:27:04PM +0000, Trond Myklebust wrote: > Right now, I see limited value in adding multipage folios to NFS. > > While basic NFSv4 does allow you to pretend there is a fundamental > underlying block size, pNFS has changed all that, and we have had to > engineer support for determining the I/O block size on the fly, and > building the RPC requests accordingly. Client side mirroring just adds > to the fun. > > As I see it, the only value that multipage folios might bring to NFS > would be smaller page cache management overhead when dealing with large > files. Yes, but that's a Really Big Deal. Machines with a lot of memory end up with very long LRU lists. We can't afford the overhead of managing memory in 4kB chunks any more. (I don't want to dwell on this point too much; I've run the numbers before and can do so again if you want me to go into more details). Beyond that, filesystems have a lot of interactions with the page cache today. When I started looking at this, I thought filesystem people all had a deep understanding of how the page cache worked. Now I realise everyone's as clueless as I am. The real benefit I see to projects like iomap/netfs is that they insulate filesystems from having to deal with the page cache. All the interactions are in two or three places and we can refactor without having to talk to the owners of 50+ filesystems. It also gives us a chance to re-examine some of the assumptions that we have made over the years about how filesystems and page cache should be interacting. We've fixed a fair few bugs in recent years that came about because filesystem people don't tend to have deep knowledge of mm internals (and they shouldn't need to!) I don't know that netfs has the perfect interface to be used for nfs. But that too can be changed to make it work better for your needs.
On Wed, 2022-08-24 at 17:53 +0100, Matthew Wilcox wrote: > On Wed, Aug 24, 2022 at 04:27:04PM +0000, Trond Myklebust wrote: > > Right now, I see limited value in adding multipage folios to NFS. > > > > While basic NFSv4 does allow you to pretend there is a fundamental > > underlying block size, pNFS has changed all that, and we have had > > to > > engineer support for determining the I/O block size on the fly, and > > building the RPC requests accordingly. Client side mirroring just > > adds > > to the fun. > > > > As I see it, the only value that multipage folios might bring to > > NFS > > would be smaller page cache management overhead when dealing with > > large > > files. > > Yes, but that's a Really Big Deal. Machines with a lot of memory end > up with very long LRU lists. We can't afford the overhead of > managing > memory in 4kB chunks any more. (I don't want to dwell on this point > too > much; I've run the numbers before and can do so again if you want me > to > go into more details). > > Beyond that, filesystems have a lot of interactions with the page > cache > today. When I started looking at this, I thought filesystem people > all > had a deep understanding of how the page cache worked. Now I realise > everyone's as clueless as I am. The real benefit I see to projects > like > iomap/netfs is that they insulate filesystems from having to deal > with > the page cache. All the interactions are in two or three places and > we > can refactor without having to talk to the owners of 50+ filesystems. > > It also gives us a chance to re-examine some of the assumptions that > we have made over the years about how filesystems and page cache > should > be interacting. We've fixed a fair few bugs in recent years that > came > about because filesystem people don't tend to have deep knowledge of > mm > internals (and they shouldn't need to!) > > I don't know that netfs has the perfect interface to be used for nfs. > But that too can be changed to make it work better for your needs. If the VM folks need it, then adding support for multi-page folios is a much smaller scope than what David was describing. It can be done without too much surgery to the existing NFS I/O stack. We already have code to support I/O block sizes that are much less than the page size, so converting that to act on larger folios is not a huge deal. What would be useful there is something like a range tree to allow us to move beyond the PG_uptodate bit, and help make the is_partially_uptodate() address_space_operation a bit more useful. Otherwise, we end up having to read in the entire folio, which is what we do today for pages, but could get onerous with large folios when doing file random access.
On Wed, Aug 24, 2022 at 05:43:36PM +0000, Trond Myklebust wrote: > On Wed, 2022-08-24 at 17:53 +0100, Matthew Wilcox wrote: > > On Wed, Aug 24, 2022 at 04:27:04PM +0000, Trond Myklebust wrote: > > > Right now, I see limited value in adding multipage folios to NFS. > > > > > > While basic NFSv4 does allow you to pretend there is a fundamental > > > underlying block size, pNFS has changed all that, and we have had > > > to > > > engineer support for determining the I/O block size on the fly, and > > > building the RPC requests accordingly. Client side mirroring just > > > adds > > > to the fun. > > > > > > As I see it, the only value that multipage folios might bring to > > > NFS > > > would be smaller page cache management overhead when dealing with > > > large > > > files. > > > > Yes, but that's a Really Big Deal. Machines with a lot of memory end > > up with very long LRU lists. We can't afford the overhead of > > managing > > memory in 4kB chunks any more. (I don't want to dwell on this point > > too > > much; I've run the numbers before and can do so again if you want me > > to > > go into more details). > > > > Beyond that, filesystems have a lot of interactions with the page > > cache > > today. When I started looking at this, I thought filesystem people > > all > > had a deep understanding of how the page cache worked. Now I realise > > everyone's as clueless as I am. The real benefit I see to projects > > like > > iomap/netfs is that they insulate filesystems from having to deal > > with > > the page cache. All the interactions are in two or three places and > > we > > can refactor without having to talk to the owners of 50+ filesystems. > > > > It also gives us a chance to re-examine some of the assumptions that > > we have made over the years about how filesystems and page cache > > should > > be interacting. We've fixed a fair few bugs in recent years that > > came > > about because filesystem people don't tend to have deep knowledge of > > mm > > internals (and they shouldn't need to!) > > > > I don't know that netfs has the perfect interface to be used for nfs. > > But that too can be changed to make it work better for your needs. > > If the VM folks need it, then adding support for multi-page folios is a > much smaller scope than what David was describing. It can be done > without too much surgery to the existing NFS I/O stack. We already have > code to support I/O block sizes that are much less than the page size, > so converting that to act on larger folios is not a huge deal. > > What would be useful there is something like a range tree to allow us > to move beyond the PG_uptodate bit, and help make the > is_partially_uptodate() address_space_operation a bit more useful. > Otherwise, we end up having to read in the entire folio, which is what > we do today for pages, but could get onerous with large folios when > doing file random access. This is interesting because nobody's asked for this before. I've had similar discussions around dirty data tracking, but not around uptodate. Random small reads shouldn't be a terrible problem; if they truly are random, we behave as today, allocating single pages, reading the entire page from the server and setting it uptodate. If the readahead code detects a contiguous large read, we increase the allocation size to match, but again we always read the entire folio from the server and mark it uptodate. As far as I know, the only time we create !uptodate folios in the page cache is partial writes to a folio which has not been previously read. Obviously, those bytes start out dirty and are tracked through the existing dirty mechanism, but once they've been written back, we have three choices that I can see: 1. transition those bytes to a mechanism which records they're uptodate 2. discard that information and re-read the entire folio from the server if any bytes are subsequently read 3. read the other bytes in that folio from the server and mark the entire folio uptodate We have a mixture of those options implemented in different filesystems today. iomap records whether a block is uptodate or not and treats every uptodate block as dirty if any block in the folio is dirty. buffer_head has two bits for each block, separately recording whether it's dirty and/or uptodate. AFS tracks one dirty range per folio, but it first brings the folio uptodate by reading it from the server before overwriting it (I suppose that's a fourth option). I don't see a compelling reason for different filesystems to behave differently here. I'd like us to settle on one design we can all share, and I was hoping netfs would be the platform for that.
Trond Myklebust <trondmy@hammerspace.com> wrote: > While basic NFSv4 does allow you to pretend there is a fundamental > underlying block size, pNFS has changed all that, and we have had to > engineer support for determining the I/O block size on the fly, and > building the RPC requests accordingly. Client side mirroring just adds > to the fun. I've been working with Jeff to make netfslib handle ceph with its distributed object model as well as 9p and afs with their more traditionally-appearing flat files. > However let's start with the "why?" question first. Why do I need an > extra layer of abstraction between NFS and the VM, when one of my > primary concerns right now is that the stack depth keeps growing? It's not exactly an extra layer - it's more a case of taking the same layer out of five[*] network filesystems, combining them and sharing it. [*] up to 7 if I can roll it out into orangefs and/or fuse as well. As to why, well I kind of covered that, but we want to add some services to network filesystems (such as content encryption) and rather than adding separately to all five, there exists the possibility of just doing it the once and sharing it (granted there may be parts that can't be shared). But also, I need to fix cachefiles - and I can't do that whilst nfs is operating on a page-by-page basis. Cachefiles has to have an early say on the size and shape of a transaction. And speaking of content encryption, if you're using a local cache and content encryption, you really don't want the unencrypted data to be stored in your local cache on your laptop, say - so that requires storage of the encrypted data into the cache. Further, the VM folks would like the PG_private_2 bit back, along with PG_checked and PG_error. So we need a different way of managing writes to the cache and preventing overlapping DIO writes. > What problems would any of this solve for NFS? I'm worried about the > cost of all this proposed code churn as well; as you said 'it is > complicated stuff', mainly for the good reason that we've been > optimising a lot of code over the last 25-30 years. First off, NFS would get to partake of services being implemented in netfslib. Granted, this isn't exactly solving problems in NFS, more providing additional features. Secondly, shared code means less code - and the code would, in theory, be better-tested as it would have more users. Thirdly, it would hopefully reduce the maintenance burden, particularly for the VM people. David
Matthew Wilcox <willy@infradead.org> wrote: > AFS tracks one dirty range per folio, but it first brings the folio uptodate > by reading it from the server before overwriting it (I suppose that's a > fourth option). I'm intending on moving afs towards the nfs way of doing things when writing to as-yet unread folios - unless a cache is in operation, then we read it anyway and store the folio(s) into the cache unless the entire cache granule is going to be overwritten unless we're supporting disconnected mode. I know that's exceptions-to-exceptions. David
On Thu, 2022-08-25 at 16:01 +0100, Matthew Wilcox wrote: > On Wed, Aug 24, 2022 at 05:43:36PM +0000, Trond Myklebust wrote: > > On Wed, 2022-08-24 at 17:53 +0100, Matthew Wilcox wrote: > > > On Wed, Aug 24, 2022 at 04:27:04PM +0000, Trond Myklebust wrote: > > > > Right now, I see limited value in adding multipage folios to > > > > NFS. > > > > > > > > While basic NFSv4 does allow you to pretend there is a > > > > fundamental > > > > underlying block size, pNFS has changed all that, and we have > > > > had > > > > to > > > > engineer support for determining the I/O block size on the fly, > > > > and > > > > building the RPC requests accordingly. Client side mirroring > > > > just > > > > adds > > > > to the fun. > > > > > > > > As I see it, the only value that multipage folios might bring > > > > to > > > > NFS > > > > would be smaller page cache management overhead when dealing > > > > with > > > > large > > > > files. > > > > > > Yes, but that's a Really Big Deal. Machines with a lot of memory > > > end > > > up with very long LRU lists. We can't afford the overhead of > > > managing > > > memory in 4kB chunks any more. (I don't want to dwell on this > > > point > > > too > > > much; I've run the numbers before and can do so again if you want > > > me > > > to > > > go into more details). > > > > > > Beyond that, filesystems have a lot of interactions with the page > > > cache > > > today. When I started looking at this, I thought filesystem > > > people > > > all > > > had a deep understanding of how the page cache worked. Now I > > > realise > > > everyone's as clueless as I am. The real benefit I see to > > > projects > > > like > > > iomap/netfs is that they insulate filesystems from having to deal > > > with > > > the page cache. All the interactions are in two or three places > > > and > > > we > > > can refactor without having to talk to the owners of 50+ > > > filesystems. > > > > > > It also gives us a chance to re-examine some of the assumptions > > > that > > > we have made over the years about how filesystems and page cache > > > should > > > be interacting. We've fixed a fair few bugs in recent years that > > > came > > > about because filesystem people don't tend to have deep knowledge > > > of > > > mm > > > internals (and they shouldn't need to!) > > > > > > I don't know that netfs has the perfect interface to be used for > > > nfs. > > > But that too can be changed to make it work better for your > > > needs. > > > > If the VM folks need it, then adding support for multi-page folios > > is a > > much smaller scope than what David was describing. It can be done > > without too much surgery to the existing NFS I/O stack. We already > > have > > code to support I/O block sizes that are much less than the page > > size, > > so converting that to act on larger folios is not a huge deal. > > > > What would be useful there is something like a range tree to allow > > us > > to move beyond the PG_uptodate bit, and help make the > > is_partially_uptodate() address_space_operation a bit more useful. > > Otherwise, we end up having to read in the entire folio, which is > > what > > we do today for pages, but could get onerous with large folios when > > doing file random access. > > This is interesting because nobody's asked for this before. I've had > similar discussions around dirty data tracking, but not around > uptodate. > Random small reads shouldn't be a terrible problem; if they truly are > random, we behave as today, allocating single pages, reading the > entire > page from the server and setting it uptodate. If the readahead code > detects a contiguous large read, we increase the allocation size to > match, but again we always read the entire folio from the server and > mark it uptodate. > > As far as I know, the only time we create !uptodate folios in the > page > cache is partial writes to a folio which has not been previously > read. > Obviously, those bytes start out dirty and are tracked through the > existing dirty mechanism, but once they've been written back, we have > three choices that I can see: > > 1. transition those bytes to a mechanism which records they're > uptodate > 2. discard that information and re-read the entire folio from the > server > if any bytes are subsequently read > 3. read the other bytes in that folio from the server and mark the > entire folio uptodate > > We have a mixture of those options implemented in different > filesystems > today. iomap records whether a block is uptodate or not and treats > every uptodate block as dirty if any block in the folio is dirty. > buffer_head has two bits for each block, separately recording whether > it's dirty and/or uptodate. AFS tracks one dirty range per folio, > but > it first brings the folio uptodate by reading it from the server > before > overwriting it (I suppose that's a fourth option). > I'm not talking about the transition of dirty->clean. We already deal with that. I'm talking about supporting large folios on read-mainly workloads. NFS can happily support 1MB sized folios, or even larger than that if there is a compelling reason to do so. However, having to read in the entire folio contents if the user is just asking for a few bytes on a database-style random read workload can quickly get onerous. While a lot of NFS servers can do 1MB reads in one RPC call, there are still many out there that can't. For those servers, we'd have to fall back to issuing multiple read RPC calls in parallel (which is what we do today if the user sets an rsize < PAGE_SIZE). This leads to unnecessary load on the server, which has to deal with multiple RPC calls for data that won't be used. The other point is that if your network bandwidth is limited, there is value in avoiding reads for data that isn't going to be used, which is why we changed the NFS readahead behaviour to be less aggressive than it used to be. This is why I'm suggesting that if you really want to cut down the LRU table size, you'll want finer grained page up to date tracking than the folio. It's not so much for the case of writes as it is for the read- mostly workloads.
On Thu, Aug 25, 2022 at 03:32:25PM +0000, Trond Myklebust wrote: > I'm not talking about the transition of dirty->clean. We already deal > with that. I'm talking about supporting large folios on read-mainly > workloads. > > NFS can happily support 1MB sized folios, or even larger than that if > there is a compelling reason to do so. > > However, having to read in the entire folio contents if the user is > just asking for a few bytes on a database-style random read workload > can quickly get onerous. Ah, we don't do that. If the user is only asking for a few bytes and there's no indication that it's part of a streaming read, we allocate & fill a single page, just as before. We adapt to the user's workload and only allocate multi-page folios when there's evidence that it'll be useful to do so.
diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig index 14a72224b657..79b241bed762 100644 --- a/fs/nfs/Kconfig +++ b/fs/nfs/Kconfig @@ -5,6 +5,7 @@ config NFS_FS select LOCKD select SUNRPC select NFS_ACL_SUPPORT if NFS_V3_ACL + select NETFS_SUPPORT help Choose Y here if you want to access files residing on other computers using Sun's Network File System protocol. To compile diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c index 5c97cad741a7..b80d6ae1ead6 100644 --- a/fs/nfs/delegation.c +++ b/fs/nfs/delegation.c @@ -306,7 +306,7 @@ nfs_start_delegation_return_locked(struct nfs_inode *nfsi) } spin_unlock(&delegation->lock); if (ret) - nfs_clear_verifier_delegated(&nfsi->vfs_inode); + nfs_clear_verifier_delegated(&nfsi->netfs.inode); out: return ret; } diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index dbab3caa15ed..e76d6d05e0e1 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -2796,7 +2796,7 @@ nfs_do_access_cache_scan(unsigned int nr_to_scan) if (nr_to_scan-- == 0) break; - inode = &nfsi->vfs_inode; + inode = &nfsi->netfs.inode; spin_lock(&inode->i_lock); if (list_empty(&nfsi->access_cache_entry_lru)) goto remove_lru_entry; diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c index e861d7bae305..a6fc1c8b6644 100644 --- a/fs/nfs/fscache.c +++ b/fs/nfs/fscache.c @@ -163,13 +163,14 @@ void nfs_fscache_init_inode(struct inode *inode) struct nfs_server *nfss = NFS_SERVER(inode); struct nfs_inode *nfsi = NFS_I(inode); - nfsi->fscache = NULL; + netfs_inode(inode)->cache = NULL; if (!(nfss->fscache && S_ISREG(inode->i_mode))) return; nfs_fscache_update_auxdata(&auxdata, inode); - nfsi->fscache = fscache_acquire_cookie(NFS_SB(inode->i_sb)->fscache, + netfs_inode(inode)->cache = fscache_acquire_cookie( + nfss->fscache, 0, nfsi->fh.data, /* index_key */ nfsi->fh.size, @@ -183,11 +184,8 @@ void nfs_fscache_init_inode(struct inode *inode) */ void nfs_fscache_clear_inode(struct inode *inode) { - struct nfs_inode *nfsi = NFS_I(inode); - struct fscache_cookie *cookie = nfs_i_fscache(inode); - - fscache_relinquish_cookie(cookie, false); - nfsi->fscache = NULL; + fscache_relinquish_cookie(netfs_i_cookie(&NFS_I(inode)->netfs), false); + netfs_inode(inode)->cache = NULL; } /* @@ -212,7 +210,7 @@ void nfs_fscache_clear_inode(struct inode *inode) void nfs_fscache_open_file(struct inode *inode, struct file *filp) { struct nfs_fscache_inode_auxdata auxdata; - struct fscache_cookie *cookie = nfs_i_fscache(inode); + struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs); bool open_for_write = inode_is_open_for_write(inode); if (!fscache_cookie_valid(cookie)) @@ -230,7 +228,7 @@ EXPORT_SYMBOL_GPL(nfs_fscache_open_file); void nfs_fscache_release_file(struct inode *inode, struct file *filp) { struct nfs_fscache_inode_auxdata auxdata; - struct fscache_cookie *cookie = nfs_i_fscache(inode); + struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs); loff_t i_size = i_size_read(inode); nfs_fscache_update_auxdata(&auxdata, inode); @@ -243,7 +241,7 @@ void nfs_fscache_release_file(struct inode *inode, struct file *filp) static int fscache_fallback_read_page(struct inode *inode, struct page *page) { struct netfs_cache_resources cres; - struct fscache_cookie *cookie = nfs_i_fscache(inode); + struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs); struct iov_iter iter; struct bio_vec bvec[1]; int ret; @@ -271,7 +269,7 @@ static int fscache_fallback_write_page(struct inode *inode, struct page *page, bool no_space_allocated_yet) { struct netfs_cache_resources cres; - struct fscache_cookie *cookie = nfs_i_fscache(inode); + struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs); struct iov_iter iter; struct bio_vec bvec[1]; loff_t start = page_offset(page); diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h index 2a37af880978..38614ed8f951 100644 --- a/fs/nfs/fscache.h +++ b/fs/nfs/fscache.h @@ -54,7 +54,7 @@ static inline bool nfs_fscache_release_folio(struct folio *folio, gfp_t gfp) if (current_is_kswapd() || !(gfp & __GFP_FS)) return false; folio_wait_fscache(folio); - fscache_note_page_release(nfs_i_fscache(folio->mapping->host)); + fscache_note_page_release(netfs_i_cookie(&NFS_I(folio->mapping->host)->netfs)); nfs_inc_fscache_stats(folio->mapping->host, NFSIOS_FSCACHE_PAGES_UNCACHED); } @@ -66,7 +66,7 @@ static inline bool nfs_fscache_release_folio(struct folio *folio, gfp_t gfp) */ static inline int nfs_fscache_read_page(struct inode *inode, struct page *page) { - if (nfs_i_fscache(inode)) + if (netfs_inode(inode)->cache) return __nfs_fscache_read_page(inode, page); return -ENOBUFS; } @@ -78,7 +78,7 @@ static inline int nfs_fscache_read_page(struct inode *inode, struct page *page) static inline void nfs_fscache_write_page(struct inode *inode, struct page *page) { - if (nfs_i_fscache(inode)) + if (netfs_inode(inode)->cache) __nfs_fscache_write_page(inode, page); } @@ -101,13 +101,10 @@ static inline void nfs_fscache_update_auxdata(struct nfs_fscache_inode_auxdata * static inline void nfs_fscache_invalidate(struct inode *inode, int flags) { struct nfs_fscache_inode_auxdata auxdata; - struct nfs_inode *nfsi = NFS_I(inode); + struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs); - if (nfsi->fscache) { - nfs_fscache_update_auxdata(&auxdata, inode); - fscache_invalidate(nfsi->fscache, &auxdata, - i_size_read(inode), flags); - } + nfs_fscache_update_auxdata(&auxdata, inode); + fscache_invalidate(cookie, &auxdata, i_size_read(inode), flags); } /* diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index b4e46b0ffa2d..3acddec41ba7 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1410,7 +1410,7 @@ int nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping) static bool nfs_file_has_writers(struct nfs_inode *nfsi) { - struct inode *inode = &nfsi->vfs_inode; + struct inode *inode = &nfsi->netfs.inode; if (!S_ISREG(inode->i_mode)) return false; @@ -2248,7 +2248,7 @@ struct inode *nfs_alloc_inode(struct super_block *sb) #ifdef CONFIG_NFS_V4_2 nfsi->xattr_cache = NULL; #endif - return &nfsi->vfs_inode; + return &nfsi->netfs.inode; } EXPORT_SYMBOL_GPL(nfs_alloc_inode); @@ -2272,7 +2272,7 @@ static void init_once(void *foo) { struct nfs_inode *nfsi = (struct nfs_inode *) foo; - inode_init_once(&nfsi->vfs_inode); + inode_init_once(&nfsi->netfs.inode); INIT_LIST_HEAD(&nfsi->open_files); INIT_LIST_HEAD(&nfsi->access_cache_entry_lru); INIT_LIST_HEAD(&nfsi->access_cache_inode_lru); diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index 27c720d71b4e..29cfee4660c5 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -355,7 +355,7 @@ nfs4_label_copy(struct nfs4_label *dst, struct nfs4_label *src) static inline void nfs_zap_label_cache_locked(struct nfs_inode *nfsi) { - if (nfs_server_capable(&nfsi->vfs_inode, NFS_CAP_SECURITY_LABEL)) + if (nfs_server_capable(&nfsi->netfs.inode, NFS_CAP_SECURITY_LABEL)) nfsi->cache_validity |= NFS_INO_INVALID_LABEL; } #else diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index 41a9b6b58fb9..4087de475829 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -763,19 +763,19 @@ static struct pnfs_layout_hdr *__pnfs_destroy_layout(struct nfs_inode *nfsi) struct pnfs_layout_hdr *lo; LIST_HEAD(tmp_list); - spin_lock(&nfsi->vfs_inode.i_lock); + spin_lock(&nfsi->netfs.inode.i_lock); lo = nfsi->layout; if (lo) { pnfs_get_layout_hdr(lo); pnfs_mark_layout_stateid_invalid(lo, &tmp_list); pnfs_layout_clear_fail_bit(lo, NFS_LAYOUT_RO_FAILED); pnfs_layout_clear_fail_bit(lo, NFS_LAYOUT_RW_FAILED); - spin_unlock(&nfsi->vfs_inode.i_lock); + spin_unlock(&nfsi->netfs.inode.i_lock); pnfs_free_lseg_list(&tmp_list); - nfs_commit_inode(&nfsi->vfs_inode, 0); + nfs_commit_inode(&nfsi->netfs.inode, 0); pnfs_put_layout_hdr(lo); } else - spin_unlock(&nfsi->vfs_inode.i_lock); + spin_unlock(&nfsi->netfs.inode.i_lock); return lo; } @@ -790,9 +790,9 @@ static bool pnfs_layout_removed(struct nfs_inode *nfsi, { bool ret; - spin_lock(&nfsi->vfs_inode.i_lock); + spin_lock(&nfsi->netfs.inode.i_lock); ret = nfsi->layout != lo; - spin_unlock(&nfsi->vfs_inode.i_lock); + spin_unlock(&nfsi->netfs.inode.i_lock); return ret; } diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 51a7e202d6e5..6a8bf34662b9 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -828,7 +828,7 @@ nfs_page_search_commits_for_head_request_locked(struct nfs_inode *nfsi, { struct nfs_page *freq, *t; struct nfs_commit_info cinfo; - struct inode *inode = &nfsi->vfs_inode; + struct inode *inode = &nfsi->netfs.inode; nfs_init_cinfo_from_inode(&cinfo, inode); diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index b32ed68e7dc4..1df6c48abc7c 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -31,6 +31,8 @@ #include <linux/sunrpc/auth.h> #include <linux/sunrpc/clnt.h> +#include <linux/netfs.h> + #include <linux/nfs.h> #include <linux/nfs2.h> #include <linux/nfs3.h> @@ -122,6 +124,8 @@ struct nfs4_xattr_cache; * nfs fs inode data in memory */ struct nfs_inode { + struct netfs_inode netfs; /* Netfslib context and vfs inode */ + /* * The 64bit 'inode number' */ @@ -203,10 +207,6 @@ struct nfs_inode { /* how many bytes have been written/read and how many bytes queued up */ __u64 write_io; __u64 read_io; -#ifdef CONFIG_NFS_FSCACHE - struct fscache_cookie *fscache; -#endif - struct inode vfs_inode; #ifdef CONFIG_NFS_V4_2 struct nfs4_xattr_cache *xattr_cache; @@ -283,7 +283,7 @@ struct nfs4_copy_state { static inline struct nfs_inode *NFS_I(const struct inode *inode) { - return container_of(inode, struct nfs_inode, vfs_inode); + return container_of(inode, struct nfs_inode, netfs.inode); } static inline struct nfs_server *NFS_SB(const struct super_block *s) @@ -328,15 +328,6 @@ static inline int NFS_STALE(const struct inode *inode) return test_bit(NFS_INO_STALE, &NFS_I(inode)->flags); } -static inline struct fscache_cookie *nfs_i_fscache(struct inode *inode) -{ -#ifdef CONFIG_NFS_FSCACHE - return NFS_I(inode)->fscache; -#else - return NULL; -#endif -} - static inline __u64 NFS_FILEID(const struct inode *inode) { return NFS_I(inode)->fileid;
As first steps for support of the netfs library, add NETFS_SUPPORT to Kconfig and add the required netfs_inode into struct nfs_inode. The struct netfs_inode is now where the vfs_inode is stored as well as the fscache_cookie. In addition, use the netfs_inode() and netfs_i_cookie() helpers, and remove our own helper, nfs_i_fscache(). Later patches will enable netfs by defining NFS specific version of struct netfs_request_ops and calling netfs_inode_init(). Signed-off-by: Dave Wysochanski <dwysocha@redhat.com> --- fs/nfs/Kconfig | 1 + fs/nfs/delegation.c | 2 +- fs/nfs/dir.c | 2 +- fs/nfs/fscache.c | 20 +++++++++----------- fs/nfs/fscache.h | 15 ++++++--------- fs/nfs/inode.c | 6 +++--- fs/nfs/internal.h | 2 +- fs/nfs/pnfs.c | 12 ++++++------ fs/nfs/write.c | 2 +- include/linux/nfs_fs.h | 19 +++++-------------- 10 files changed, 34 insertions(+), 47 deletions(-)