This is a second version of a series posted previously [1]
These ran into conflicts [2] with dhowell's fscache-rewrite patches [3],
and they probably should go in his fscache-rewrite. Note that a couple
patches have been either dropped or folded into other patches, so there's
only 4 patches left from the original 7.
The patches are also at:
https://github.com/DaveWysochanskiRH/kernel/tree/fscache-rewrite-plus-nfs
Changes since v1
- Rebase on top of dhowells fscache-rewrite (v3) and Anna's linux-next to
avoid future conflicts
- Fold in "NFS: Use nfs_i_fscache() consistently within NFS fscache code"
to other patches
- Drop "NFS: Convert NFS fscache enable/disable dfprintks to tracepoints"
since we can use fscache trace events
- Combine the last two patches into one:
NFS: Remove remaining dfprintks related to fscache cookies
NFS: Remove remaining usages of NFSDBG_FSCACHE
- Dropped Signed-off-by and Reviewed-by tags due to rebase
[1] https://marc.info/?l=linux-nfs&m=163718744111509&w=2
[2] https://marc.info/?l=linux-nfs&m=163974120915758&w=2
[3] https://marc.info/?l=linux-nfs&m=163967071213398&w=2
Dave Wysochanski (4):
NFS: Cleanup usage of nfs_inode in fscache interface and handle i_size
properly
NFS: Rename fscache read and write pages functions
NFS: Replace dfprintks with tracepoints in fscache read and write page
functions
NFS: Remove remaining dfprintks related to fscache and remove
NFSDBG_FSCACHE
fs/nfs/fscache.c | 53 +++++++++-----------------
fs/nfs/fscache.h | 45 ++++++++++------------
fs/nfs/nfstrace.h | 91 +++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/read.c | 4 +-
include/uapi/linux/nfs_fs.h | 2 +-
5 files changed, 130 insertions(+), 65 deletions(-)
--
1.8.3.1
The fscache cookie APIs including fscache_acquire_cookie() and
fscache_relinquish_cookie() now have very good tracing. Thus,
there is no real need for dfprintks in the NFS fscache interface.
The NFS fscache interface has removed all dfprintks so remove the
NFSDBG_FSCACHE defines.
Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 10 ----------
include/uapi/linux/nfs_fs.h | 2 +-
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 841b69aef189..4dee53ceb941 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -21,8 +21,6 @@
#include "fscache.h"
#include "nfstrace.h"
-#define NFSDBG_FACILITY NFSDBG_FSCACHE
-
#define NFS_MAX_KEY_LEN 1000
static bool nfs_append_int(char *key, int *_len, unsigned long long x)
@@ -129,8 +127,6 @@ int nfs_fscache_get_super_cookie(struct super_block *sb, const char *uniq, int u
vcookie = fscache_acquire_volume(key,
NULL, /* preferred_cache */
NULL, 0 /* coherency_data */);
- dfprintk(FSCACHE, "NFS: get superblock cookie (0x%p/0x%p)\n",
- nfss, vcookie);
if (IS_ERR(vcookie)) {
if (vcookie != ERR_PTR(-EBUSY)) {
kfree(key);
@@ -153,9 +149,6 @@ void nfs_fscache_release_super_cookie(struct super_block *sb)
{
struct nfs_server *nfss = NFS_SB(sb);
- dfprintk(FSCACHE, "NFS: releasing superblock cookie (0x%p/0x%p)\n",
- nfss, nfss->fscache);
-
fscache_relinquish_volume(nfss->fscache, NULL, false);
nfss->fscache = NULL;
kfree(nfss->fscache_uniq);
@@ -193,8 +186,6 @@ void nfs_fscache_clear_inode(struct inode *inode)
struct nfs_inode *nfsi = NFS_I(inode);
struct fscache_cookie *cookie = nfs_i_fscache(inode);
- dfprintk(FSCACHE, "NFS: clear cookie (0x%p/0x%p)\n", nfsi, cookie);
-
fscache_relinquish_cookie(cookie, false);
nfsi->fscache = NULL;
}
@@ -229,7 +220,6 @@ void nfs_fscache_open_file(struct inode *inode, struct file *filp)
fscache_use_cookie(cookie, open_for_write);
if (open_for_write) {
- dfprintk(FSCACHE, "NFS: nfsi 0x%p disabling cache\n", nfsi);
nfs_fscache_update_auxdata(&auxdata, inode);
fscache_invalidate(cookie, &auxdata, i_size_read(inode),
FSCACHE_INVAL_DIO_WRITE);
diff --git a/include/uapi/linux/nfs_fs.h b/include/uapi/linux/nfs_fs.h
index 3afe3767c55d..ae0de165c014 100644
--- a/include/uapi/linux/nfs_fs.h
+++ b/include/uapi/linux/nfs_fs.h
@@ -52,7 +52,7 @@
#define NFSDBG_CALLBACK 0x0100
#define NFSDBG_CLIENT 0x0200
#define NFSDBG_MOUNT 0x0400
-#define NFSDBG_FSCACHE 0x0800
+#define NFSDBG_FSCACHE 0x0800 /* unused */
#define NFSDBG_PNFS 0x1000
#define NFSDBG_PNFS_LD 0x2000
#define NFSDBG_STATE 0x4000
--
1.8.3.1
Rename NFS fscache functions in a more consistent fashion
to better reflect when we read from and write to fscache.
Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 6 +++---
fs/nfs/fscache.h | 27 ++++++++++-----------------
fs/nfs/read.c | 4 ++--
3 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 81bd2770e640..62fbce28fe85 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -317,7 +317,7 @@ static int fscache_fallback_write_page(struct inode *inode, struct page *page,
/*
* Retrieve a page from fscache
*/
-int __nfs_readpage_from_fscache(struct inode *inode, struct page *page)
+int __nfs_fscache_read_page(struct inode *inode, struct page *page)
{
int ret;
@@ -351,7 +351,7 @@ int __nfs_readpage_from_fscache(struct inode *inode, struct page *page)
* 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_readpage_to_fscache(struct inode *inode, struct page *page)
+void __nfs_fscache_write_page(struct inode *inode, struct page *page)
{
int ret;
@@ -362,7 +362,7 @@ void __nfs_readpage_to_fscache(struct inode *inode, struct page *page)
ret = fscache_fallback_write_page(inode, page, true);
dfprintk(FSCACHE,
- "NFS: readpage_to_fscache: p:%p(i:%lu f:%lx) ret %d\n",
+ "NFS: nfs_fscache_write_page: p:%p(i:%lu f:%lx) ret %d\n",
page, page->index, page->flags, ret);
if (ret != 0) {
diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 5cf7238e4886..ab7962ffe5f0 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -44,10 +44,8 @@ struct nfs_fscache_inode_auxdata {
extern void nfs_fscache_open_file(struct inode *, struct file *);
extern void nfs_fscache_release_file(struct inode *, struct file *);
-extern int __nfs_readpage_from_fscache(struct inode *, struct page *);
-extern void __nfs_read_completion_to_fscache(struct nfs_pgio_header *hdr,
- unsigned long bytes);
-extern void __nfs_readpage_to_fscache(struct inode *, struct page *);
+extern int __nfs_fscache_read_page(struct inode *, struct page *);
+extern void __nfs_fscache_write_page(struct inode *, struct page *);
static inline int nfs_fscache_release_page(struct page *page, gfp_t gfp)
{
@@ -65,11 +63,10 @@ static inline int nfs_fscache_release_page(struct page *page, gfp_t gfp)
/*
* Retrieve a page from an inode data storage object.
*/
-static inline int nfs_readpage_from_fscache(struct inode *inode,
- struct page *page)
+static inline int nfs_fscache_read_page(struct inode *inode, struct page *page)
{
- if (NFS_I(inode)->fscache)
- return __nfs_readpage_from_fscache(inode, page);
+ if (nfs_i_fscache(inode))
+ return __nfs_fscache_read_page(inode, page);
return -ENOBUFS;
}
@@ -77,11 +74,11 @@ static inline int nfs_readpage_from_fscache(struct inode *inode,
* Store a page newly fetched from the server in an inode data storage object
* in the cache.
*/
-static inline void nfs_readpage_to_fscache(struct inode *inode,
+static inline void nfs_fscache_write_page(struct inode *inode,
struct page *page)
{
- if (NFS_I(inode)->fscache)
- __nfs_readpage_to_fscache(inode, page);
+ if (nfs_i_fscache(inode))
+ __nfs_fscache_write_page(inode, page);
}
static inline void nfs_fscache_update_auxdata(struct nfs_fscache_inode_auxdata *auxdata,
@@ -135,15 +132,11 @@ static inline int nfs_fscache_release_page(struct page *page, gfp_t gfp)
{
return 1; /* True: may release page */
}
-static inline int nfs_readpage_from_fscache(struct inode *inode,
- struct page *page)
+static inline int nfs_fscache_read_page(struct inode *inode, struct page *page)
{
return -ENOBUFS;
}
-static inline void nfs_readpage_to_fscache(struct inode *inode,
- struct page *page) {}
-
-
+static inline void nfs_fscache_write_page(struct inode *inode, struct page *page) {}
static inline void nfs_fscache_invalidate(struct inode *inode, int flags) {}
static inline const char *nfs_server_fscache_state(struct nfs_server *server)
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index eb00229c1a50..f84b6b73c45b 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -123,7 +123,7 @@ static void nfs_readpage_release(struct nfs_page *req, int error)
struct address_space *mapping = page_file_mapping(page);
if (PageUptodate(page))
- nfs_readpage_to_fscache(inode, page);
+ nfs_fscache_write_page(inode, page);
else if (!PageError(page) && !PagePrivate(page))
generic_error_remove_page(mapping, page);
unlock_page(page);
@@ -306,7 +306,7 @@ static void nfs_readpage_result(struct rpc_task *task,
aligned_len = min_t(unsigned int, ALIGN(len, rsize), PAGE_SIZE);
if (!IS_SYNC(page->mapping->host)) {
- error = nfs_readpage_from_fscache(page->mapping->host, page);
+ error = nfs_fscache_read_page(page->mapping->host, page);
if (error == 0)
goto out_unlock;
}
--
1.8.3.1
Most of fscache and other NFS IO paths are now using tracepoints.
Remove the dfprintks in the NFS fscache read/write page functions
and replace with tracepoints at the begin and end of the functions.
Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 29 +++++++-----------
fs/nfs/nfstrace.h | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 102 insertions(+), 18 deletions(-)
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 62fbce28fe85..841b69aef189 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -19,6 +19,7 @@
#include "internal.h"
#include "iostat.h"
#include "fscache.h"
+#include "nfstrace.h"
#define NFSDBG_FACILITY NFSDBG_FSCACHE
@@ -321,30 +322,27 @@ int __nfs_fscache_read_page(struct inode *inode, struct page *page)
{
int ret;
- dfprintk(FSCACHE,
- "NFS: readpage_from_fscache(fsc:%p/p:%p(i:%lx f:%lx)/0x%p)\n",
- nfs_i_fscache(inode), page, page->index, page->flags, inode);
-
+ trace_nfs_fscache_read_page(inode, page);
if (PageChecked(page)) {
- dfprintk(FSCACHE, "NFS: readpage_from_fscache: PageChecked\n");
ClearPageChecked(page);
- return 1;
+ ret = 1;
+ goto out;
}
ret = fscache_fallback_read_page(inode, page);
if (ret < 0) {
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
- dfprintk(FSCACHE,
- "NFS: readpage_from_fscache failed %d\n", ret);
SetPageChecked(page);
- return ret;
+ goto out;
}
/* Read completed synchronously */
- dfprintk(FSCACHE, "NFS: readpage_from_fscache: read successful\n");
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
SetPageUptodate(page);
- return 0;
+ ret = 0;
+out:
+ trace_nfs_fscache_read_page_exit(inode, page, ret);
+ return ret;
}
/*
@@ -355,20 +353,15 @@ void __nfs_fscache_write_page(struct inode *inode, struct page *page)
{
int ret;
- dfprintk(FSCACHE,
- "NFS: readpage_to_fscache(fsc:%p/p:%p(i:%lx f:%lx))\n",
- nfs_i_fscache(inode), page, page->index, page->flags);
+ trace_nfs_fscache_write_page(inode, page);
ret = fscache_fallback_write_page(inode, page, true);
- dfprintk(FSCACHE,
- "NFS: nfs_fscache_write_page: p:%p(i:%lu f:%lx) ret %d\n",
- page, page->index, page->flags, ret);
-
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);
}
+ trace_nfs_fscache_write_page_exit(inode, page, ret);
}
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 317ce27bdc4b..f4d335c22113 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -1095,6 +1095,97 @@
)
);
+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(
const struct nfs_pgio_header *hdr,
--
1.8.3.1
Dave Wysochanski <[email protected]> wrote:
> Rename NFS fscache functions in a more consistent fashion
> to better reflect when we read from and write to fscache.
Do you want me to merge this into my patch that rewrites the nfs cache I/O?
David
On Mon, Dec 20, 2021 at 12:30 PM David Howells <[email protected]> wrote:
>
> Dave Wysochanski <[email protected]> wrote:
>
> > Rename NFS fscache functions in a more consistent fashion
> > to better reflect when we read from and write to fscache.
>
> Do you want me to merge this into my patch that rewrites the nfs cache I/O?
>
Yes, I think it makes sense to merge this patch with the following
patch from your series:
[PATCH v3 64/68] nfs: Implement cache I/O by accessing the cache directly
FYI, these apply cleanly on top of linus tree now after the merge of
dhowells fscache-rewrite
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8834147f9505661859ce44549bf601e2a06bba7c
I have rebased (no changes needed from the Dec post), did some final
tests (unit and xfstests) and re-pushed to
https://github.com/DaveWysochanskiRH/kernel/tree/nfs-fscache-5.17-rc1
I am not sure if it is too late for the merge window or not, or how
you handle such dependencies,
but wanted to let you know my status. If you want me to re-post or
anything let me know.
Thanks.
---------- Forwarded message ---------
From: Dave Wysochanski <[email protected]>
Date: Fri, Dec 17, 2021 at 12:54 PM
Subject: [PATCH v2 0/4] Cleanups for NFS fscache and convert from
dfprintk to trace events
To: Anna Schumaker <[email protected]>, David Howells
<[email protected]>
Cc: <[email protected]>, <[email protected]>
This is a second version of a series posted previously [1]
These ran into conflicts [2] with dhowell's fscache-rewrite patches [3],
and they probably should go in his fscache-rewrite. Note that a couple
patches have been either dropped or folded into other patches, so there's
only 4 patches left from the original 7.
The patches are also at:
https://github.com/DaveWysochanskiRH/kernel/tree/fscache-rewrite-plus-nfs
Changes since v1
- Rebase on top of dhowells fscache-rewrite (v3) and Anna's linux-next to
avoid future conflicts
- Fold in "NFS: Use nfs_i_fscache() consistently within NFS fscache code"
to other patches
- Drop "NFS: Convert NFS fscache enable/disable dfprintks to tracepoints"
since we can use fscache trace events
- Combine the last two patches into one:
NFS: Remove remaining dfprintks related to fscache cookies
NFS: Remove remaining usages of NFSDBG_FSCACHE
- Dropped Signed-off-by and Reviewed-by tags due to rebase
[1] https://marc.info/?l=linux-nfs&m=163718744111509&w=2
[2] https://marc.info/?l=linux-nfs&m=163974120915758&w=2
[3] https://marc.info/?l=linux-nfs&m=163967071213398&w=2
Dave Wysochanski (4):
NFS: Cleanup usage of nfs_inode in fscache interface and handle i_size
properly
NFS: Rename fscache read and write pages functions
NFS: Replace dfprintks with tracepoints in fscache read and write page
functions
NFS: Remove remaining dfprintks related to fscache and remove
NFSDBG_FSCACHE
fs/nfs/fscache.c | 53 +++++++++-----------------
fs/nfs/fscache.h | 45 ++++++++++------------
fs/nfs/nfstrace.h | 91 +++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/read.c | 4 +-
include/uapi/linux/nfs_fs.h | 2 +-
5 files changed, 130 insertions(+), 65 deletions(-)
--
1.8.3.1