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/[email protected]/
[2] https://lore.kernel.org/linux-nfs/[email protected]/
[3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
[4] https://www.mail-archive.com/[email protected]/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
The NFS specific trace points are no longer needed as tracing is well
covered by netfs and fscache.
Signed-off-by: Dave Wysochanski <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
---
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
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 <[email protected]>
---
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
On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski <[email protected]> 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/[email protected]/
> [2] https://lore.kernel.org/linux-nfs/[email protected]/
> [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> [4] https://www.mail-archive.com/[email protected]/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
> [email protected]
> 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
On Thu, 2023-02-09 at 09:57 -0500, David Wysochanski wrote:
> On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski
> <[email protected]> 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/[email protected]/
> > [2]
> > https://lore.kernel.org/linux-nfs/[email protected]/
> > [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> > [4]
> > https://www.mail-archive.com/[email protected]/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
> > [email protected]
> > 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
[email protected]
On Thu, Feb 9, 2023 at 12:41 PM Trond Myklebust <[email protected]> wrote:
>
> On Thu, 2023-02-09 at 09:57 -0500, David Wysochanski wrote:
> > On Thu, Nov 3, 2022 at 12:16 PM Dave Wysochanski
> > <[email protected]> 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/[email protected]/
> > > [2]
> > > https://lore.kernel.org/linux-nfs/[email protected]/
> > > [3] https://marc.info/?l=linux-nfs&m=160597917525083&w=4
> > > [4]
> > > https://www.mail-archive.com/[email protected]/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
> > > [email protected]
> > > 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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;