* [PATCH v10 0/6] Convert NFS with fscache to the netfs API
@ 2022-11-03 16:16 Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 1/6] NFS: Rename readpage_async_filler to nfs_read_add_page Dave Wysochanski
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
This v10 patchset addresses at least some of Trond's latest concerns.
Some of the feedback like the unlock_page() wrapper function in
nfs_read_completion() I don't know how to address without an
ifdef. Other feedback I'm not quite sure about splitting out
netfs bits or what you would like to see. Trond I do not want to
in any way ignore or miss any of your feedback so please elaborate
as needed.
This patchset converts NFS with fscache non-direct 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 concerns about modifying the IO path [1] as
well as only enabling netfs when fscache is configured and enabled [2].
I have not attempted performance comparisions to address Chuck
Lever's concern [3] because we are not converting the non-fscache
enabled NFS IO paths to netfs.
The patchset is based on 6.1-rc3 and has been pushed to github at:
https://github.com/DaveWysochanskiRH/kernel/commits/nfs-fscache-netfs
https://github.com/DaveWysochanskiRH/kernel/commit/bff09aa979010f38a11a6f92451e85d04d850715
Changes since v9 [7]
====================
PATCH1: Rename nfs_pageio_add_page to nfs_read_add_page (Trond)
PATCH3: Remove a few #ifdef's and replace with wrappers (Trond) [8]
PATCH6: RFC patch to reduce increase in nfs_inode memory footprint
when netfs is configured but not enabled (Trond) [9]
Testing
=======
I did not do much testing on this as the changes to patches 1 and 3
are cosmetic. Patch #6 is RFC patch and may change, so if that is
added it may need more testing.
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 linux-cachefs [4] patch to resolve
"Stop read optimisation when folio removed from pagecache"
* Daire Byrne also verified the patch fixes his issue as well
2. "Cache volume key already in use" after xfstest runs
* xfstests (hammerspace with vers=4.2,fsc) shows the following on the
console after some tests:
"NFS: Cache volume key already in use (nfs,4.1,2,c50,cfe0100a,3,,,8000,100000,100000,bb8,ea60,7530,ea60,1)"
* This may be fixed with another patch [5] that is in progress
3. Daire Byrne reported a NULL pointer oops at cachefiles_prepare_write+0x28/0x90
* harder to reproduce/debug but under investigation [6]
* only reproduced on RHEL7.9 based NFS re-export server using fscache with upstream kernel plus
the previous patches
* Debug in progress, first pass at where the problem is indicates a race
between fscache cookie LRU and use_cookie; looking at cookie state machine [10]
[58710.346376] BUG: kernel NULL pointer dereference, address: 0000000000000008
[58710.371212] CPU: 12 PID: 9134 Comm: kworker/u129:0 Tainted: G E 6.0.0-2.dneg.x86_64 #1
...
[58710.389995] Workqueue: events_unbound netfs_rreq_write_to_cache_work [netfs]
[58710.397188] RIP: 0010:cachefiles_prepare_write+0x28/0x90 [cachefiles]
...
[58710.500316] Call Trace:
[58710.502894] <TASK>
[58710.505126] netfs_rreq_write_to_cache_work+0x11c/0x320 [netfs]
[58710.511201] process_one_work+0x217/0x3e0
[58710.515358] worker_thread+0x4a/0x3b0
[58710.519152] ? process_one_work+0x3e0/0x3e0
[58710.523467] kthread+0xd6/0x100
[58710.526740] ? kthread_complete_and_exit+0x20/0x20
[58710.531659] ret_from_fork+0x1f/0x30
References
==========
[1] https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@hammerspace.com/
[2] https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@hammerspace.com/
[3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
[4] https://www.mail-archive.com/linux-cachefs@redhat.com/msg03043.html
[5] https://marc.info/?l=linux-nfs&m=165962662200679&w=4
[6] https://listman.redhat.com/archives/linux-cachefs/2022-September/007183.html
[7] https://marc.info/?l=linux-nfs&m=166600357429305&w=4
[8] https://marc.info/?l=linux-nfs&m=166697599503342&w=4
[9] https://marc.info/?l=linux-nfs&m=166717208305834&w=4
[10] https://listman.redhat.com/archives/linux-cachefs/2022-October/007259.html
Dave Wysochanski (5):
NFS: Rename readpage_async_filler to nfs_pageio_add_page
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/delegation.c | 2 +-
fs/nfs/dir.c | 2 +-
fs/nfs/fscache.c | 242 ++++++++++++++++++++++---------------
fs/nfs/fscache.h | 111 +++++++++++------
fs/nfs/inode.c | 8 +-
fs/nfs/internal.h | 11 +-
fs/nfs/iostat.h | 17 ---
fs/nfs/nfstrace.h | 91 --------------
fs/nfs/pagelist.c | 12 ++
fs/nfs/pnfs.c | 12 +-
fs/nfs/read.c | 110 +++++++++--------
fs/nfs/super.c | 11 --
fs/nfs/write.c | 2 +-
include/linux/nfs_fs.h | 35 ++++--
include/linux/nfs_iostat.h | 12 --
include/linux/nfs_page.h | 3 +
include/linux/nfs_xdr.h | 3 +
18 files changed, 335 insertions(+), 350 deletions(-)
--
2.31.1
*** BLURB HERE ***
Dave Wysochanski (6):
NFS: Rename readpage_async_filler to nfs_read_add_page
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
netfs: Change netfs_inode_init to allocate memory to allow opt-in
fs/9p/cache.c | 2 +-
fs/9p/vfs_inode.c | 17 ++-
fs/afs/dynroot.c | 7 +-
fs/afs/inode.c | 14 +--
fs/afs/internal.h | 2 +-
fs/afs/super.c | 7 ++
fs/afs/write.c | 2 +-
fs/ceph/inode.c | 6 +-
fs/netfs/buffered_read.c | 16 +--
fs/netfs/internal.h | 2 +-
fs/netfs/objects.c | 2 +-
fs/nfs/Kconfig | 1 +
fs/nfs/delegation.c | 2 +-
fs/nfs/dir.c | 2 +-
fs/nfs/fscache.c | 242 ++++++++++++++++++++++---------------
fs/nfs/fscache.h | 136 +++++++++++++++------
fs/nfs/inode.c | 15 ++-
fs/nfs/internal.h | 11 +-
fs/nfs/iostat.h | 17 ---
fs/nfs/nfstrace.h | 91 --------------
fs/nfs/pagelist.c | 4 +
fs/nfs/pnfs.c | 12 +-
fs/nfs/read.c | 110 +++++++++--------
fs/nfs/super.c | 11 --
fs/nfs/write.c | 2 +-
include/linux/netfs.h | 41 +++++--
include/linux/nfs_fs.h | 35 ++++--
include/linux/nfs_iostat.h | 12 --
include/linux/nfs_page.h | 3 +
include/linux/nfs_xdr.h | 3 +
30 files changed, 428 insertions(+), 399 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v10 1/6] NFS: Rename readpage_async_filler to nfs_read_add_page
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 2/6] NFS: Configure support for netfs when NFS fscache is configured Dave Wysochanski
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
Rename readpage_async_filler to nfs_read_add_page to
better reflect what this function does (add a page to
the nfs_pageio_descriptor), and simplify arguments to
this function by removing struct nfs_readdesc.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
fs/nfs/read.c | 58 +++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 8ae2c8d1219d..71267a3174f1 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -127,11 +127,6 @@ static void nfs_readpage_release(struct nfs_page *req, int error)
nfs_release_request(req);
}
-struct nfs_readdesc {
- struct nfs_pageio_descriptor pgio;
- struct nfs_open_context *ctx;
-};
-
static void nfs_page_group_set_uptodate(struct nfs_page *req)
{
if (nfs_page_group_sync_on_bit(req, PG_UPTODATE))
@@ -153,7 +148,8 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr)
if (test_bit(NFS_IOHDR_EOF, &hdr->flags)) {
/* note: regions of the page not covered by a
- * request are zeroed in readpage_async_filler */
+ * request are zeroed in nfs_read_add_page
+ */
if (bytes > hdr->good_bytes) {
/* nothing in this request was good, so zero
* the full extent of the request */
@@ -282,7 +278,9 @@ static void nfs_readpage_result(struct rpc_task *task,
}
static int
-readpage_async_filler(struct nfs_readdesc *desc, struct page *page)
+nfs_read_add_page(struct nfs_pageio_descriptor *pgio,
+ struct nfs_open_context *ctx,
+ struct page *page)
{
struct inode *inode = page_file_mapping(page)->host;
unsigned int rsize = NFS_SERVER(inode)->rsize;
@@ -302,15 +300,15 @@ readpage_async_filler(struct nfs_readdesc *desc, struct page *page)
goto out_unlock;
}
- new = nfs_create_request(desc->ctx, page, 0, aligned_len);
+ new = nfs_create_request(ctx, page, 0, aligned_len);
if (IS_ERR(new))
goto out_error;
if (len < PAGE_SIZE)
zero_user_segment(page, len, PAGE_SIZE);
- if (!nfs_pageio_add_request(&desc->pgio, new)) {
+ if (!nfs_pageio_add_request(pgio, new)) {
nfs_list_remove_request(new);
- error = desc->pgio.pg_error;
+ error = pgio->pg_error;
nfs_readpage_release(new, error);
goto out;
}
@@ -332,7 +330,8 @@ readpage_async_filler(struct nfs_readdesc *desc, struct page *page)
int nfs_read_folio(struct file *file, struct folio *folio)
{
struct page *page = &folio->page;
- struct nfs_readdesc desc;
+ struct nfs_pageio_descriptor pgio;
+ struct nfs_open_context *ctx;
struct inode *inode = page_file_mapping(page)->host;
int ret;
@@ -358,29 +357,29 @@ int nfs_read_folio(struct file *file, struct folio *folio)
if (file == NULL) {
ret = -EBADF;
- desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
- if (desc.ctx == NULL)
+ ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
+ if (ctx == NULL)
goto out_unlock;
} else
- desc.ctx = get_nfs_open_context(nfs_file_open_context(file));
+ ctx = get_nfs_open_context(nfs_file_open_context(file));
- xchg(&desc.ctx->error, 0);
- nfs_pageio_init_read(&desc.pgio, inode, false,
+ xchg(&ctx->error, 0);
+ nfs_pageio_init_read(&pgio, inode, false,
&nfs_async_read_completion_ops);
- ret = readpage_async_filler(&desc, page);
+ ret = nfs_read_add_page(&pgio, ctx, page);
if (ret)
goto out;
- nfs_pageio_complete_read(&desc.pgio);
- ret = desc.pgio.pg_error < 0 ? desc.pgio.pg_error : 0;
+ nfs_pageio_complete_read(&pgio);
+ ret = pgio.pg_error < 0 ? pgio.pg_error : 0;
if (!ret) {
ret = wait_on_page_locked_killable(page);
if (!PageUptodate(page) && !ret)
- ret = xchg(&desc.ctx->error, 0);
+ ret = xchg(&ctx->error, 0);
}
out:
- put_nfs_open_context(desc.ctx);
+ put_nfs_open_context(ctx);
trace_nfs_aop_readpage_done(inode, page, ret);
return ret;
out_unlock:
@@ -391,9 +390,10 @@ int nfs_read_folio(struct file *file, struct folio *folio)
void nfs_readahead(struct readahead_control *ractl)
{
+ struct nfs_pageio_descriptor pgio;
+ struct nfs_open_context *ctx;
unsigned int nr_pages = readahead_count(ractl);
struct file *file = ractl->file;
- struct nfs_readdesc desc;
struct inode *inode = ractl->mapping->host;
struct page *page;
int ret;
@@ -407,25 +407,25 @@ void nfs_readahead(struct readahead_control *ractl)
if (file == NULL) {
ret = -EBADF;
- desc.ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
- if (desc.ctx == NULL)
+ ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
+ if (ctx == NULL)
goto out;
} else
- desc.ctx = get_nfs_open_context(nfs_file_open_context(file));
+ ctx = get_nfs_open_context(nfs_file_open_context(file));
- nfs_pageio_init_read(&desc.pgio, inode, false,
+ nfs_pageio_init_read(&pgio, inode, false,
&nfs_async_read_completion_ops);
while ((page = readahead_page(ractl)) != NULL) {
- ret = readpage_async_filler(&desc, page);
+ ret = nfs_read_add_page(&pgio, ctx, page);
put_page(page);
if (ret)
break;
}
- nfs_pageio_complete_read(&desc.pgio);
+ nfs_pageio_complete_read(&pgio);
- put_nfs_open_context(desc.ctx);
+ put_nfs_open_context(ctx);
out:
trace_nfs_aop_readahead_done(inode, nr_pages, ret);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v10 2/6] NFS: Configure support for netfs when NFS fscache is configured
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 1/6] NFS: Rename readpage_async_filler to nfs_read_add_page Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 3/6] NFS: Convert buffered read paths to use netfs when fscache is enabled Dave Wysochanski
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
As first steps for support of the netfs library when NFS_FSCACHE is
configured, add NETFS_SUPPORT to Kconfig and add the required netfs_inode
into struct nfs_inode.
Using netfs requires we move the VFS inode structure to be stored
inside struct netfs_inode, along with the fscache_cookie.
Thus, create a new helper, VFS_I(), which is defined
differently depending on whether NFS_FSCACHE is configured.
In addition, use the netfs_inode() and netfs_i_cookie() helpers,
and remove our own helper, nfs_i_fscache().
Later patches will convert NFS fscache to fully use netfs.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
---
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 | 34 +++++++++++++++++++++++-----------
10 files changed, 52 insertions(+), 44 deletions(-)
diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 14a72224b657..8fbb6caf3481 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -171,6 +171,7 @@ config ROOT_NFS
config NFS_FSCACHE
bool "Provide NFS client caching support"
depends on NFS_FS=m && FSCACHE || NFS_FS=y && FSCACHE=y
+ select NETFS_SUPPORT
help
Say Y here if you want NFS data to be cached locally on disc through
the general filesystem cache manager
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 5c97cad741a7..b5c492d40367 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(VFS_I(nfsi));
out:
return ret;
}
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 58036f657126..36315ee2a248 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2799,7 +2799,7 @@ nfs_do_access_cache_scan(unsigned int nr_to_scan)
if (nr_to_scan-- == 0)
break;
- inode = &nfsi->vfs_inode;
+ inode = VFS_I(nfsi);
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 6b2cfa59a1a2..acc0f5899577 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1407,7 +1407,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 = VFS_I(nfsi);
if (!S_ISREG(inode->i_mode))
return false;
@@ -2245,7 +2245,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 VFS_I(nfsi);
}
EXPORT_SYMBOL_GPL(nfs_alloc_inode);
@@ -2269,7 +2269,7 @@ static void init_once(void *foo)
{
struct nfs_inode *nfsi = foo;
- inode_init_once(&nfsi->vfs_inode);
+ inode_init_once(VFS_I(nfsi));
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 d914d609b85b..13ea6d42d26f 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(VFS_I(nfsi), 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 a5db5158c634..d55648ae5101 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -766,19 +766,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(&VFS_I(nfsi)->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(&VFS_I(nfsi)->i_lock);
pnfs_free_lseg_list(&tmp_list);
- nfs_commit_inode(&nfsi->vfs_inode, 0);
+ nfs_commit_inode(VFS_I(nfsi), 0);
pnfs_put_layout_hdr(lo);
} else
- spin_unlock(&nfsi->vfs_inode.i_lock);
+ spin_unlock(&VFS_I(nfsi)->i_lock);
return lo;
}
@@ -793,9 +793,9 @@ static bool pnfs_layout_removed(struct nfs_inode *nfsi,
{
bool ret;
- spin_lock(&nfsi->vfs_inode.i_lock);
+ spin_lock(&VFS_I(nfsi)->i_lock);
ret = nfsi->layout != lo;
- spin_unlock(&nfsi->vfs_inode.i_lock);
+ spin_unlock(&VFS_I(nfsi)->i_lock);
return ret;
}
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index f41d24b54fd1..911569022036 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 = VFS_I(nfsi);
nfs_init_cinfo_from_inode(&cinfo, inode);
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 7931fa472561..a1c402e26abf 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -31,6 +31,10 @@
#include <linux/sunrpc/auth.h>
#include <linux/sunrpc/clnt.h>
+#ifdef CONFIG_NFS_FSCACHE
+#include <linux/netfs.h>
+#endif
+
#include <linux/nfs.h>
#include <linux/nfs2.h>
#include <linux/nfs3.h>
@@ -204,9 +208,11 @@ struct nfs_inode {
__u64 write_io;
__u64 read_io;
#ifdef CONFIG_NFS_FSCACHE
- struct fscache_cookie *fscache;
-#endif
+ struct netfs_inode netfs; /* netfs context and VFS inode */
+#else
struct inode vfs_inode;
+#endif
+
#ifdef CONFIG_NFS_V4_2
struct nfs4_xattr_cache *xattr_cache;
@@ -281,10 +287,25 @@ struct nfs4_copy_state {
#define NFS_INO_LAYOUTSTATS (11) /* layoutstats inflight */
#define NFS_INO_ODIRECT (12) /* I/O setting is O_DIRECT */
+#ifdef CONFIG_NFS_FSCACHE
+static inline struct inode *VFS_I(struct nfs_inode *nfsi)
+{
+ return &nfsi->netfs.inode;
+}
+static inline struct nfs_inode *NFS_I(const struct inode *inode)
+{
+ return container_of(inode, struct nfs_inode, netfs.inode);
+}
+#else
+static inline struct inode *VFS_I(struct nfs_inode *nfsi)
+{
+ return &nfsi->vfs_inode;
+}
static inline struct nfs_inode *NFS_I(const struct inode *inode)
{
return container_of(inode, struct nfs_inode, vfs_inode);
}
+#endif
static inline struct nfs_server *NFS_SB(const struct super_block *s)
{
@@ -328,15 +349,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v10 3/6] NFS: Convert buffered read paths to use netfs when fscache is enabled
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 1/6] NFS: Rename readpage_async_filler to nfs_read_add_page Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 2/6] NFS: Configure support for netfs when NFS fscache is configured Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 4/6] NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API Dave Wysochanski
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
Convert the NFS buffered read code paths to corresponding netfs APIs,
but only when fscache is configured and enabled.
The netfs API defines struct netfs_request_ops which must be filled
in by the network filesystem. For NFS, we only need to define 5 of
the functions, the main one being the issue_read() function.
The issue_read() function is called by the netfs layer when a read
cannot be fulfilled locally, and must be sent to the server (either
the cache is not active, or it is active but the data is not available).
Once the read from the server is complete, netfs requires a call to
netfs_subreq_terminated() which conveys either how many bytes were read
successfully, or an error. Note that issue_read() is called with a
structure, netfs_io_subrequest, which defines the IO requested, and
contains a start and a length (both in bytes), and assumes the underlying
netfs will return a either an error on the whole region, or the number
of bytes successfully read.
The NFS IO path is page based and the main APIs are the pgio APIs defined
in pagelist.c. For the pgio APIs, there is no way for the caller to
know how many RPCs will be sent and how the pages will be broken up
into underlying RPCs, each of which will have their own completion and
return code. In contrast, netfs is subrequest based, a single
subrequest may contain multiple pages, and a single subrequest is
initiated with issue_read() and terminated with netfs_subreq_terminated().
Thus, to utilze the netfs APIs, NFS needs some way to accommodate
the netfs API requirement on the single response to the whole
subrequest, while also minimizing disruptive changes to the NFS
pgio layer.
The approach taken with this patch is to allocate a small structure
for each nfs_netfs_issue_read() call, store the final error and number
of bytes successfully transferred in the structure, and update these values
as each RPC completes. The refcount on the structure is used as a marker
for the last RPC completion, is incremented in nfs_netfs_read_initiate(),
and decremented inside nfs_netfs_read_completion(), when a nfs_pgio_header
contains a valid pointer to the data. On the final put (which signals
the final outstanding RPC is complete) in nfs_netfs_read_completion(),
call netfs_subreq_terminated() with either the final error value (if
one or more READs complete with an error) or the number of bytes
successfully transferred (if all RPCs complete successfully). Note
that when all RPCs complete successfully, the number of bytes transferred
is capped to the length of the subrequest. Capping the transferred length
to the subrequest length prevents "Subreq overread" warnings from netfs.
This is due to the "aligned_len" in nfs_pageio_add_page(), and the
corner case where NFS requests a full page at the end of the file,
even when i_size reflects only a partial page (NFS overread).
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
fs/nfs/fscache.c | 232 +++++++++++++++++++++++----------------
fs/nfs/fscache.h | 122 ++++++++++++++------
fs/nfs/inode.c | 2 +
fs/nfs/internal.h | 9 ++
fs/nfs/pagelist.c | 4 +
fs/nfs/read.c | 52 +++++----
include/linux/nfs_page.h | 3 +
include/linux/nfs_xdr.h | 3 +
8 files changed, 273 insertions(+), 154 deletions(-)
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index a6fc1c8b6644..8dac02d05f70 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -15,6 +15,9 @@
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/iversion.h>
+#include <linux/xarray.h>
+#include <linux/fscache.h>
+#include <linux/netfs.h>
#include "internal.h"
#include "iostat.h"
@@ -184,7 +187,7 @@ void nfs_fscache_init_inode(struct inode *inode)
*/
void nfs_fscache_clear_inode(struct inode *inode)
{
- fscache_relinquish_cookie(netfs_i_cookie(&NFS_I(inode)->netfs), false);
+ fscache_relinquish_cookie(netfs_i_cookie(netfs_inode(inode)), false);
netfs_inode(inode)->cache = NULL;
}
@@ -210,7 +213,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 = netfs_i_cookie(&NFS_I(inode)->netfs);
+ struct fscache_cookie *cookie = netfs_i_cookie(netfs_inode(inode));
bool open_for_write = inode_is_open_for_write(inode);
if (!fscache_cookie_valid(cookie))
@@ -228,119 +231,160 @@ 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 = netfs_i_cookie(&NFS_I(inode)->netfs);
+ struct fscache_cookie *cookie = netfs_i_cookie(netfs_inode(inode));
loff_t i_size = i_size_read(inode);
nfs_fscache_update_auxdata(&auxdata, inode);
fscache_unuse_cookie(cookie, &auxdata, &i_size);
}
-/*
- * Fallback page reading interface.
- */
-static int fscache_fallback_read_page(struct inode *inode, struct page *page)
+int nfs_netfs_read_folio(struct file *file, struct folio *folio)
{
- struct netfs_cache_resources cres;
- struct fscache_cookie *cookie = netfs_i_cookie(&NFS_I(inode)->netfs);
- struct iov_iter iter;
- struct bio_vec bvec[1];
- int ret;
-
- memset(&cres, 0, sizeof(cres));
- bvec[0].bv_page = page;
- bvec[0].bv_offset = 0;
- bvec[0].bv_len = PAGE_SIZE;
- iov_iter_bvec(&iter, READ, bvec, ARRAY_SIZE(bvec), PAGE_SIZE);
-
- ret = fscache_begin_read_operation(&cres, cookie);
- if (ret < 0)
- return ret;
-
- ret = fscache_read(&cres, page_offset(page), &iter, NETFS_READ_HOLE_FAIL,
- NULL, NULL);
- fscache_end_operation(&cres);
- return ret;
+ if (!netfs_inode(folio_inode(folio))->cache)
+ return -ENOBUFS;
+
+ return netfs_read_folio(file, folio);
}
-/*
- * Fallback page writing interface.
- */
-static int fscache_fallback_write_page(struct inode *inode, struct page *page,
- bool no_space_allocated_yet)
+int nfs_netfs_readahead(struct readahead_control *ractl)
{
- struct netfs_cache_resources cres;
- 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);
- size_t len = PAGE_SIZE;
- int ret;
-
- memset(&cres, 0, sizeof(cres));
- bvec[0].bv_page = page;
- bvec[0].bv_offset = 0;
- bvec[0].bv_len = PAGE_SIZE;
- iov_iter_bvec(&iter, WRITE, bvec, ARRAY_SIZE(bvec), PAGE_SIZE);
-
- ret = fscache_begin_write_operation(&cres, cookie);
- if (ret < 0)
- return ret;
-
- ret = cres.ops->prepare_write(&cres, &start, &len, i_size_read(inode),
- no_space_allocated_yet);
- if (ret == 0)
- ret = fscache_write(&cres, page_offset(page), &iter, NULL, NULL);
- fscache_end_operation(&cres);
- return ret;
+ struct inode *inode = ractl->mapping->host;
+
+ if (!netfs_inode(inode)->cache)
+ return -ENOBUFS;
+
+ netfs_readahead(ractl);
+ return 0;
}
-/*
- * Retrieve a page from fscache
- */
-int __nfs_fscache_read_page(struct inode *inode, struct page *page)
+atomic_t nfs_netfs_debug_id;
+static int nfs_netfs_init_request(struct netfs_io_request *rreq, struct file *file)
{
- int ret;
+ rreq->netfs_priv = get_nfs_open_context(nfs_file_open_context(file));
+ rreq->debug_id = atomic_inc_return(&nfs_netfs_debug_id);
- trace_nfs_fscache_read_page(inode, page);
- if (PageChecked(page)) {
- ClearPageChecked(page);
- ret = 1;
- goto out;
- }
+ return 0;
+}
- ret = fscache_fallback_read_page(inode, page);
- if (ret < 0) {
- nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
- SetPageChecked(page);
- goto out;
- }
+static void nfs_netfs_free_request(struct netfs_io_request *rreq)
+{
+ put_nfs_open_context(rreq->netfs_priv);
+}
- /* Read completed synchronously */
- nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
- SetPageUptodate(page);
- ret = 0;
-out:
- trace_nfs_fscache_read_page_exit(inode, page, ret);
- return ret;
+static inline int nfs_netfs_begin_cache_operation(struct netfs_io_request *rreq)
+{
+ return fscache_begin_read_operation(&rreq->cache_resources,
+ netfs_i_cookie(netfs_inode(rreq->inode)));
}
-/*
- * Store a newly fetched page in fscache. We can be certain there's no page
- * stored in the cache as yet otherwise we would've read it from there.
- */
-void __nfs_fscache_write_page(struct inode *inode, struct page *page)
+static struct nfs_netfs_io_data *nfs_netfs_alloc(struct netfs_io_subrequest *sreq)
{
- int ret;
+ struct nfs_netfs_io_data *netfs;
+
+ netfs = kzalloc(sizeof(*netfs), GFP_KERNEL_ACCOUNT);
+ if (!netfs)
+ return NULL;
+ netfs->sreq = sreq;
+ refcount_set(&netfs->refcount, 1);
+ return netfs;
+}
- trace_nfs_fscache_write_page(inode, page);
+static bool nfs_netfs_clamp_length(struct netfs_io_subrequest *sreq)
+{
+ size_t rsize = NFS_SB(sreq->rreq->inode->i_sb)->rsize;
- ret = fscache_fallback_write_page(inode, page, true);
+ sreq->len = min(sreq->len, rsize);
+ return true;
+}
- if (ret != 0) {
- nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_WRITTEN_FAIL);
- nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_UNCACHED);
- } else {
- nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_WRITTEN_OK);
+static void nfs_netfs_issue_read(struct netfs_io_subrequest *sreq)
+{
+ struct nfs_netfs_io_data *netfs;
+ struct nfs_pageio_descriptor pgio;
+ struct inode *inode = sreq->rreq->inode;
+ struct nfs_open_context *ctx = sreq->rreq->netfs_priv;
+ struct page *page;
+ int err;
+ pgoff_t start = (sreq->start + sreq->transferred) >> PAGE_SHIFT;
+ pgoff_t last = ((sreq->start + sreq->len -
+ sreq->transferred - 1) >> PAGE_SHIFT);
+ XA_STATE(xas, &sreq->rreq->mapping->i_pages, start);
+
+ nfs_pageio_init_read(&pgio, inode, false,
+ &nfs_async_read_completion_ops);
+
+ netfs = nfs_netfs_alloc(sreq);
+ if (!netfs)
+ return netfs_subreq_terminated(sreq, -ENOMEM, false);
+
+ pgio.pg_netfs = netfs; /* used in completion */
+
+ xas_lock(&xas);
+ xas_for_each(&xas, page, last) {
+ /* nfs_read_add_page() may schedule() due to pNFS layout and other RPCs */
+ xas_pause(&xas);
+ xas_unlock(&xas);
+ err = nfs_read_add_page(&pgio, ctx, page);
+ if (err < 0) {
+ netfs->error = err;
+ goto out;
+ }
+ xas_lock(&xas);
}
- trace_nfs_fscache_write_page_exit(inode, page, ret);
+ xas_unlock(&xas);
+out:
+ nfs_pageio_complete_read(&pgio);
+ nfs_netfs_put(netfs);
+}
+
+void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr)
+{
+ struct nfs_netfs_io_data *netfs = hdr->netfs;
+
+ if (!netfs)
+ return;
+
+ nfs_netfs_get(netfs);
+}
+
+void nfs_netfs_readpage_release(struct nfs_page *req)
+{
+ struct inode *inode = d_inode(nfs_req_openctx(req)->dentry);
+
+ /*
+ * If fscache is enabled, netfs will unlock pages.
+ */
+ if (netfs_inode(inode)->cache)
+ return;
+
+ unlock_page(req->wb_page);
}
+
+void nfs_netfs_read_completion(struct nfs_pgio_header *hdr)
+{
+ struct nfs_netfs_io_data *netfs = hdr->netfs;
+ struct netfs_io_subrequest *sreq;
+
+ if (!netfs)
+ return;
+
+ sreq = netfs->sreq;
+ if (test_bit(NFS_IOHDR_EOF, &hdr->flags))
+ __set_bit(NETFS_SREQ_CLEAR_TAIL, &sreq->flags);
+
+ if (hdr->error)
+ netfs->error = hdr->error;
+ else
+ atomic64_add(hdr->res.count, &netfs->transferred);
+
+ nfs_netfs_put(netfs);
+ hdr->netfs = NULL;
+}
+
+const struct netfs_request_ops nfs_netfs_ops = {
+ .init_request = nfs_netfs_init_request,
+ .free_request = nfs_netfs_free_request,
+ .begin_cache_operation = nfs_netfs_begin_cache_operation,
+ .issue_read = nfs_netfs_issue_read,
+ .clamp_length = nfs_netfs_clamp_length
+};
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 38614ed8f951..18da26157fda 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -34,6 +34,58 @@ struct nfs_fscache_inode_auxdata {
u64 change_attr;
};
+struct nfs_netfs_io_data {
+ /*
+ * NFS may split a netfs_io_subrequest into multiple RPCs, each
+ * with their own read completion. In netfs, we can only call
+ * netfs_subreq_terminated() once for each subrequest. Use the
+ * refcount here to double as a marker of the last RPC completion,
+ * and only call netfs via netfs_subreq_terminated() once.
+ */
+ refcount_t refcount;
+ struct netfs_io_subrequest *sreq;
+
+ /*
+ * Final disposition of the netfs_io_subrequest, sent in
+ * netfs_subreq_terminated()
+ */
+ atomic64_t transferred;
+ int error;
+};
+
+static inline void nfs_netfs_get(struct nfs_netfs_io_data *netfs)
+{
+ refcount_inc(&netfs->refcount);
+}
+
+static inline void nfs_netfs_put(struct nfs_netfs_io_data *netfs)
+{
+ ssize_t final_len;
+
+ /* Only the last RPC completion should call netfs_subreq_terminated() */
+ if (!refcount_dec_and_test(&netfs->refcount))
+ return;
+
+ /*
+ * The NFS pageio interface may read a complete page, even when netfs
+ * only asked for a partial page. Specifically, this may be seen when
+ * one thread is truncating a file while another one is reading the last
+ * page of the file.
+ * Correct the final length here to be no larger than the netfs subrequest
+ * length, and prevent netfs from complain throwing "Subreq overread".
+ */
+ final_len = min_t(s64, netfs->sreq->len, atomic64_read(&netfs->transferred));
+ netfs_subreq_terminated(netfs->sreq, netfs->error ?: final_len, false);
+ kfree(netfs);
+}
+static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi)
+{
+ netfs_inode_init(&nfsi->netfs, &nfs_netfs_ops);
+}
+extern void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr);
+extern void nfs_netfs_read_completion(struct nfs_pgio_header *hdr);
+extern void nfs_netfs_readpage_release(struct nfs_page *req);
+
/*
* fscache.c
*/
@@ -44,9 +96,8 @@ extern void nfs_fscache_init_inode(struct inode *);
extern void nfs_fscache_clear_inode(struct inode *);
extern void nfs_fscache_open_file(struct inode *, struct file *);
extern void nfs_fscache_release_file(struct inode *, struct file *);
-
-extern int __nfs_fscache_read_page(struct inode *, struct page *);
-extern void __nfs_fscache_write_page(struct inode *, struct page *);
+extern int nfs_netfs_readahead(struct readahead_control *ractl);
+extern int nfs_netfs_read_folio(struct file *file, struct folio *folio);
static inline bool nfs_fscache_release_folio(struct folio *folio, gfp_t gfp)
{
@@ -54,34 +105,11 @@ 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(netfs_i_cookie(&NFS_I(folio->mapping->host)->netfs));
- nfs_inc_fscache_stats(folio->mapping->host,
- NFSIOS_FSCACHE_PAGES_UNCACHED);
}
+ fscache_note_page_release(netfs_i_cookie(&NFS_I(folio->mapping->host)->netfs));
return true;
}
-/*
- * Retrieve a page from an inode data storage object.
- */
-static inline int nfs_fscache_read_page(struct inode *inode, struct page *page)
-{
- if (netfs_inode(inode)->cache)
- return __nfs_fscache_read_page(inode, page);
- return -ENOBUFS;
-}
-
-/*
- * Store a page newly fetched from the server in an inode data storage object
- * in the cache.
- */
-static inline void nfs_fscache_write_page(struct inode *inode,
- struct page *page)
-{
- if (netfs_inode(inode)->cache)
- __nfs_fscache_write_page(inode, page);
-}
-
static inline void nfs_fscache_update_auxdata(struct nfs_fscache_inode_auxdata *auxdata,
struct inode *inode)
{
@@ -117,7 +145,28 @@ static inline const char *nfs_server_fscache_state(struct nfs_server *server)
return "no ";
}
+static inline void nfs_netfs_set_pgio_header(struct nfs_pgio_header *hdr,
+ struct nfs_pageio_descriptor *desc)
+{
+ hdr->netfs = desc->pg_netfs;
+}
+static inline void nfs_netfs_set_pageio_descriptor(struct nfs_pageio_descriptor *desc,
+ struct nfs_pgio_header *hdr)
+{
+ desc->pg_netfs = hdr->netfs;
+}
+static inline void nfs_netfs_reset_pageio_descriptor(struct nfs_pageio_descriptor *desc)
+{
+ desc->pg_netfs = NULL;
+}
#else /* CONFIG_NFS_FSCACHE */
+static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi) {}
+static inline void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr) {}
+static inline void nfs_netfs_read_completion(struct nfs_pgio_header *hdr) {}
+static inline void nfs_netfs_readpage_release(struct nfs_page *req)
+{
+ unlock_page(req->wb_page);
+}
static inline void nfs_fscache_release_super_cookie(struct super_block *sb) {}
static inline void nfs_fscache_init_inode(struct inode *inode) {}
@@ -125,22 +174,29 @@ static inline void nfs_fscache_clear_inode(struct inode *inode) {}
static inline void nfs_fscache_open_file(struct inode *inode,
struct file *filp) {}
static inline void nfs_fscache_release_file(struct inode *inode, struct file *file) {}
-
-static inline bool nfs_fscache_release_folio(struct folio *folio, gfp_t gfp)
+static inline int nfs_netfs_readahead(struct readahead_control *ractl)
{
- return true; /* may release folio */
+ return -ENOBUFS;
}
-static inline int nfs_fscache_read_page(struct inode *inode, struct page *page)
+static inline int nfs_netfs_read_folio(struct file *file, struct folio *folio)
{
return -ENOBUFS;
}
-static inline void nfs_fscache_write_page(struct inode *inode, struct page *page) {}
+
+static inline bool nfs_fscache_release_folio(struct folio *folio, gfp_t gfp)
+{
+ return true; /* may release folio */
+}
static inline void nfs_fscache_invalidate(struct inode *inode, int flags) {}
static inline const char *nfs_server_fscache_state(struct nfs_server *server)
{
return "no ";
}
-
+static inline void nfs_netfs_set_pgio_header(struct nfs_pgio_header *hdr,
+ struct nfs_pageio_descriptor *desc) {}
+static inline void nfs_netfs_set_pageio_descriptor(struct nfs_pageio_descriptor *desc,
+ struct nfs_pgio_header *hdr) {}
+static inline void nfs_netfs_reset_pageio_descriptor(struct nfs_pageio_descriptor *desc) {}
#endif /* CONFIG_NFS_FSCACHE */
#endif /* _NFS_FSCACHE_H */
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index acc0f5899577..116ae689a490 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2245,6 +2245,8 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
#ifdef CONFIG_NFS_V4_2
nfsi->xattr_cache = NULL;
#endif
+ nfs_netfs_inode_init(nfsi);
+
return VFS_I(nfsi);
}
EXPORT_SYMBOL_GPL(nfs_alloc_inode);
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 13ea6d42d26f..6dd6d114406b 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -452,6 +452,10 @@ extern void nfs_sb_deactive(struct super_block *sb);
extern int nfs_client_for_each_server(struct nfs_client *clp,
int (*fn)(struct nfs_server *, void *),
void *data);
+#ifdef CONFIG_NFS_FSCACHE
+extern const struct netfs_request_ops nfs_netfs_ops;
+#endif
+
/* io.c */
extern void nfs_start_io_read(struct inode *inode);
extern void nfs_end_io_read(struct inode *inode);
@@ -481,9 +485,14 @@ extern int nfs4_get_rootfh(struct nfs_server *server, struct nfs_fh *mntfh, bool
struct nfs_pgio_completion_ops;
/* read.c */
+extern const struct nfs_pgio_completion_ops nfs_async_read_completion_ops;
extern void nfs_pageio_init_read(struct nfs_pageio_descriptor *pgio,
struct inode *inode, bool force_mds,
const struct nfs_pgio_completion_ops *compl_ops);
+extern int nfs_read_add_page(struct nfs_pageio_descriptor *pgio,
+ struct nfs_open_context *ctx,
+ struct page *page);
+extern void nfs_pageio_complete_read(struct nfs_pageio_descriptor *pgio);
extern void nfs_read_prepare(struct rpc_task *task, void *calldata);
extern void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio);
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 317cedfa52bf..870a7a8f299e 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -25,6 +25,7 @@
#include "internal.h"
#include "pnfs.h"
#include "nfstrace.h"
+#include "fscache.h"
#define NFSDBG_FACILITY NFSDBG_PAGECACHE
@@ -68,6 +69,7 @@ void nfs_pgheader_init(struct nfs_pageio_descriptor *desc,
hdr->good_bytes = mirror->pg_count;
hdr->io_completion = desc->pg_io_completion;
hdr->dreq = desc->pg_dreq;
+ nfs_netfs_set_pgio_header(hdr, desc);
hdr->release = release;
hdr->completion_ops = desc->pg_completion_ops;
if (hdr->completion_ops->init_hdr)
@@ -846,6 +848,7 @@ void nfs_pageio_init(struct nfs_pageio_descriptor *desc,
desc->pg_lseg = NULL;
desc->pg_io_completion = NULL;
desc->pg_dreq = NULL;
+ nfs_netfs_reset_pageio_descriptor(desc);
desc->pg_bsize = bsize;
desc->pg_mirror_count = 1;
@@ -1360,6 +1363,7 @@ int nfs_pageio_resend(struct nfs_pageio_descriptor *desc,
desc->pg_io_completion = hdr->io_completion;
desc->pg_dreq = hdr->dreq;
+ nfs_netfs_set_pageio_descriptor(desc, hdr);
list_splice_init(&hdr->pages, &pages);
while (!list_empty(&pages)) {
struct nfs_page *req = nfs_list_entry(pages.next);
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 71267a3174f1..bede5dea0671 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -30,7 +30,7 @@
#define NFSDBG_FACILITY NFSDBG_PAGECACHE
-static const struct nfs_pgio_completion_ops nfs_async_read_completion_ops;
+const struct nfs_pgio_completion_ops nfs_async_read_completion_ops;
static const struct nfs_rw_ops nfs_rw_read_ops;
static struct kmem_cache *nfs_rdata_cachep;
@@ -74,7 +74,7 @@ void nfs_pageio_init_read(struct nfs_pageio_descriptor *pgio,
}
EXPORT_SYMBOL_GPL(nfs_pageio_init_read);
-static void nfs_pageio_complete_read(struct nfs_pageio_descriptor *pgio)
+void nfs_pageio_complete_read(struct nfs_pageio_descriptor *pgio)
{
struct nfs_pgio_mirror *pgm;
unsigned long npages;
@@ -110,20 +110,13 @@ EXPORT_SYMBOL_GPL(nfs_pageio_reset_read_mds);
static void nfs_readpage_release(struct nfs_page *req, int error)
{
- struct inode *inode = d_inode(nfs_req_openctx(req)->dentry);
struct page *page = req->wb_page;
- dprintk("NFS: read done (%s/%llu %d@%lld)\n", inode->i_sb->s_id,
- (unsigned long long)NFS_FILEID(inode), req->wb_bytes,
- (long long)req_offset(req));
-
if (nfs_error_is_fatal_on_server(error) && error != -ETIMEDOUT)
SetPageError(page);
- if (nfs_page_group_sync_on_bit(req, PG_UNLOCKPAGE)) {
- if (PageUptodate(page))
- nfs_fscache_write_page(inode, page);
- unlock_page(page);
- }
+ if (nfs_page_group_sync_on_bit(req, PG_UNLOCKPAGE))
+ nfs_netfs_readpage_release(req);
+
nfs_release_request(req);
}
@@ -177,6 +170,8 @@ static void nfs_read_completion(struct nfs_pgio_header *hdr)
nfs_list_remove_request(req);
nfs_readpage_release(req, error);
}
+ nfs_netfs_read_completion(hdr);
+
out:
hdr->release(hdr);
}
@@ -187,6 +182,7 @@ static void nfs_initiate_read(struct nfs_pgio_header *hdr,
struct rpc_task_setup *task_setup_data, int how)
{
rpc_ops->read_setup(hdr, msg);
+ nfs_netfs_initiate_read(hdr);
trace_nfs_initiate_read(hdr);
}
@@ -202,7 +198,7 @@ nfs_async_read_error(struct list_head *head, int error)
}
}
-static const struct nfs_pgio_completion_ops nfs_async_read_completion_ops = {
+const struct nfs_pgio_completion_ops nfs_async_read_completion_ops = {
.error_cleanup = nfs_async_read_error,
.completion = nfs_read_completion,
};
@@ -277,7 +273,7 @@ static void nfs_readpage_result(struct rpc_task *task,
nfs_readpage_retry(task, hdr);
}
-static int
+int
nfs_read_add_page(struct nfs_pageio_descriptor *pgio,
struct nfs_open_context *ctx,
struct page *page)
@@ -294,12 +290,6 @@ nfs_read_add_page(struct nfs_pageio_descriptor *pgio,
aligned_len = min_t(unsigned int, ALIGN(len, rsize), PAGE_SIZE);
- if (!IS_SYNC(page->mapping->host)) {
- error = nfs_fscache_read_page(page->mapping->host, page);
- if (error == 0)
- goto out_unlock;
- }
-
new = nfs_create_request(ctx, page, 0, aligned_len);
if (IS_ERR(new))
goto out_error;
@@ -315,8 +305,6 @@ nfs_read_add_page(struct nfs_pageio_descriptor *pgio,
return 0;
out_error:
error = PTR_ERR(new);
-out_unlock:
- unlock_page(page);
out:
return error;
}
@@ -355,6 +343,10 @@ int nfs_read_folio(struct file *file, struct folio *folio)
if (NFS_STALE(inode))
goto out_unlock;
+ ret = nfs_netfs_read_folio(file, folio);
+ if (!ret)
+ goto out;
+
if (file == NULL) {
ret = -EBADF;
ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
@@ -368,8 +360,10 @@ int nfs_read_folio(struct file *file, struct folio *folio)
&nfs_async_read_completion_ops);
ret = nfs_read_add_page(&pgio, ctx, page);
- if (ret)
- goto out;
+ if (ret) {
+ put_nfs_open_context(ctx);
+ goto out_unlock;
+ }
nfs_pageio_complete_read(&pgio);
ret = pgio.pg_error < 0 ? pgio.pg_error : 0;
@@ -378,12 +372,12 @@ int nfs_read_folio(struct file *file, struct folio *folio)
if (!PageUptodate(page) && !ret)
ret = xchg(&ctx->error, 0);
}
-out:
put_nfs_open_context(ctx);
- trace_nfs_aop_readpage_done(inode, page, ret);
- return ret;
+ goto out;
+
out_unlock:
unlock_page(page);
+out:
trace_nfs_aop_readpage_done(inode, page, ret);
return ret;
}
@@ -405,6 +399,10 @@ void nfs_readahead(struct readahead_control *ractl)
if (NFS_STALE(inode))
goto out;
+ ret = nfs_netfs_readahead(ractl);
+ if (!ret)
+ goto out;
+
if (file == NULL) {
ret = -EBADF;
ctx = nfs_find_open_context(inode, NULL, FMODE_READ);
diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
index ba7e2e4b0926..8eeb16d9bacd 100644
--- a/include/linux/nfs_page.h
+++ b/include/linux/nfs_page.h
@@ -101,6 +101,9 @@ struct nfs_pageio_descriptor {
struct pnfs_layout_segment *pg_lseg;
struct nfs_io_completion *pg_io_completion;
struct nfs_direct_req *pg_dreq;
+#ifdef CONFIG_NFS_FSCACHE
+ void *pg_netfs;
+#endif
unsigned int pg_bsize; /* default bsize for mirrors */
u32 pg_mirror_count;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index e86cf6642d21..e196ef595908 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1619,6 +1619,9 @@ struct nfs_pgio_header {
const struct nfs_rw_ops *rw_ops;
struct nfs_io_completion *io_completion;
struct nfs_direct_req *dreq;
+#ifdef CONFIG_NFS_FSCACHE
+ void *netfs;
+#endif
int pnfs_error;
int error; /* merge with pnfs_error */
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v10 4/6] NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
` (2 preceding siblings ...)
2022-11-03 16:16 ` [PATCH v10 3/6] NFS: Convert buffered read paths to use netfs when fscache is enabled Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 5/6] NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit Dave Wysochanski
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
The old NFSIOS_FSCACHE counters are no longer accurate or useful with
the conversion to the new netfs API. The new API does not have a page
based interface, and so the counters in nfs_stat_fscachecounters are
no longer obtainable. The new netfs the API has extensive statistics
inside /proc/fs/fscache/stats so we no longer need NFS specific fscache
stats.
Note this also removes the 'fsc:' line from /proc/self/mountstats so
it will be a user-visible change.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfs/iostat.h | 17 -----------------
fs/nfs/super.c | 11 -----------
include/linux/nfs_iostat.h | 12 ------------
3 files changed, 40 deletions(-)
diff --git a/fs/nfs/iostat.h b/fs/nfs/iostat.h
index 2ddaab1ac653..5aa776b5a3e7 100644
--- a/fs/nfs/iostat.h
+++ b/fs/nfs/iostat.h
@@ -17,9 +17,6 @@
struct nfs_iostats {
unsigned long long bytes[__NFSIOS_BYTESMAX];
-#ifdef CONFIG_NFS_FSCACHE
- unsigned long long fscache[__NFSIOS_FSCACHEMAX];
-#endif
unsigned long events[__NFSIOS_COUNTSMAX];
} ____cacheline_aligned;
@@ -49,20 +46,6 @@ static inline void nfs_add_stats(const struct inode *inode,
nfs_add_server_stats(NFS_SERVER(inode), stat, addend);
}
-#ifdef CONFIG_NFS_FSCACHE
-static inline void nfs_add_fscache_stats(struct inode *inode,
- enum nfs_stat_fscachecounters stat,
- long addend)
-{
- this_cpu_add(NFS_SERVER(inode)->io_stats->fscache[stat], addend);
-}
-static inline void nfs_inc_fscache_stats(struct inode *inode,
- enum nfs_stat_fscachecounters stat)
-{
- this_cpu_inc(NFS_SERVER(inode)->io_stats->fscache[stat]);
-}
-#endif
-
static inline struct nfs_iostats __percpu *nfs_alloc_iostats(void)
{
return alloc_percpu(struct nfs_iostats);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index ee66ffdb985e..302148258ff1 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -692,10 +692,6 @@ int nfs_show_stats(struct seq_file *m, struct dentry *root)
totals.events[i] += stats->events[i];
for (i = 0; i < __NFSIOS_BYTESMAX; i++)
totals.bytes[i] += stats->bytes[i];
-#ifdef CONFIG_NFS_FSCACHE
- for (i = 0; i < __NFSIOS_FSCACHEMAX; i++)
- totals.fscache[i] += stats->fscache[i];
-#endif
preempt_enable();
}
@@ -706,13 +702,6 @@ int nfs_show_stats(struct seq_file *m, struct dentry *root)
seq_puts(m, "\n\tbytes:\t");
for (i = 0; i < __NFSIOS_BYTESMAX; i++)
seq_printf(m, "%Lu ", totals.bytes[i]);
-#ifdef CONFIG_NFS_FSCACHE
- if (nfss->options & NFS_OPTION_FSCACHE) {
- seq_puts(m, "\n\tfsc:\t");
- for (i = 0; i < __NFSIOS_FSCACHEMAX; i++)
- seq_printf(m, "%Lu ", totals.fscache[i]);
- }
-#endif
seq_putc(m, '\n');
rpc_clnt_show_stats(m, nfss->client);
diff --git a/include/linux/nfs_iostat.h b/include/linux/nfs_iostat.h
index 027874c36c88..8d946089d151 100644
--- a/include/linux/nfs_iostat.h
+++ b/include/linux/nfs_iostat.h
@@ -119,16 +119,4 @@ enum nfs_stat_eventcounters {
__NFSIOS_COUNTSMAX,
};
-/*
- * NFS local caching servicing counters
- */
-enum nfs_stat_fscachecounters {
- NFSIOS_FSCACHE_PAGES_READ_OK,
- NFSIOS_FSCACHE_PAGES_READ_FAIL,
- NFSIOS_FSCACHE_PAGES_WRITTEN_OK,
- NFSIOS_FSCACHE_PAGES_WRITTEN_FAIL,
- NFSIOS_FSCACHE_PAGES_UNCACHED,
- __NFSIOS_FSCACHEMAX,
-};
-
#endif /* _LINUX_NFS_IOSTAT */
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v10 5/6] NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
` (3 preceding siblings ...)
2022-11-03 16:16 ` [PATCH v10 4/6] NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:16 ` [RFC PATCH v10 6/6] netfs: Change netfs_inode_init to allocate memory to allow opt-in Dave Wysochanski
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
The NFS specific trace points are no longer needed as tracing is well
covered by netfs and fscache.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfs/nfstrace.h | 91 ------------------------------------------
include/linux/nfs_fs.h | 1 -
2 files changed, 92 deletions(-)
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 8c6cc58679ff..6b56abe49ec2 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -39,7 +39,6 @@
{ BIT(NFS_INO_STALE), "STALE" }, \
{ BIT(NFS_INO_ACL_LRU_SET), "ACL_LRU_SET" }, \
{ BIT(NFS_INO_INVALIDATING), "INVALIDATING" }, \
- { BIT(NFS_INO_FSCACHE), "FSCACHE" }, \
{ BIT(NFS_INO_LAYOUTCOMMIT), "NEED_LAYOUTCOMMIT" }, \
{ BIT(NFS_INO_LAYOUTCOMMITTING), "LAYOUTCOMMIT" }, \
{ BIT(NFS_INO_LAYOUTSTATS), "LAYOUTSTATS" }, \
@@ -1213,96 +1212,6 @@ TRACE_EVENT(nfs_readpage_short,
)
);
-DECLARE_EVENT_CLASS(nfs_fscache_page_event,
- TP_PROTO(
- const struct inode *inode,
- struct page *page
- ),
-
- TP_ARGS(inode, page),
-
- TP_STRUCT__entry(
- __field(dev_t, dev)
- __field(u32, fhandle)
- __field(u64, fileid)
- __field(loff_t, offset)
- ),
-
- TP_fast_assign(
- const struct nfs_inode *nfsi = NFS_I(inode);
- const struct nfs_fh *fh = &nfsi->fh;
-
- __entry->offset = page_index(page) << PAGE_SHIFT;
- __entry->dev = inode->i_sb->s_dev;
- __entry->fileid = nfsi->fileid;
- __entry->fhandle = nfs_fhandle_hash(fh);
- ),
-
- TP_printk(
- "fileid=%02x:%02x:%llu fhandle=0x%08x "
- "offset=%lld",
- MAJOR(__entry->dev), MINOR(__entry->dev),
- (unsigned long long)__entry->fileid,
- __entry->fhandle,
- (long long)__entry->offset
- )
-);
-DECLARE_EVENT_CLASS(nfs_fscache_page_event_done,
- TP_PROTO(
- const struct inode *inode,
- struct page *page,
- int error
- ),
-
- TP_ARGS(inode, page, error),
-
- TP_STRUCT__entry(
- __field(int, error)
- __field(dev_t, dev)
- __field(u32, fhandle)
- __field(u64, fileid)
- __field(loff_t, offset)
- ),
-
- TP_fast_assign(
- const struct nfs_inode *nfsi = NFS_I(inode);
- const struct nfs_fh *fh = &nfsi->fh;
-
- __entry->offset = page_index(page) << PAGE_SHIFT;
- __entry->dev = inode->i_sb->s_dev;
- __entry->fileid = nfsi->fileid;
- __entry->fhandle = nfs_fhandle_hash(fh);
- __entry->error = error;
- ),
-
- TP_printk(
- "fileid=%02x:%02x:%llu fhandle=0x%08x "
- "offset=%lld error=%d",
- MAJOR(__entry->dev), MINOR(__entry->dev),
- (unsigned long long)__entry->fileid,
- __entry->fhandle,
- (long long)__entry->offset, __entry->error
- )
-);
-#define DEFINE_NFS_FSCACHE_PAGE_EVENT(name) \
- DEFINE_EVENT(nfs_fscache_page_event, name, \
- TP_PROTO( \
- const struct inode *inode, \
- struct page *page \
- ), \
- TP_ARGS(inode, page))
-#define DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(name) \
- DEFINE_EVENT(nfs_fscache_page_event_done, name, \
- TP_PROTO( \
- const struct inode *inode, \
- struct page *page, \
- int error \
- ), \
- TP_ARGS(inode, page, error))
-DEFINE_NFS_FSCACHE_PAGE_EVENT(nfs_fscache_read_page);
-DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(nfs_fscache_read_page_exit);
-DEFINE_NFS_FSCACHE_PAGE_EVENT(nfs_fscache_write_page);
-DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(nfs_fscache_write_page_exit);
TRACE_EVENT(nfs_pgio_error,
TP_PROTO(
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index a1c402e26abf..0150a5673419 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -281,7 +281,6 @@ struct nfs4_copy_state {
#define NFS_INO_ACL_LRU_SET (2) /* Inode is on the LRU list */
#define NFS_INO_INVALIDATING (3) /* inode is being invalidated */
#define NFS_INO_PRESERVE_UNLINKED (4) /* preserve file if removed while open */
-#define NFS_INO_FSCACHE (5) /* inode can be cached by FS-Cache */
#define NFS_INO_LAYOUTCOMMIT (9) /* layoutcommit required */
#define NFS_INO_LAYOUTCOMMITTING (10) /* layoutcommit inflight */
#define NFS_INO_LAYOUTSTATS (11) /* layoutstats inflight */
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [RFC PATCH v10 6/6] netfs: Change netfs_inode_init to allocate memory to allow opt-in
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
` (4 preceding siblings ...)
2022-11-03 16:16 ` [PATCH v10 5/6] NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit Dave Wysochanski
@ 2022-11-03 16:16 ` Dave Wysochanski
2022-11-03 16:38 ` David Howells
2023-02-09 14:57 ` [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API David Wysochanski
7 siblings, 0 replies; 11+ messages in thread
From: Dave Wysochanski @ 2022-11-03 16:16 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard, Daire Byrne
Reduce the memory footprint for non-netfs enabled mounts by changing
the netfs_inode structure to contain a "struct inode" and a pointer
to the rest of the netfs data, which is now inside a new structure
"netfs_info".
This keeps the memory footprint equal to the previous footprint
inside a network filesystems inode structure when fscache was
enabled (a struct inode, plus a pointer).
This saves memory for network filesystems inode that would build
in netfs support, but like to opt-in to netfs on some mounts while
opting-out of netfs on other mounts.
FIXME: call netfs_inode_free() as needed (afs, ceph, etc)
FIXME: Check setting of remote_i_size
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
---
fs/9p/cache.c | 2 +-
fs/9p/vfs_inode.c | 17 +++++++----------
fs/afs/dynroot.c | 7 ++++++-
fs/afs/inode.c | 14 ++------------
fs/afs/internal.h | 2 +-
fs/afs/super.c | 7 +++++++
fs/afs/write.c | 2 +-
fs/ceph/inode.c | 6 +++++-
fs/netfs/buffered_read.c | 16 ++++++++--------
fs/netfs/internal.h | 2 +-
fs/netfs/objects.c | 2 +-
fs/nfs/fscache.c | 12 ++++++------
fs/nfs/fscache.h | 11 ++++++++---
fs/nfs/inode.c | 9 ++++++++-
include/linux/netfs.h | 41 +++++++++++++++++++++++++++++-----------
15 files changed, 92 insertions(+), 58 deletions(-)
diff --git a/fs/9p/cache.c b/fs/9p/cache.c
index cebba4eaa0b5..9940ab8383c9 100644
--- a/fs/9p/cache.c
+++ b/fs/9p/cache.c
@@ -62,7 +62,7 @@ void v9fs_cache_inode_get_cookie(struct inode *inode)
version = cpu_to_le32(v9inode->qid.version);
path = cpu_to_le64(v9inode->qid.path);
v9ses = v9fs_inode2v9ses(inode);
- v9inode->netfs.cache =
+ v9inode->netfs.info->cache =
fscache_acquire_cookie(v9fs_session_cache(v9ses),
0,
&path, sizeof(path),
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 4d1a4a8d9277..4219882d58eb 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -227,10 +227,16 @@ v9fs_blank_wstat(struct p9_wstat *wstat)
struct inode *v9fs_alloc_inode(struct super_block *sb)
{
struct v9fs_inode *v9inode;
+ int ret;
v9inode = alloc_inode_sb(sb, v9fs_inode_cache, GFP_KERNEL);
if (!v9inode)
return NULL;
+ ret = netfs_inode_init(&v9inode->netfs, &v9fs_req_ops);
+ if (ret) {
+ kmem_cache_free(v9fs_inode_cache, v9inode);
+ return NULL;
+ }
v9inode->writeback_fid = NULL;
v9inode->cache_validity = 0;
mutex_init(&v9inode->v_mutex);
@@ -244,18 +250,10 @@ struct inode *v9fs_alloc_inode(struct super_block *sb)
void v9fs_free_inode(struct inode *inode)
{
+ netfs_inode_free(&V9FS_I(inode)->netfs);
kmem_cache_free(v9fs_inode_cache, V9FS_I(inode));
}
-/*
- * Set parameters for the netfs library
- */
-static void v9fs_set_netfs_context(struct inode *inode)
-{
- struct v9fs_inode *v9inode = V9FS_I(inode);
- netfs_inode_init(&v9inode->netfs, &v9fs_req_ops);
-}
-
int v9fs_init_inode(struct v9fs_session_info *v9ses,
struct inode *inode, umode_t mode, dev_t rdev)
{
@@ -345,7 +343,6 @@ int v9fs_init_inode(struct v9fs_session_info *v9ses,
goto error;
}
- v9fs_set_netfs_context(inode);
error:
return err;
diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index d7d9402ff718..6006c1210d47 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -48,6 +48,7 @@ struct inode *afs_iget_pseudo_dir(struct super_block *sb, bool root)
struct afs_vnode *vnode;
struct inode *inode;
struct afs_fid fid = {};
+ int ret;
_enter("");
@@ -76,7 +77,11 @@ struct inode *afs_iget_pseudo_dir(struct super_block *sb, bool root)
/* there shouldn't be an existing inode */
BUG_ON(!(inode->i_state & I_NEW));
- netfs_inode_init(&vnode->netfs, NULL);
+ ret = netfs_inode_init(&vnode->netfs, NULL);
+ if (ret) {
+ _leave("= %d", ret);
+ return ERR_PTR(ret);
+ }
inode->i_size = 0;
inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
if (root) {
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 6d3a3dbe4928..a69683f1c213 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -53,14 +53,6 @@ static noinline void dump_vnode(struct afs_vnode *vnode, struct afs_vnode *paren
dump_stack();
}
-/*
- * Set parameters for the netfs library
- */
-static void afs_set_netfs_context(struct afs_vnode *vnode)
-{
- netfs_inode_init(&vnode->netfs, &afs_req_ops);
-}
-
/*
* Initialise an inode from the vnode status.
*/
@@ -138,7 +130,6 @@ static int afs_inode_init_from_status(struct afs_operation *op,
}
afs_set_i_size(vnode, status->size);
- afs_set_netfs_context(vnode);
vnode->invalid_before = status->data_version;
inode_set_iversion_raw(&vnode->netfs.inode, status->data_version);
@@ -248,7 +239,7 @@ static void afs_apply_status(struct afs_operation *op,
* idea of what the size should be that's not the same as
* what's on the server.
*/
- vnode->netfs.remote_i_size = status->size;
+ vnode->netfs.info->remote_i_size = status->size;
if (change_size) {
afs_set_i_size(vnode, status->size);
inode->i_ctime = t;
@@ -432,7 +423,7 @@ static void afs_get_inode_cache(struct afs_vnode *vnode)
struct afs_vnode_cache_aux aux;
if (vnode->status.type != AFS_FTYPE_FILE) {
- vnode->netfs.cache = NULL;
+ vnode->netfs.info->cache = NULL;
return;
}
@@ -542,7 +533,6 @@ struct inode *afs_root_iget(struct super_block *sb, struct key *key)
vnode = AFS_FS_I(inode);
vnode->cb_v_break = as->volume->cb_v_break,
- afs_set_netfs_context(vnode);
op = afs_alloc_operation(key, as->volume);
if (IS_ERR(op)) {
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 723d162078a3..141b8458d989 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -679,7 +679,7 @@ static inline void afs_vnode_set_cache(struct afs_vnode *vnode,
struct fscache_cookie *cookie)
{
#ifdef CONFIG_AFS_FSCACHE
- vnode->netfs.cache = cookie;
+ vnode->netfs.info->cache = cookie;
#endif
}
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..d0ff1349a17e 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -678,11 +678,17 @@ static void afs_i_init_once(void *_vnode)
static struct inode *afs_alloc_inode(struct super_block *sb)
{
struct afs_vnode *vnode;
+ int ret;
vnode = alloc_inode_sb(sb, afs_inode_cachep, GFP_KERNEL);
if (!vnode)
return NULL;
+ ret = netfs_inode_init(&vnode->netfs, &afs_req_ops);
+ if (ret) {
+ afs_free_inode(AFS_VNODE_TO_I(vnode));
+ return NULL;
+ }
atomic_inc(&afs_count_active_inodes);
/* Reset anything that shouldn't leak from one inode to the next. */
@@ -706,6 +712,7 @@ static struct inode *afs_alloc_inode(struct super_block *sb)
static void afs_free_inode(struct inode *inode)
{
+ netfs_inode_free(&AFS_FS_I(inode)->netfs);
kmem_cache_free(afs_inode_cachep, AFS_FS_I(inode));
}
diff --git a/fs/afs/write.c b/fs/afs/write.c
index 9ebdd36eaf2f..983924427edf 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -384,7 +384,7 @@ static int afs_store_data(struct afs_vnode *vnode, struct iov_iter *iter, loff_t
op->store.write_iter = iter;
op->store.pos = pos;
op->store.size = size;
- op->store.i_size = max(pos + size, vnode->netfs.remote_i_size);
+ op->store.i_size = max(pos + size, vnode->netfs.info->remote_i_size);
op->store.laundering = laundering;
op->mtime = vnode->netfs.inode.i_mtime;
op->flags |= AFS_OPERATION_UNINTR;
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 4af5e55abc15..a6ca69591c93 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -452,6 +452,7 @@ struct inode *ceph_alloc_inode(struct super_block *sb)
{
struct ceph_inode_info *ci;
int i;
+ int ret;
ci = alloc_inode_sb(sb, ceph_inode_cachep, GFP_NOFS);
if (!ci)
@@ -460,7 +461,9 @@ struct inode *ceph_alloc_inode(struct super_block *sb)
dout("alloc_inode %p\n", &ci->netfs.inode);
/* Set parameters for the netfs library */
- netfs_inode_init(&ci->netfs, &ceph_netfs_ops);
+ ret = netfs_inode_init(&ci->netfs, &ceph_netfs_ops);
+ if (ret)
+ return NULL;
spin_lock_init(&ci->i_ceph_lock);
@@ -554,6 +557,7 @@ void ceph_free_inode(struct inode *inode)
{
struct ceph_inode_info *ci = ceph_inode(inode);
+ netfs_inode_free(&ceph_inode(inode)->netfs);
kfree(ci->i_symlink);
kmem_cache_free(ceph_inode_cachep, ci);
}
diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 0ce535852151..3a75d3784adc 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -170,8 +170,8 @@ void netfs_readahead(struct readahead_control *ractl)
if (IS_ERR(rreq))
return;
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
+ if (ctx->info->ops->begin_cache_operation) {
+ ret = ctx->info->ops->begin_cache_operation(rreq);
if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
goto cleanup_free;
}
@@ -228,8 +228,8 @@ int netfs_read_folio(struct file *file, struct folio *folio)
goto alloc_error;
}
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
+ if (ctx->info->ops->begin_cache_operation) {
+ ret = ctx->info->ops->begin_cache_operation(rreq);
if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
goto discard;
}
@@ -347,9 +347,9 @@ int netfs_write_begin(struct netfs_inode *ctx,
if (!folio)
return -ENOMEM;
- if (ctx->ops->check_write_begin) {
+ if (ctx->info->ops->check_write_begin) {
/* Allow the netfs (eg. ceph) to flush conflicts. */
- ret = ctx->ops->check_write_begin(file, pos, len, &folio, _fsdata);
+ ret = ctx->info->ops->check_write_begin(file, pos, len, &folio, _fsdata);
if (ret < 0) {
trace_netfs_failure(NULL, NULL, ret, netfs_fail_check_write_begin);
goto error;
@@ -381,8 +381,8 @@ int netfs_write_begin(struct netfs_inode *ctx,
rreq->no_unlock_folio = folio_index(folio);
__set_bit(NETFS_RREQ_NO_UNLOCK_FOLIO, &rreq->flags);
- if (ctx->ops->begin_cache_operation) {
- ret = ctx->ops->begin_cache_operation(rreq);
+ if (ctx->info->ops->begin_cache_operation) {
+ ret = ctx->info->ops->begin_cache_operation(rreq);
if (ret == -ENOMEM || ret == -EINTR || ret == -ERESTARTSYS)
goto error_put;
}
diff --git a/fs/netfs/internal.h b/fs/netfs/internal.h
index 43fac1b14e40..be3c8388812d 100644
--- a/fs/netfs/internal.h
+++ b/fs/netfs/internal.h
@@ -94,7 +94,7 @@ static inline void netfs_stat_d(atomic_t *stat)
static inline bool netfs_is_cache_enabled(struct netfs_inode *ctx)
{
#if IS_ENABLED(CONFIG_FSCACHE)
- struct fscache_cookie *cookie = ctx->cache;
+ struct fscache_cookie *cookie = ctx->info->cache;
return fscache_cookie_valid(cookie) && cookie->cache_priv &&
fscache_cookie_enabled(cookie);
diff --git a/fs/netfs/objects.c b/fs/netfs/objects.c
index e17cdf53f6a7..a1bcec71686f 100644
--- a/fs/netfs/objects.c
+++ b/fs/netfs/objects.c
@@ -29,7 +29,7 @@ struct netfs_io_request *netfs_alloc_request(struct address_space *mapping,
rreq->start = start;
rreq->len = len;
rreq->origin = origin;
- rreq->netfs_ops = ctx->ops;
+ rreq->netfs_ops = ctx->info->ops;
rreq->mapping = mapping;
rreq->inode = inode;
rreq->i_size = i_size_read(inode);
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 8dac02d05f70..e873332a9e43 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -166,13 +166,13 @@ void nfs_fscache_init_inode(struct inode *inode)
struct nfs_server *nfss = NFS_SERVER(inode);
struct nfs_inode *nfsi = NFS_I(inode);
- netfs_inode(inode)->cache = NULL;
+ netfs_inode(inode)->info->cache = NULL;
if (!(nfss->fscache && S_ISREG(inode->i_mode)))
return;
nfs_fscache_update_auxdata(&auxdata, inode);
- netfs_inode(inode)->cache = fscache_acquire_cookie(
+ netfs_inode(inode)->info->cache = fscache_acquire_cookie(
nfss->fscache,
0,
nfsi->fh.data, /* index_key */
@@ -188,7 +188,7 @@ void nfs_fscache_init_inode(struct inode *inode)
void nfs_fscache_clear_inode(struct inode *inode)
{
fscache_relinquish_cookie(netfs_i_cookie(netfs_inode(inode)), false);
- netfs_inode(inode)->cache = NULL;
+ netfs_inode(inode)->info->cache = NULL;
}
/*
@@ -240,7 +240,7 @@ void nfs_fscache_release_file(struct inode *inode, struct file *filp)
int nfs_netfs_read_folio(struct file *file, struct folio *folio)
{
- if (!netfs_inode(folio_inode(folio))->cache)
+ if (!netfs_inode(folio_inode(folio))->info->cache)
return -ENOBUFS;
return netfs_read_folio(file, folio);
@@ -250,7 +250,7 @@ int nfs_netfs_readahead(struct readahead_control *ractl)
{
struct inode *inode = ractl->mapping->host;
- if (!netfs_inode(inode)->cache)
+ if (!netfs_inode(inode)->info->cache)
return -ENOBUFS;
netfs_readahead(ractl);
@@ -354,7 +354,7 @@ void nfs_netfs_readpage_release(struct nfs_page *req)
/*
* If fscache is enabled, netfs will unlock pages.
*/
- if (netfs_inode(inode)->cache)
+ if (netfs_inode(inode)->info->cache)
return;
unlock_page(req->wb_page);
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 18da26157fda..23c9412a671e 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -78,9 +78,13 @@ static inline void nfs_netfs_put(struct nfs_netfs_io_data *netfs)
netfs_subreq_terminated(netfs->sreq, netfs->error ?: final_len, false);
kfree(netfs);
}
-static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi)
+static inline int nfs_netfs_inode_init(struct nfs_inode *nfsi)
{
- netfs_inode_init(&nfsi->netfs, &nfs_netfs_ops);
+ return netfs_inode_init(&nfsi->netfs, &nfs_netfs_ops);
+}
+static inline void nfs_netfs_inode_free(struct nfs_inode *nfsi)
+{
+ netfs_inode_free(&nfsi->netfs);
}
extern void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr);
extern void nfs_netfs_read_completion(struct nfs_pgio_header *hdr);
@@ -160,7 +164,8 @@ static inline void nfs_netfs_reset_pageio_descriptor(struct nfs_pageio_descripto
desc->pg_netfs = NULL;
}
#else /* CONFIG_NFS_FSCACHE */
-static inline void nfs_netfs_inode_init(struct nfs_inode *nfsi) {}
+static inline int nfs_netfs_inode_init(struct nfs_inode *nfsi) { return 0; }
+static inline void nfs_netfs_inode_free(struct nfs_inode *nfsi) { }
static inline void nfs_netfs_initiate_read(struct nfs_pgio_header *hdr) {}
static inline void nfs_netfs_read_completion(struct nfs_pgio_header *hdr) {}
static inline void nfs_netfs_readpage_release(struct nfs_page *req)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 116ae689a490..ba7a0f3167b6 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -2234,6 +2234,8 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
struct inode *nfs_alloc_inode(struct super_block *sb)
{
struct nfs_inode *nfsi;
+ int ret;
+
nfsi = alloc_inode_sb(sb, nfs_inode_cachep, GFP_KERNEL);
if (!nfsi)
return NULL;
@@ -2245,7 +2247,11 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
#ifdef CONFIG_NFS_V4_2
nfsi->xattr_cache = NULL;
#endif
- nfs_netfs_inode_init(nfsi);
+ ret = nfs_netfs_inode_init(nfsi);
+ if (ret) {
+ kmem_cache_free(nfs_inode_cachep, nfsi);
+ return NULL;
+ }
return VFS_I(nfsi);
}
@@ -2253,6 +2259,7 @@ EXPORT_SYMBOL_GPL(nfs_alloc_inode);
void nfs_free_inode(struct inode *inode)
{
+ nfs_netfs_inode_free(NFS_I(inode));
kmem_cache_free(nfs_inode_cachep, NFS_I(inode));
}
EXPORT_SYMBOL_GPL(nfs_free_inode);
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index f2402ddeafbf..4b4ddafab5e3 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -118,11 +118,7 @@ enum netfs_io_source {
typedef void (*netfs_io_terminated_t)(void *priv, ssize_t transferred_or_error,
bool was_async);
-/*
- * Per-inode context. This wraps the VFS inode.
- */
-struct netfs_inode {
- struct inode inode; /* The VFS inode */
+struct netfs_info {
const struct netfs_request_ops *ops;
#if IS_ENABLED(CONFIG_FSCACHE)
struct fscache_cookie *cache;
@@ -130,6 +126,14 @@ struct netfs_inode {
loff_t remote_i_size; /* Size of the remote file */
};
+/*
+ * Per-inode context. This wraps the VFS inode.
+ */
+struct netfs_inode {
+ struct inode inode; /* The VFS inode */
+ struct netfs_info *info; /* Rest of netfs data */
+};
+
/*
* Resources required to do operations on a cache.
*/
@@ -309,14 +313,29 @@ static inline struct netfs_inode *netfs_inode(struct inode *inode)
* Initialise the netfs library context struct. This is expected to follow on
* directly from the VFS inode struct.
*/
-static inline void netfs_inode_init(struct netfs_inode *ctx,
+static inline int netfs_inode_init(struct netfs_inode *ctx,
const struct netfs_request_ops *ops)
{
- ctx->ops = ops;
- ctx->remote_i_size = i_size_read(&ctx->inode);
+ ctx->info = kzalloc(sizeof(struct netfs_info), GFP_KERNEL);
+ if (!ctx->info )
+ return -ENOMEM;
+ ctx->info->ops = ops;
+ ctx->info->remote_i_size = i_size_read(&ctx->inode);
#if IS_ENABLED(CONFIG_FSCACHE)
- ctx->cache = NULL;
+ ctx->info->cache = NULL;
#endif
+ return 0;
+}
+
+/**
+ * netfs_inode_free - Free a netfslib inode context
+ * @ctx: The netfs inode to free
+ *
+ * Free the netfs library info struct.
+ */
+static inline void netfs_inode_free(struct netfs_inode *ctx)
+{
+ kfree(ctx->info);
}
/**
@@ -328,7 +347,7 @@ static inline void netfs_inode_init(struct netfs_inode *ctx,
*/
static inline void netfs_resize_file(struct netfs_inode *ctx, loff_t new_i_size)
{
- ctx->remote_i_size = new_i_size;
+ ctx->info->remote_i_size = new_i_size;
}
/**
@@ -340,7 +359,7 @@ static inline void netfs_resize_file(struct netfs_inode *ctx, loff_t new_i_size)
static inline struct fscache_cookie *netfs_i_cookie(struct netfs_inode *ctx)
{
#if IS_ENABLED(CONFIG_FSCACHE)
- return ctx->cache;
+ return ctx->info->cache;
#else
return NULL;
#endif
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC PATCH v10 6/6] netfs: Change netfs_inode_init to allocate memory to allow opt-in
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
` (5 preceding siblings ...)
2022-11-03 16:16 ` [RFC PATCH v10 6/6] netfs: Change netfs_inode_init to allocate memory to allow opt-in Dave Wysochanski
@ 2022-11-03 16:38 ` David Howells
2023-02-09 14:57 ` [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API David Wysochanski
7 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2022-11-03 16:38 UTC (permalink / raw)
To: Dave Wysochanski
Cc: dhowells, Anna Schumaker, Trond Myklebust, linux-nfs,
linux-cachefs, Benjamin Maynard, Daire Byrne
Dave Wysochanski <dwysocha@redhat.com> wrote:
> This saves memory for network filesystems inode that would build
> in netfs support, but like to opt-in to netfs on some mounts while
> opting-out of netfs on other mounts.
> ...
> static struct inode *afs_alloc_inode(struct super_block *sb)
> {
> struct afs_vnode *vnode;
> + int ret;
>
> vnode = alloc_inode_sb(sb, afs_inode_cachep, GFP_KERNEL);
> if (!vnode)
> return NULL;
>
> + ret = netfs_inode_init(&vnode->netfs, &afs_req_ops);
> + if (ret) {
> + afs_free_inode(AFS_VNODE_TO_I(vnode));
> + return NULL;
> + }
> atomic_inc(&afs_count_active_inodes);
>
> /* Reset anything that shouldn't leak from one inode to the next. */
This makes the memory footprint worse for 9p, afs, ceph and cifs - and adds a
time penalty to inode creation and destruction as well (though probably not a
huge amount).
David
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
` (6 preceding siblings ...)
2022-11-03 16:38 ` David Howells
@ 2023-02-09 14:57 ` David Wysochanski
2023-02-09 17:40 ` Trond Myklebust
7 siblings, 1 reply; 11+ messages in thread
From: David Wysochanski @ 2023-02-09 14:57 UTC (permalink / raw)
To: Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard
On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski <dwysocha@redhat.com> wrote:
>
> This v10 patchset addresses at least some of Trond's latest concerns.
> Some of the feedback like the unlock_page() wrapper function in
> nfs_read_completion() I don't know how to address without an
> ifdef. Other feedback I'm not quite sure about splitting out
> netfs bits or what you would like to see. Trond I do not want to
> in any way ignore or miss any of your feedback so please elaborate
> as needed.
>
> This patchset converts NFS with fscache non-direct 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 concerns about modifying the IO path [1] as
> well as only enabling netfs when fscache is configured and enabled [2].
> I have not attempted performance comparisions to address Chuck
> Lever's concern [3] because we are not converting the non-fscache
> enabled NFS IO paths to netfs.
>
> The patchset is based on 6.1-rc3 and has been pushed to github at:
> https://github.com/DaveWysochanskiRH/kernel/commits/nfs-fscache-netfs
> https://github.com/DaveWysochanskiRH/kernel/commit/bff09aa979010f38a11a6f92451e85d04d850715
>
> Changes since v9 [7]
> ====================
> PATCH1: Rename nfs_pageio_add_page to nfs_read_add_page (Trond)
> PATCH3: Remove a few #ifdef's and replace with wrappers (Trond) [8]
> PATCH6: RFC patch to reduce increase in nfs_inode memory footprint
> when netfs is configured but not enabled (Trond) [9]
>
> Testing
> =======
> I did not do much testing on this as the changes to patches 1 and 3
> are cosmetic. Patch #6 is RFC patch and may change, so if that is
> added it may need more testing.
>
> 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 linux-cachefs [4] patch to resolve
> "Stop read optimisation when folio removed from pagecache"
> * Daire Byrne also verified the patch fixes his issue as well
>
> 2. "Cache volume key already in use" after xfstest runs
> * xfstests (hammerspace with vers=4.2,fsc) shows the following on the
> console after some tests:
> "NFS: Cache volume key already in use (nfs,4.1,2,c50,cfe0100a,3,,,8000,100000,100000,bb8,ea60,7530,ea60,1)"
> * This may be fixed with another patch [5] that is in progress
>
> 3. Daire Byrne reported a NULL pointer oops at cachefiles_prepare_write+0x28/0x90
> * harder to reproduce/debug but under investigation [6]
> * only reproduced on RHEL7.9 based NFS re-export server using fscache with upstream kernel plus
> the previous patches
> * Debug in progress, first pass at where the problem is indicates a race
> between fscache cookie LRU and use_cookie; looking at cookie state machine [10]
>
> [58710.346376] BUG: kernel NULL pointer dereference, address: 0000000000000008
> [58710.371212] CPU: 12 PID: 9134 Comm: kworker/u129:0 Tainted: G E 6.0.0-2.dneg.x86_64 #1
> ...
> [58710.389995] Workqueue: events_unbound netfs_rreq_write_to_cache_work [netfs]
> [58710.397188] RIP: 0010:cachefiles_prepare_write+0x28/0x90 [cachefiles]
> ...
> [58710.500316] Call Trace:
> [58710.502894] <TASK>
> [58710.505126] netfs_rreq_write_to_cache_work+0x11c/0x320 [netfs]
> [58710.511201] process_one_work+0x217/0x3e0
> [58710.515358] worker_thread+0x4a/0x3b0
> [58710.519152] ? process_one_work+0x3e0/0x3e0
> [58710.523467] kthread+0xd6/0x100
> [58710.526740] ? kthread_complete_and_exit+0x20/0x20
> [58710.531659] ret_from_fork+0x1f/0x30
>
>
>
> References
> ==========
> [1] https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@hammerspace.com/
> [2] https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@hammerspace.com/
> [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> [4] https://www.mail-archive.com/linux-cachefs@redhat.com/msg03043.html
> [5] https://marc.info/?l=linux-nfs&m=165962662200679&w=4
> [6] https://listman.redhat.com/archives/linux-cachefs/2022-September/007183.html
> [7] https://marc.info/?l=linux-nfs&m=166600357429305&w=4
> [8] https://marc.info/?l=linux-nfs&m=166697599503342&w=4
> [9] https://marc.info/?l=linux-nfs&m=166717208305834&w=4
> [10] https://listman.redhat.com/archives/linux-cachefs/2022-October/007259.html
>
> Dave Wysochanski (5):
> NFS: Rename readpage_async_filler to nfs_pageio_add_page
> 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/delegation.c | 2 +-
> fs/nfs/dir.c | 2 +-
> fs/nfs/fscache.c | 242 ++++++++++++++++++++++---------------
> fs/nfs/fscache.h | 111 +++++++++++------
> fs/nfs/inode.c | 8 +-
> fs/nfs/internal.h | 11 +-
> fs/nfs/iostat.h | 17 ---
> fs/nfs/nfstrace.h | 91 --------------
> fs/nfs/pagelist.c | 12 ++
> fs/nfs/pnfs.c | 12 +-
> fs/nfs/read.c | 110 +++++++++--------
> fs/nfs/super.c | 11 --
> fs/nfs/write.c | 2 +-
> include/linux/nfs_fs.h | 35 ++++--
> include/linux/nfs_iostat.h | 12 --
> include/linux/nfs_page.h | 3 +
> include/linux/nfs_xdr.h | 3 +
> 18 files changed, 335 insertions(+), 350 deletions(-)
>
> --
> 2.31.1
>
> *** BLURB HERE ***
>
> Dave Wysochanski (6):
> NFS: Rename readpage_async_filler to nfs_read_add_page
> 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
> netfs: Change netfs_inode_init to allocate memory to allow opt-in
>
> fs/9p/cache.c | 2 +-
> fs/9p/vfs_inode.c | 17 ++-
> fs/afs/dynroot.c | 7 +-
> fs/afs/inode.c | 14 +--
> fs/afs/internal.h | 2 +-
> fs/afs/super.c | 7 ++
> fs/afs/write.c | 2 +-
> fs/ceph/inode.c | 6 +-
> fs/netfs/buffered_read.c | 16 +--
> fs/netfs/internal.h | 2 +-
> fs/netfs/objects.c | 2 +-
> fs/nfs/Kconfig | 1 +
> fs/nfs/delegation.c | 2 +-
> fs/nfs/dir.c | 2 +-
> fs/nfs/fscache.c | 242 ++++++++++++++++++++++---------------
> fs/nfs/fscache.h | 136 +++++++++++++++------
> fs/nfs/inode.c | 15 ++-
> fs/nfs/internal.h | 11 +-
> fs/nfs/iostat.h | 17 ---
> fs/nfs/nfstrace.h | 91 --------------
> fs/nfs/pagelist.c | 4 +
> fs/nfs/pnfs.c | 12 +-
> fs/nfs/read.c | 110 +++++++++--------
> fs/nfs/super.c | 11 --
> fs/nfs/write.c | 2 +-
> include/linux/netfs.h | 41 +++++--
> include/linux/nfs_fs.h | 35 ++++--
> include/linux/nfs_iostat.h | 12 --
> include/linux/nfs_page.h | 3 +
> include/linux/nfs_xdr.h | 3 +
> 30 files changed, 428 insertions(+), 399 deletions(-)
>
> --
> 2.31.1
>
> --
> Linux-cachefs mailing list
> Linux-cachefs@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-cachefs
>
Trond, David H, Ben, Daire, others,
I am not sure about the next steps.
I did not see any responses to this v10 posting, other than dhowells
did not like the overhead that patch 6 added to other filesystems
using netfs. I'm not sure if that's a full NACK on that patch but it
sounded like it to me.
Trond is it ok if I drop patch 6?
Beyond patch 6, Trond, I could post a rebased v11 but I am not sure it
is acceptable to you the way it is and I don't want to do that if
there's changes you want.
From your responses on v9, one issue seems to be that you do not like
the wrapping the NFS requests inside netfs requests for example.
But I do not know another approach other than bypassing pgio layer
completely which as far as I understand creates a whole new set of
issues to be solved.
Possibly you have another approach in mind or see the need for other
refactoring or patches that should be done that would make this set
more acceptable?
I am not sure if you have other concerns on this v10. If steps can be
outlined a little better I can work on them.
As it is now I'm not sure whether this needs a rebase and a v11
posting, or a rethinking of the approach.
Regarding the known issues, as far as I know issues #1 and #2 are
still outstanding.
I know issue #3 is fixed with
b5b52de3214a fscache: Fix oops due to race with cookie_lru and use_cookie
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API
2023-02-09 14:57 ` [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API David Wysochanski
@ 2023-02-09 17:40 ` Trond Myklebust
2023-02-13 15:55 ` David Wysochanski
0 siblings, 1 reply; 11+ messages in thread
From: Trond Myklebust @ 2023-02-09 17:40 UTC (permalink / raw)
To: David Wysochanski, Anna Schumaker, Trond Myklebust, David Howells
Cc: linux-nfs, linux-cachefs, Benjamin Maynard
On Thu, 2023-02-09 at 09:57 -0500, David Wysochanski wrote:
> On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski
> <dwysocha@redhat.com> wrote:
> >
> > This v10 patchset addresses at least some of Trond's latest
> > concerns.
> > Some of the feedback like the unlock_page() wrapper function in
> > nfs_read_completion() I don't know how to address without an
> > ifdef. Other feedback I'm not quite sure about splitting out
> > netfs bits or what you would like to see. Trond I do not want to
> > in any way ignore or miss any of your feedback so please elaborate
> > as needed.
> >
> > This patchset converts NFS with fscache non-direct 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 concerns about modifying the IO path [1]
> > as
> > well as only enabling netfs when fscache is configured and enabled
> > [2].
> > I have not attempted performance comparisions to address Chuck
> > Lever's concern [3] because we are not converting the non-fscache
> > enabled NFS IO paths to netfs.
> >
> > The patchset is based on 6.1-rc3 and has been pushed to github at:
> > https://github.com/DaveWysochanskiRH/kernel/commits/nfs-fscache-netfs
> > https://github.com/DaveWysochanskiRH/kernel/commit/bff09aa979010f38a11a6f92451e85d04d850715
> >
> > Changes since v9 [7]
> > ====================
> > PATCH1: Rename nfs_pageio_add_page to nfs_read_add_page (Trond)
> > PATCH3: Remove a few #ifdef's and replace with wrappers (Trond) [8]
> > PATCH6: RFC patch to reduce increase in nfs_inode memory footprint
> > when netfs is configured but not enabled (Trond) [9]
> >
> > Testing
> > =======
> > I did not do much testing on this as the changes to patches 1 and 3
> > are cosmetic. Patch #6 is RFC patch and may change, so if that is
> > added it may need more testing.
> >
> > 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 linux-cachefs [4] patch to
> > resolve
> > "Stop read optimisation when folio removed from pagecache"
> > * Daire Byrne also verified the patch fixes his issue as well
> >
> > 2. "Cache volume key already in use" after xfstest runs
> > * xfstests (hammerspace with vers=4.2,fsc) shows the following on
> > the
> > console after some tests:
> > "NFS: Cache volume key already in use
> > (nfs,4.1,2,c50,cfe0100a,3,,,8000,100000,100000,bb8,ea60,7530,ea60,1
> > )"
> > * This may be fixed with another patch [5] that is in progress
> >
> > 3. Daire Byrne reported a NULL pointer oops at
> > cachefiles_prepare_write+0x28/0x90
> > * harder to reproduce/debug but under investigation [6]
> > * only reproduced on RHEL7.9 based NFS re-export server using
> > fscache with upstream kernel plus
> > the previous patches
> > * Debug in progress, first pass at where the problem is indicates a
> > race
> > between fscache cookie LRU and use_cookie; looking at cookie state
> > machine [10]
> >
> > [58710.346376] BUG: kernel NULL pointer dereference, address:
> > 0000000000000008
> > [58710.371212] CPU: 12 PID: 9134 Comm: kworker/u129:0 Tainted: G
> > E 6.0.0-2.dneg.x86_64 #1
> > ...
> > [58710.389995] Workqueue: events_unbound
> > netfs_rreq_write_to_cache_work [netfs]
> > [58710.397188] RIP: 0010:cachefiles_prepare_write+0x28/0x90
> > [cachefiles]
> > ...
> > [58710.500316] Call Trace:
> > [58710.502894] <TASK>
> > [58710.505126] netfs_rreq_write_to_cache_work+0x11c/0x320 [netfs]
> > [58710.511201] process_one_work+0x217/0x3e0
> > [58710.515358] worker_thread+0x4a/0x3b0
> > [58710.519152] ? process_one_work+0x3e0/0x3e0
> > [58710.523467] kthread+0xd6/0x100
> > [58710.526740] ? kthread_complete_and_exit+0x20/0x20
> > [58710.531659] ret_from_fork+0x1f/0x30
> >
> >
> >
> > References
> > ==========
> > [1]
> > https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@hammerspace.com/
> > [2]
> > https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@hammerspace.com/
> > [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> > [4]
> > https://www.mail-archive.com/linux-cachefs@redhat.com/msg03043.html
> > [5] https://marc.info/?l=linux-nfs&m=165962662200679&w=4
> > [6]
> > https://listman.redhat.com/archives/linux-cachefs/2022-September/007183.html
> > [7] https://marc.info/?l=linux-nfs&m=166600357429305&w=4
> > [8] https://marc.info/?l=linux-nfs&m=166697599503342&w=4
> > [9] https://marc.info/?l=linux-nfs&m=166717208305834&w=4
> > [10]
> > https://listman.redhat.com/archives/linux-cachefs/2022-October/007259.html
> >
> > Dave Wysochanski (5):
> > NFS: Rename readpage_async_filler to nfs_pageio_add_page
> > 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/delegation.c | 2 +-
> > fs/nfs/dir.c | 2 +-
> > fs/nfs/fscache.c | 242 ++++++++++++++++++++++-----------
> > ----
> > fs/nfs/fscache.h | 111 +++++++++++------
> > fs/nfs/inode.c | 8 +-
> > fs/nfs/internal.h | 11 +-
> > fs/nfs/iostat.h | 17 ---
> > fs/nfs/nfstrace.h | 91 --------------
> > fs/nfs/pagelist.c | 12 ++
> > fs/nfs/pnfs.c | 12 +-
> > fs/nfs/read.c | 110 +++++++++--------
> > fs/nfs/super.c | 11 --
> > fs/nfs/write.c | 2 +-
> > include/linux/nfs_fs.h | 35 ++++--
> > include/linux/nfs_iostat.h | 12 --
> > include/linux/nfs_page.h | 3 +
> > include/linux/nfs_xdr.h | 3 +
> > 18 files changed, 335 insertions(+), 350 deletions(-)
> >
> > --
> > 2.31.1
> >
> > *** BLURB HERE ***
> >
> > Dave Wysochanski (6):
> > NFS: Rename readpage_async_filler to nfs_read_add_page
> > 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
> > netfs: Change netfs_inode_init to allocate memory to allow opt-in
> >
> > fs/9p/cache.c | 2 +-
> > fs/9p/vfs_inode.c | 17 ++-
> > fs/afs/dynroot.c | 7 +-
> > fs/afs/inode.c | 14 +--
> > fs/afs/internal.h | 2 +-
> > fs/afs/super.c | 7 ++
> > fs/afs/write.c | 2 +-
> > fs/ceph/inode.c | 6 +-
> > fs/netfs/buffered_read.c | 16 +--
> > fs/netfs/internal.h | 2 +-
> > fs/netfs/objects.c | 2 +-
> > fs/nfs/Kconfig | 1 +
> > fs/nfs/delegation.c | 2 +-
> > fs/nfs/dir.c | 2 +-
> > fs/nfs/fscache.c | 242 ++++++++++++++++++++++-----------
> > ----
> > fs/nfs/fscache.h | 136 +++++++++++++++------
> > fs/nfs/inode.c | 15 ++-
> > fs/nfs/internal.h | 11 +-
> > fs/nfs/iostat.h | 17 ---
> > fs/nfs/nfstrace.h | 91 --------------
> > fs/nfs/pagelist.c | 4 +
> > fs/nfs/pnfs.c | 12 +-
> > fs/nfs/read.c | 110 +++++++++--------
> > fs/nfs/super.c | 11 --
> > fs/nfs/write.c | 2 +-
> > include/linux/netfs.h | 41 +++++--
> > include/linux/nfs_fs.h | 35 ++++--
> > include/linux/nfs_iostat.h | 12 --
> > include/linux/nfs_page.h | 3 +
> > include/linux/nfs_xdr.h | 3 +
> > 30 files changed, 428 insertions(+), 399 deletions(-)
> >
> > --
> > 2.31.1
> >
> > --
> > Linux-cachefs mailing list
> > Linux-cachefs@redhat.com
> > https://listman.redhat.com/mailman/listinfo/linux-cachefs
> >
>
> Trond, David H, Ben, Daire, others,
>
> I am not sure about the next steps.
>
> I did not see any responses to this v10 posting, other than dhowells
> did not like the overhead that patch 6 added to other filesystems
> using netfs. I'm not sure if that's a full NACK on that patch but it
> sounded like it to me.
> Trond is it ok if I drop patch 6?
>
If you drop patch 6, then we need another way to get rid of the
ugliness introduced by netfs_inode. I don't want to add those wrappers
in order to access the inode in 'struct nfs_inode'.
One solution might be an anonymous union. i.e.
struct nfs_inode {
....
union {
struct inode vfs_inode;
#ifdef CONFIG_NFS_FSCACHE
struct netfs_inode netfs_inode;
#endif
};
};
...and then move the wretched xattr_cache field to reside above that
union.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API
2023-02-09 17:40 ` Trond Myklebust
@ 2023-02-13 15:55 ` David Wysochanski
0 siblings, 0 replies; 11+ messages in thread
From: David Wysochanski @ 2023-02-13 15:55 UTC (permalink / raw)
To: Trond Myklebust
Cc: Anna Schumaker, Trond Myklebust, David Howells, linux-nfs,
linux-cachefs, Benjamin Maynard
On Thu, Feb 9, 2023 at 12:41 PM Trond Myklebust <trondmy@kernel.org> wrote:
>
> On Thu, 2023-02-09 at 09:57 -0500, David Wysochanski wrote:
> > On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski
> > <dwysocha@redhat.com> wrote:
> > >
> > > This v10 patchset addresses at least some of Trond's latest
> > > concerns.
> > > Some of the feedback like the unlock_page() wrapper function in
> > > nfs_read_completion() I don't know how to address without an
> > > ifdef. Other feedback I'm not quite sure about splitting out
> > > netfs bits or what you would like to see. Trond I do not want to
> > > in any way ignore or miss any of your feedback so please elaborate
> > > as needed.
> > >
> > > This patchset converts NFS with fscache non-direct 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 concerns about modifying the IO path [1]
> > > as
> > > well as only enabling netfs when fscache is configured and enabled
> > > [2].
> > > I have not attempted performance comparisions to address Chuck
> > > Lever's concern [3] because we are not converting the non-fscache
> > > enabled NFS IO paths to netfs.
> > >
> > > The patchset is based on 6.1-rc3 and has been pushed to github at:
> > > https://github.com/DaveWysochanskiRH/kernel/commits/nfs-fscache-netfs
> > > https://github.com/DaveWysochanskiRH/kernel/commit/bff09aa979010f38a11a6f92451e85d04d850715
> > >
> > > Changes since v9 [7]
> > > ====================
> > > PATCH1: Rename nfs_pageio_add_page to nfs_read_add_page (Trond)
> > > PATCH3: Remove a few #ifdef's and replace with wrappers (Trond) [8]
> > > PATCH6: RFC patch to reduce increase in nfs_inode memory footprint
> > > when netfs is configured but not enabled (Trond) [9]
> > >
> > > Testing
> > > =======
> > > I did not do much testing on this as the changes to patches 1 and 3
> > > are cosmetic. Patch #6 is RFC patch and may change, so if that is
> > > added it may need more testing.
> > >
> > > 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 linux-cachefs [4] patch to
> > > resolve
> > > "Stop read optimisation when folio removed from pagecache"
> > > * Daire Byrne also verified the patch fixes his issue as well
> > >
> > > 2. "Cache volume key already in use" after xfstest runs
> > > * xfstests (hammerspace with vers=4.2,fsc) shows the following on
> > > the
> > > console after some tests:
> > > "NFS: Cache volume key already in use
> > > (nfs,4.1,2,c50,cfe0100a,3,,,8000,100000,100000,bb8,ea60,7530,ea60,1
> > > )"
> > > * This may be fixed with another patch [5] that is in progress
> > >
> > > 3. Daire Byrne reported a NULL pointer oops at
> > > cachefiles_prepare_write+0x28/0x90
> > > * harder to reproduce/debug but under investigation [6]
> > > * only reproduced on RHEL7.9 based NFS re-export server using
> > > fscache with upstream kernel plus
> > > the previous patches
> > > * Debug in progress, first pass at where the problem is indicates a
> > > race
> > > between fscache cookie LRU and use_cookie; looking at cookie state
> > > machine [10]
> > >
> > > [58710.346376] BUG: kernel NULL pointer dereference, address:
> > > 0000000000000008
> > > [58710.371212] CPU: 12 PID: 9134 Comm: kworker/u129:0 Tainted: G
> > > E 6.0.0-2.dneg.x86_64 #1
> > > ...
> > > [58710.389995] Workqueue: events_unbound
> > > netfs_rreq_write_to_cache_work [netfs]
> > > [58710.397188] RIP: 0010:cachefiles_prepare_write+0x28/0x90
> > > [cachefiles]
> > > ...
> > > [58710.500316] Call Trace:
> > > [58710.502894] <TASK>
> > > [58710.505126] netfs_rreq_write_to_cache_work+0x11c/0x320 [netfs]
> > > [58710.511201] process_one_work+0x217/0x3e0
> > > [58710.515358] worker_thread+0x4a/0x3b0
> > > [58710.519152] ? process_one_work+0x3e0/0x3e0
> > > [58710.523467] kthread+0xd6/0x100
> > > [58710.526740] ? kthread_complete_and_exit+0x20/0x20
> > > [58710.531659] ret_from_fork+0x1f/0x30
> > >
> > >
> > >
> > > References
> > > ==========
> > > [1]
> > > https://lore.kernel.org/linux-nfs/9cfd5bc3cfc6abc2d3316b0387222e708d67f595.camel@hammerspace.com/
> > > [2]
> > > https://lore.kernel.org/linux-nfs/da9200f1bded9b8b078a7aef227fd6b92eb028fb.camel@hammerspace.com/
> > > [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> > > [4]
> > > https://www.mail-archive.com/linux-cachefs@redhat.com/msg03043.html
> > > [5] https://marc.info/?l=linux-nfs&m=165962662200679&w=4
> > > [6]
> > > https://listman.redhat.com/archives/linux-cachefs/2022-September/007183.html
> > > [7] https://marc.info/?l=linux-nfs&m=166600357429305&w=4
> > > [8] https://marc.info/?l=linux-nfs&m=166697599503342&w=4
> > > [9] https://marc.info/?l=linux-nfs&m=166717208305834&w=4
> > > [10]
> > > https://listman.redhat.com/archives/linux-cachefs/2022-October/007259.html
> > >
> > > Dave Wysochanski (5):
> > > NFS: Rename readpage_async_filler to nfs_pageio_add_page
> > > 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/delegation.c | 2 +-
> > > fs/nfs/dir.c | 2 +-
> > > fs/nfs/fscache.c | 242 ++++++++++++++++++++++-----------
> > > ----
> > > fs/nfs/fscache.h | 111 +++++++++++------
> > > fs/nfs/inode.c | 8 +-
> > > fs/nfs/internal.h | 11 +-
> > > fs/nfs/iostat.h | 17 ---
> > > fs/nfs/nfstrace.h | 91 --------------
> > > fs/nfs/pagelist.c | 12 ++
> > > fs/nfs/pnfs.c | 12 +-
> > > fs/nfs/read.c | 110 +++++++++--------
> > > fs/nfs/super.c | 11 --
> > > fs/nfs/write.c | 2 +-
> > > include/linux/nfs_fs.h | 35 ++++--
> > > include/linux/nfs_iostat.h | 12 --
> > > include/linux/nfs_page.h | 3 +
> > > include/linux/nfs_xdr.h | 3 +
> > > 18 files changed, 335 insertions(+), 350 deletions(-)
> > >
> > > --
> > > 2.31.1
> > >
> > > *** BLURB HERE ***
> > >
> > > Dave Wysochanski (6):
> > > NFS: Rename readpage_async_filler to nfs_read_add_page
> > > 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
> > > netfs: Change netfs_inode_init to allocate memory to allow opt-in
> > >
> > > fs/9p/cache.c | 2 +-
> > > fs/9p/vfs_inode.c | 17 ++-
> > > fs/afs/dynroot.c | 7 +-
> > > fs/afs/inode.c | 14 +--
> > > fs/afs/internal.h | 2 +-
> > > fs/afs/super.c | 7 ++
> > > fs/afs/write.c | 2 +-
> > > fs/ceph/inode.c | 6 +-
> > > fs/netfs/buffered_read.c | 16 +--
> > > fs/netfs/internal.h | 2 +-
> > > fs/netfs/objects.c | 2 +-
> > > fs/nfs/Kconfig | 1 +
> > > fs/nfs/delegation.c | 2 +-
> > > fs/nfs/dir.c | 2 +-
> > > fs/nfs/fscache.c | 242 ++++++++++++++++++++++-----------
> > > ----
> > > fs/nfs/fscache.h | 136 +++++++++++++++------
> > > fs/nfs/inode.c | 15 ++-
> > > fs/nfs/internal.h | 11 +-
> > > fs/nfs/iostat.h | 17 ---
> > > fs/nfs/nfstrace.h | 91 --------------
> > > fs/nfs/pagelist.c | 4 +
> > > fs/nfs/pnfs.c | 12 +-
> > > fs/nfs/read.c | 110 +++++++++--------
> > > fs/nfs/super.c | 11 --
> > > fs/nfs/write.c | 2 +-
> > > include/linux/netfs.h | 41 +++++--
> > > include/linux/nfs_fs.h | 35 ++++--
> > > include/linux/nfs_iostat.h | 12 --
> > > include/linux/nfs_page.h | 3 +
> > > include/linux/nfs_xdr.h | 3 +
> > > 30 files changed, 428 insertions(+), 399 deletions(-)
> > >
> > > --
> > > 2.31.1
> > >
> > > --
> > > Linux-cachefs mailing list
> > > Linux-cachefs@redhat.com
> > > https://listman.redhat.com/mailman/listinfo/linux-cachefs
> > >
> >
> > Trond, David H, Ben, Daire, others,
> >
> > I am not sure about the next steps.
> >
> > I did not see any responses to this v10 posting, other than dhowells
> > did not like the overhead that patch 6 added to other filesystems
> > using netfs. I'm not sure if that's a full NACK on that patch but it
> > sounded like it to me.
> > Trond is it ok if I drop patch 6?
> >
>
> If you drop patch 6, then we need another way to get rid of the
> ugliness introduced by netfs_inode. I don't want to add those wrappers
> in order to access the inode in 'struct nfs_inode'.
>
> One solution might be an anonymous union. i.e.
> struct nfs_inode {
> ....
> union {
> struct inode vfs_inode;
> #ifdef CONFIG_NFS_FSCACHE
> struct netfs_inode netfs_inode;
> #endif
> };
> };
>
>
> ...and then move the wretched xattr_cache field to reside above that
> union.
>
Yes I definitely can do this. Making this change reduces the churn
in patch #2, significantly (see below).
Do you want me to rebase, test, and re-post a v11 of this series
through patch #5 or do you want more time to comment on patches
3-5?
$ git show --stat
commit a8b2617550ea85f40a546430f3199670beccec1d (HEAD ->
nfs-fscache-netfs, origin/nfs-fscache-netfs)
Author: Dave Wysochanski <dwysocha@redhat.com>
Date: Wed May 4 10:12:47 2022 -0400
NFS: Configure support for netfs when NFS fscache is configured
As first steps for support of the netfs library when NFS_FSCACHE is
configured, add NETFS_SUPPORT to Kconfig and add the required netfs_inode
into struct nfs_inode.
Using netfs requires we move the VFS inode structure to be stored
inside struct netfs_inode, along with the fscache_cookie.
Thus, if NFS_FSCACHE is configured, place netfs_inode inside an
anonymous union so the vfs_inode memory is the same and we do
not need to modify other non-fscache areas of NFS.
In addition, inside the NFS fscache code, use the new helpers,
netfs_inode() and netfs_i_cookie() helpers, and remove our own
helper, nfs_i_fscache().
Later patches will convert NFS fscache to fully use netfs.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
fs/nfs/Kconfig | 1 +
fs/nfs/fscache.c | 20 +++++++++-----------
fs/nfs/fscache.h | 15 ++++++---------
include/linux/nfs_fs.h | 24 ++++++++++--------------
4 files changed, 26 insertions(+), 34 deletions(-)
[dwysocha@dwysocha kernel]$ git show include/linux/nfs_fs.h
commit a8b2617550ea85f40a546430f3199670beccec1d (HEAD ->
nfs-fscache-netfs, origin/nfs-fscache-netfs)
Author: Dave Wysochanski <dwysocha@redhat.com>
Date: Wed May 4 10:12:47 2022 -0400
NFS: Configure support for netfs when NFS fscache is configured
As first steps for support of the netfs library when NFS_FSCACHE is
configured, add NETFS_SUPPORT to Kconfig and add the required netfs_inode
into struct nfs_inode.
Using netfs requires we move the VFS inode structure to be stored
inside struct netfs_inode, along with the fscache_cookie.
Thus, if NFS_FSCACHE is configured, place netfs_inode inside an
anonymous union so the vfs_inode memory is the same and we do
not need to modify other non-fscache areas of NFS.
In addition, inside the NFS fscache code, use the new helpers,
netfs_inode() and netfs_i_cookie() helpers, and remove our own
helper, nfs_i_fscache().
Later patches will convert NFS fscache to fully use netfs.
Signed-off-by: Dave Wysochanski <dwysocha@redhat.com>
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 45c44211e50e..580847c70fec 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -31,6 +31,10 @@
#include <linux/sunrpc/auth.h>
#include <linux/sunrpc/clnt.h>
+#ifdef CONFIG_NFS_FSCACHE
+#include <linux/netfs.h>
+#endif
+
#include <linux/nfs.h>
#include <linux/nfs2.h>
#include <linux/nfs3.h>
@@ -204,14 +208,15 @@ 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;
#endif
+ union {
+ struct inode vfs_inode;
+#ifdef CONFIG_NFS_FSCACHE
+ struct netfs_inode netfs; /* netfs context and VFS inode */
+#endif
+ };
};
struct nfs4_copy_state {
@@ -329,15 +334,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;
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-02-13 15:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 16:16 [PATCH v10 0/6] Convert NFS with fscache to the netfs API Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 1/6] NFS: Rename readpage_async_filler to nfs_read_add_page Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 2/6] NFS: Configure support for netfs when NFS fscache is configured Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 3/6] NFS: Convert buffered read paths to use netfs when fscache is enabled Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 4/6] NFS: Remove all NFSIOS_FSCACHE counters due to conversion to netfs API Dave Wysochanski
2022-11-03 16:16 ` [PATCH v10 5/6] NFS: Remove fscache specific trace points and NFS_INO_FSCACHE bit Dave Wysochanski
2022-11-03 16:16 ` [RFC PATCH v10 6/6] netfs: Change netfs_inode_init to allocate memory to allow opt-in Dave Wysochanski
2022-11-03 16:38 ` David Howells
2023-02-09 14:57 ` [Linux-cachefs] [PATCH v10 0/6] Convert NFS with fscache to the netfs API David Wysochanski
2023-02-09 17:40 ` Trond Myklebust
2023-02-13 15:55 ` David Wysochanski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.