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 <[email protected]>
---
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
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;
--
2.31.1
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 <[email protected]>
> ---
> 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.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Wed, Aug 24, 2022 at 8:42 AM Trond Myklebust <[email protected]> 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 <[email protected]>
> > ---
> > 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
> <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
Linux NFS client maintainer, Hammerspace
[email protected]
Trond Myklebust <[email protected]> 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 <[email protected]> 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?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
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.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
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 <[email protected]> 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 <[email protected]> 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.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
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.