2021-11-17 22:17:35

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 0/7] Cleanups for NFS fscache and convert from dfprintk to trace events

The first 3 patches are cleanups and refactorings of the NFS fscache code.
The last 4 patches convert dfprintks to trace events in the NFS fscache code.
These patches were built / tested on 5.16.0-rc1.

Dave Wysochanski (7):
NFS: Use nfs_i_fscache() consistently within NFS fscache code
NFS: Cleanup usage of nfs_inode in fscache interface and handle i_size
properly
NFS: Rename fscache read and write pages functions
NFS: Convert NFS fscache enable/disable dfprintks to tracepoints
NFS: Replace dfprintks with tracepoints in fscache read and write page
functions
NFS: Remove remaining dfprintks related to fscache cookies
NFS: Remove remaining usages of NFSDBG_FSCACHE

fs/nfs/fscache-index.c | 2 -
fs/nfs/fscache.c | 106 ++++++++++++++++----------------------------
fs/nfs/fscache.h | 33 +++++++-------
fs/nfs/nfstrace.h | 103 ++++++++++++++++++++++++++++++++++++++++++
fs/nfs/read.c | 6 +--
include/uapi/linux/nfs_fs.h | 2 +-
6 files changed, 162 insertions(+), 90 deletions(-)

--
1.8.3.1



2021-11-17 22:17:33

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 1/7] NFS: Use nfs_i_fscache() consistently within NFS fscache code

The nfs_i_fscache() is the API defined to check whether fscache
is enabled on an NFS inode or not, so use it consistently through
the code.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index 6754c8607230..840608a38713 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -131,7 +131,7 @@ static inline int nfs_readpage_from_fscache(struct nfs_open_context *ctx,
struct inode *inode,
struct page *page)
{
- if (NFS_I(inode)->fscache)
+ if (nfs_i_fscache(inode))
return __nfs_readpage_from_fscache(ctx, inode, page);
return -ENOBUFS;
}
@@ -145,7 +145,7 @@ static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
struct list_head *pages,
unsigned *nr_pages)
{
- if (NFS_I(inode)->fscache)
+ if (nfs_i_fscache(inode))
return __nfs_readpages_from_fscache(ctx, inode, mapping, pages,
nr_pages);
return -ENOBUFS;
@@ -168,7 +168,7 @@ static inline void nfs_readpage_to_fscache(struct inode *inode,
*/
static inline void nfs_fscache_invalidate(struct inode *inode)
{
- fscache_invalidate(NFS_I(inode)->fscache);
+ fscache_invalidate(nfs_i_fscache(inode));
}

/*
@@ -176,7 +176,7 @@ static inline void nfs_fscache_invalidate(struct inode *inode)
*/
static inline void nfs_fscache_wait_on_invalidate(struct inode *inode)
{
- fscache_wait_on_invalidate(NFS_I(inode)->fscache);
+ fscache_wait_on_invalidate(nfs_i_fscache(inode));
}

/*
--
1.8.3.1


2021-11-17 22:17:36

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 4/7] NFS: Convert NFS fscache enable/disable dfprintks to tracepoints

Convert the enable / disable NFS fscache dfprintks to tracepoints.
Utilize the existing NFS inode trace event class, which allows us
to keep the same output format to other NFS inode tracepoints.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 5 +++--
fs/nfs/nfstrace.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index cb701d9c4e47..913a29f2b9e4 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

@@ -314,12 +315,12 @@ void nfs_fscache_open_file(struct inode *inode, struct file *filp)
nfs_fscache_update_auxdata(&auxdata, inode);

if (inode_is_open_for_write(inode)) {
- dfprintk(FSCACHE, "NFS: nfsi 0x%p disabling cache\n", nfsi);
+ trace_nfs_fscache_disable_inode(inode);
clear_bit(NFS_INO_FSCACHE, &nfsi->flags);
fscache_disable_cookie(cookie, &auxdata, true);
fscache_uncache_all_inode_pages(cookie, inode);
} else {
- dfprintk(FSCACHE, "NFS: nfsi 0x%p enabling cache\n", nfsi);
+ trace_nfs_fscache_enable_inode(inode);
fscache_enable_cookie(cookie, &auxdata, i_size_read(inode),
nfs_fscache_can_enable, inode);
if (fscache_cookie_enabled(cookie))
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 21dac847f1e4..2da68adda88f 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -144,6 +144,8 @@
int error \
), \
TP_ARGS(inode, error))
+DEFINE_NFS_INODE_EVENT(nfs_fscache_enable_inode);
+DEFINE_NFS_INODE_EVENT(nfs_fscache_disable_inode);
DEFINE_NFS_INODE_EVENT(nfs_set_inode_stale);
DEFINE_NFS_INODE_EVENT(nfs_refresh_inode_enter);
DEFINE_NFS_INODE_EVENT_DONE(nfs_refresh_inode_exit);
--
1.8.3.1


2021-11-17 22:17:37

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 2/7] NFS: Cleanup usage of nfs_inode in fscache interface and handle i_size properly

A number of places in the fscache interface used nfs_inode when inode could
be used, simplifying the code. Also, handle the read of i_size properly by
utilizing the i_size_read() interface.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index d743629e05e1..ebc91e4b7655 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -226,16 +226,16 @@ void nfs_fscache_release_super_cookie(struct super_block *sb)
}

static void nfs_fscache_update_auxdata(struct nfs_fscache_inode_auxdata *auxdata,
- struct nfs_inode *nfsi)
+ struct inode *inode)
{
memset(auxdata, 0, sizeof(*auxdata));
- auxdata->mtime_sec = nfsi->vfs_inode.i_mtime.tv_sec;
- auxdata->mtime_nsec = nfsi->vfs_inode.i_mtime.tv_nsec;
- auxdata->ctime_sec = nfsi->vfs_inode.i_ctime.tv_sec;
- auxdata->ctime_nsec = nfsi->vfs_inode.i_ctime.tv_nsec;
+ auxdata->mtime_sec = inode->i_mtime.tv_sec;
+ auxdata->mtime_nsec = inode->i_mtime.tv_nsec;
+ auxdata->ctime_sec = inode->i_ctime.tv_sec;
+ auxdata->ctime_nsec = inode->i_ctime.tv_nsec;

- if (NFS_SERVER(&nfsi->vfs_inode)->nfs_client->rpc_ops->version == 4)
- auxdata->change_attr = inode_peek_iversion_raw(&nfsi->vfs_inode);
+ if (NFS_SERVER(inode)->nfs_client->rpc_ops->version == 4)
+ auxdata->change_attr = inode_peek_iversion_raw(inode);
}

/*
@@ -251,13 +251,13 @@ void nfs_fscache_init_inode(struct inode *inode)
if (!(nfss->fscache && S_ISREG(inode->i_mode)))
return;

- nfs_fscache_update_auxdata(&auxdata, nfsi);
+ nfs_fscache_update_auxdata(&auxdata, inode);

nfsi->fscache = fscache_acquire_cookie(NFS_SB(inode->i_sb)->fscache,
&nfs_fscache_inode_object_def,
nfsi->fh.data, nfsi->fh.size,
&auxdata, sizeof(auxdata),
- nfsi, nfsi->vfs_inode.i_size, false);
+ nfsi, i_size_read(inode), false);
}

/*
@@ -271,7 +271,7 @@ void nfs_fscache_clear_inode(struct inode *inode)

dfprintk(FSCACHE, "NFS: clear cookie (0x%p/0x%p)\n", nfsi, cookie);

- nfs_fscache_update_auxdata(&auxdata, nfsi);
+ nfs_fscache_update_auxdata(&auxdata, inode);
fscache_relinquish_cookie(cookie, &auxdata, false);
nfsi->fscache = NULL;
}
@@ -311,7 +311,7 @@ void nfs_fscache_open_file(struct inode *inode, struct file *filp)
if (!fscache_cookie_valid(cookie))
return;

- nfs_fscache_update_auxdata(&auxdata, nfsi);
+ nfs_fscache_update_auxdata(&auxdata, inode);

if (inode_is_open_for_write(inode)) {
dfprintk(FSCACHE, "NFS: nfsi 0x%p disabling cache\n", nfsi);
@@ -320,7 +320,7 @@ void nfs_fscache_open_file(struct inode *inode, struct file *filp)
fscache_uncache_all_inode_pages(cookie, inode);
} else {
dfprintk(FSCACHE, "NFS: nfsi 0x%p enabling cache\n", nfsi);
- fscache_enable_cookie(cookie, &auxdata, nfsi->vfs_inode.i_size,
+ fscache_enable_cookie(cookie, &auxdata, i_size_read(inode),
nfs_fscache_can_enable, inode);
if (fscache_cookie_enabled(cookie))
set_bit(NFS_INO_FSCACHE, &NFS_I(inode)->flags);
--
1.8.3.1


2021-11-17 22:17:39

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 5/7] NFS: Replace dfprintks with tracepoints in fscache read and write page functions

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 | 51 ++++++++++-----------------
fs/nfs/nfstrace.h | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 913a29f2b9e4..f4147a7915bd 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -382,9 +382,7 @@ static void nfs_fscache_read_page_complete(struct page *page,
void *context,
int error)
{
- dfprintk(FSCACHE,
- "NFS: fscache_read_page_complete (0x%p/0x%p/%d)\n",
- page, context, error);
+ trace_nfs_fscache_read_page_complete(page->mapping->host, page, 1, error);

/*
* If the read completes with an error, mark the page with PG_checked,
@@ -405,13 +403,11 @@ int __nfs_fscache_read_page(struct nfs_open_context *ctx,
{
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_pages(inode, page, 1);
if (PageChecked(page)) {
ClearPageChecked(page);
- return 1;
+ ret = 1;
+ goto out;
}

ret = fscache_read_or_alloc_page(nfs_i_fscache(inode),
@@ -422,22 +418,21 @@ int __nfs_fscache_read_page(struct nfs_open_context *ctx,

switch (ret) {
case 0: /* read BIO submitted (page in fscache) */
- dfprintk(FSCACHE,
- "NFS: fscache_read_page: BIO submitted\n");
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
- return ret;
+ break;

case -ENOBUFS: /* inode not in cache */
case -ENODATA: /* page not in cache */
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
- dfprintk(FSCACHE,
- "NFS: fscache_read_page %d\n", ret);
- return 1;
+ ret = 1;
+ break;

default:
- dfprintk(FSCACHE, "NFS: fscache_read_page %d\n", ret);
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
}
+
+out:
+ trace_nfs_fscache_read_pages_exit(inode, page, 1, ret);
return ret;
}

@@ -453,8 +448,7 @@ int __nfs_fscache_read_pages(struct nfs_open_context *ctx,
unsigned npages = *nr_pages;
int ret;

- dfprintk(FSCACHE, "NFS: fscache_read_pages (0x%p/%u/0x%p)\n",
- nfs_i_fscache(inode), npages, inode);
+ trace_nfs_fscache_read_pages(inode, lru_to_page(pages), *nr_pages);

ret = fscache_read_or_alloc_pages(nfs_i_fscache(inode),
mapping, pages, nr_pages,
@@ -472,22 +466,18 @@ int __nfs_fscache_read_pages(struct nfs_open_context *ctx,
case 0: /* read submitted to the cache for all pages */
BUG_ON(!list_empty(pages));
BUG_ON(*nr_pages != 0);
- dfprintk(FSCACHE,
- "NFS: fscache_read_pages: submitted\n");
-
- return ret;
+ break;

case -ENOBUFS: /* some pages aren't cached and can't be */
case -ENODATA: /* some pages aren't cached */
- dfprintk(FSCACHE,
- "NFS: fscache_read_pages: no page: %d\n", ret);
- return 1;
+ ret = 1;
+ break;

default:
- dfprintk(FSCACHE,
- "NFS: fscache_read_pages: ret %d\n", ret);
+ ;
}

+ trace_nfs_fscache_read_pages_exit(inode, lru_to_page(pages), *nr_pages, ret);
return ret;
}

@@ -499,16 +489,10 @@ void __nfs_fscache_write_page(struct inode *inode, struct page *page, int sync)
{
int ret;

- dfprintk(FSCACHE,
- "NFS: fscache_write_page(fsc:%p/p:%p(i:%lx f:%lx)/%d)\n",
- nfs_i_fscache(inode), page, page->index, page->flags, sync);
+ trace_nfs_fscache_write_page(inode, page, 1);

ret = fscache_write_page(nfs_i_fscache(inode), page,
inode->i_size, GFP_KERNEL);
- 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) {
fscache_uncache_page(nfs_i_fscache(inode), page);
nfs_inc_fscache_stats(inode,
@@ -518,4 +502,5 @@ void __nfs_fscache_write_page(struct inode *inode, struct page *page, int sync)
nfs_inc_fscache_stats(inode,
NFSIOS_FSCACHE_PAGES_WRITTEN_OK);
}
+ trace_nfs_fscache_write_page_exit(inode, page, 1, ret);
}
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 2da68adda88f..62c78e6b4582 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -1097,6 +1097,107 @@
)
);

+DECLARE_EVENT_CLASS(nfs_fscache_page_event,
+ TP_PROTO(
+ const struct inode *inode,
+ struct page *page,
+ unsigned int nr_pages
+ ),
+
+ TP_ARGS(inode, page, nr_pages),
+
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(u32, fhandle)
+ __field(u64, fileid)
+ __field(loff_t, offset)
+ __field(u32, count)
+ ),
+
+ 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->count = PAGE_SIZE*nr_pages;
+ __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 count=%u",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ (unsigned long long)__entry->fileid,
+ __entry->fhandle,
+ (long long)__entry->offset, __entry->count
+ )
+);
+DECLARE_EVENT_CLASS(nfs_fscache_page_event_done,
+ TP_PROTO(
+ const struct inode *inode,
+ struct page *page,
+ unsigned int nr_pages,
+ int error
+ ),
+
+ TP_ARGS(inode, page, nr_pages, error),
+
+ TP_STRUCT__entry(
+ __field(int, error)
+ __field(dev_t, dev)
+ __field(u32, fhandle)
+ __field(u64, fileid)
+ __field(loff_t, offset)
+ __field(u32, count)
+ ),
+
+ 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->count = PAGE_SIZE*nr_pages;
+ __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 count=%u error=%d",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ (unsigned long long)__entry->fileid,
+ __entry->fhandle,
+ (long long)__entry->offset, __entry->count,
+ __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, \
+ unsigned int nr_pages \
+ ), \
+ TP_ARGS(inode, page, nr_pages))
+#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, \
+ unsigned int nr_pages, \
+ int error \
+ ), \
+ TP_ARGS(inode, page, nr_pages, error))
+DEFINE_NFS_FSCACHE_PAGE_EVENT(nfs_fscache_read_pages);
+DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(nfs_fscache_read_pages_exit);
+DEFINE_NFS_FSCACHE_PAGE_EVENT(nfs_fscache_write_page);
+DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(nfs_fscache_write_page_exit);
+DEFINE_NFS_FSCACHE_PAGE_EVENT_DONE(nfs_fscache_read_page_complete);
+
TRACE_EVENT(nfs_pgio_error,
TP_PROTO(
const struct nfs_pgio_header *hdr,
--
1.8.3.1


2021-11-17 22:17:39

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 3/7] NFS: Rename fscache read and write pages functions

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 | 32 ++++++++++++++++----------------
fs/nfs/fscache.h | 25 ++++++++++++-------------
fs/nfs/read.c | 6 +++---
3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index ebc91e4b7655..cb701d9c4e47 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -377,12 +377,12 @@ void __nfs_fscache_invalidate_page(struct page *page, struct inode *inode)
* Handle completion of a page being read from the cache.
* - Called in process (keventd) context.
*/
-static void nfs_readpage_from_fscache_complete(struct page *page,
+static void nfs_fscache_read_page_complete(struct page *page,
void *context,
int error)
{
dfprintk(FSCACHE,
- "NFS: readpage_from_fscache_complete (0x%p/0x%p/%d)\n",
+ "NFS: fscache_read_page_complete (0x%p/0x%p/%d)\n",
page, context, error);

/*
@@ -399,7 +399,7 @@ static void nfs_readpage_from_fscache_complete(struct page *page,
/*
* Retrieve a page from fscache
*/
-int __nfs_readpage_from_fscache(struct nfs_open_context *ctx,
+int __nfs_fscache_read_page(struct nfs_open_context *ctx,
struct inode *inode, struct page *page)
{
int ret;
@@ -415,14 +415,14 @@ int __nfs_readpage_from_fscache(struct nfs_open_context *ctx,

ret = fscache_read_or_alloc_page(nfs_i_fscache(inode),
page,
- nfs_readpage_from_fscache_complete,
+ nfs_fscache_read_page_complete,
ctx,
GFP_KERNEL);

switch (ret) {
case 0: /* read BIO submitted (page in fscache) */
dfprintk(FSCACHE,
- "NFS: readpage_from_fscache: BIO submitted\n");
+ "NFS: fscache_read_page: BIO submitted\n");
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_OK);
return ret;

@@ -430,11 +430,11 @@ int __nfs_readpage_from_fscache(struct nfs_open_context *ctx,
case -ENODATA: /* page not in cache */
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
dfprintk(FSCACHE,
- "NFS: readpage_from_fscache %d\n", ret);
+ "NFS: fscache_read_page %d\n", ret);
return 1;

default:
- dfprintk(FSCACHE, "NFS: readpage_from_fscache %d\n", ret);
+ dfprintk(FSCACHE, "NFS: fscache_read_page %d\n", ret);
nfs_inc_fscache_stats(inode, NFSIOS_FSCACHE_PAGES_READ_FAIL);
}
return ret;
@@ -443,7 +443,7 @@ int __nfs_readpage_from_fscache(struct nfs_open_context *ctx,
/*
* Retrieve a set of pages from fscache
*/
-int __nfs_readpages_from_fscache(struct nfs_open_context *ctx,
+int __nfs_fscache_read_pages(struct nfs_open_context *ctx,
struct inode *inode,
struct address_space *mapping,
struct list_head *pages,
@@ -452,12 +452,12 @@ int __nfs_readpages_from_fscache(struct nfs_open_context *ctx,
unsigned npages = *nr_pages;
int ret;

- dfprintk(FSCACHE, "NFS: nfs_getpages_from_fscache (0x%p/%u/0x%p)\n",
+ dfprintk(FSCACHE, "NFS: fscache_read_pages (0x%p/%u/0x%p)\n",
nfs_i_fscache(inode), npages, inode);

ret = fscache_read_or_alloc_pages(nfs_i_fscache(inode),
mapping, pages, nr_pages,
- nfs_readpage_from_fscache_complete,
+ nfs_fscache_read_page_complete,
ctx,
mapping_gfp_mask(mapping));
if (*nr_pages < npages)
@@ -472,19 +472,19 @@ int __nfs_readpages_from_fscache(struct nfs_open_context *ctx,
BUG_ON(!list_empty(pages));
BUG_ON(*nr_pages != 0);
dfprintk(FSCACHE,
- "NFS: nfs_getpages_from_fscache: submitted\n");
+ "NFS: fscache_read_pages: submitted\n");

return ret;

case -ENOBUFS: /* some pages aren't cached and can't be */
case -ENODATA: /* some pages aren't cached */
dfprintk(FSCACHE,
- "NFS: nfs_getpages_from_fscache: no page: %d\n", ret);
+ "NFS: fscache_read_pages: no page: %d\n", ret);
return 1;

default:
dfprintk(FSCACHE,
- "NFS: nfs_getpages_from_fscache: ret %d\n", ret);
+ "NFS: fscache_read_pages: ret %d\n", ret);
}

return ret;
@@ -494,18 +494,18 @@ int __nfs_readpages_from_fscache(struct nfs_open_context *ctx,
* Store a newly fetched page in fscache
* - PG_fscache must be set on the page
*/
-void __nfs_readpage_to_fscache(struct inode *inode, struct page *page, int sync)
+void __nfs_fscache_write_page(struct inode *inode, struct page *page, int sync)
{
int ret;

dfprintk(FSCACHE,
- "NFS: readpage_to_fscache(fsc:%p/p:%p(i:%lx f:%lx)/%d)\n",
+ "NFS: fscache_write_page(fsc:%p/p:%p(i:%lx f:%lx)/%d)\n",
nfs_i_fscache(inode), page, page->index, page->flags, sync);

ret = fscache_write_page(nfs_i_fscache(inode), page,
inode->i_size, GFP_KERNEL);
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 840608a38713..876edc4a5f37 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -96,12 +96,12 @@ struct nfs_fscache_inode_auxdata {
extern void __nfs_fscache_invalidate_page(struct page *, struct inode *);
extern int nfs_fscache_release_page(struct page *, gfp_t);

-extern int __nfs_readpage_from_fscache(struct nfs_open_context *,
+extern int __nfs_fscache_read_page(struct nfs_open_context *,
struct inode *, struct page *);
-extern int __nfs_readpages_from_fscache(struct nfs_open_context *,
+extern int __nfs_fscache_read_pages(struct nfs_open_context *,
struct inode *, struct address_space *,
struct list_head *, unsigned *);
-extern void __nfs_readpage_to_fscache(struct inode *, struct page *, int);
+extern void __nfs_fscache_write_page(struct inode *, struct page *, int);

/*
* wait for a page to complete writing to the cache
@@ -127,26 +127,26 @@ static inline void nfs_fscache_invalidate_page(struct page *page,
/*
* Retrieve a page from an inode data storage object.
*/
-static inline int nfs_readpage_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_fscache_read_page(struct nfs_open_context *ctx,
struct inode *inode,
struct page *page)
{
if (nfs_i_fscache(inode))
- return __nfs_readpage_from_fscache(ctx, inode, page);
+ return __nfs_fscache_read_page(ctx, inode, page);
return -ENOBUFS;
}

/*
* Retrieve a set of pages from an inode data storage object.
*/
-static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_fscache_read_pages(struct nfs_open_context *ctx,
struct inode *inode,
struct address_space *mapping,
struct list_head *pages,
unsigned *nr_pages)
{
if (nfs_i_fscache(inode))
- return __nfs_readpages_from_fscache(ctx, inode, mapping, pages,
+ return __nfs_fscache_read_pages(ctx, inode, mapping, pages,
nr_pages);
return -ENOBUFS;
}
@@ -155,12 +155,12 @@ static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
* 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,
int sync)
{
if (PageFsCache(page))
- __nfs_readpage_to_fscache(inode, page, sync);
+ __nfs_fscache_write_page(inode, page, sync);
}

/*
@@ -212,13 +212,13 @@ static inline void nfs_fscache_invalidate_page(struct page *page,
static inline void nfs_fscache_wait_on_page_write(struct nfs_inode *nfsi,
struct page *page) {}

-static inline int nfs_readpage_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_fscache_read_page(struct nfs_open_context *ctx,
struct inode *inode,
struct page *page)
{
return -ENOBUFS;
}
-static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
+static inline int nfs_fscache_read_pages(struct nfs_open_context *ctx,
struct inode *inode,
struct address_space *mapping,
struct list_head *pages,
@@ -226,10 +226,9 @@ static inline int nfs_readpages_from_fscache(struct nfs_open_context *ctx,
{
return -ENOBUFS;
}
-static inline void nfs_readpage_to_fscache(struct inode *inode,
+static inline void nfs_fscache_write_page(struct inode *inode,
struct page *page, int sync) {}

-
static inline void nfs_fscache_invalidate(struct inode *inode) {}
static inline void nfs_fscache_wait_on_invalidate(struct inode *inode) {}

diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index d11af2a9299c..7e8d80ba56a5 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, 0);
+ nfs_fscache_write_page(inode, page, 0);
else if (!PageError(page) && !PagePrivate(page))
generic_error_remove_page(mapping, page);
unlock_page(page);
@@ -367,7 +367,7 @@ int nfs_readpage(struct file *file, struct page *page)

xchg(&desc.ctx->error, 0);
if (!IS_SYNC(inode)) {
- ret = nfs_readpage_from_fscache(desc.ctx, inode, page);
+ ret = nfs_fscache_read_page(desc.ctx, inode, page);
if (ret == 0)
goto out_wait;
}
@@ -422,7 +422,7 @@ int nfs_readpages(struct file *file, struct address_space *mapping,
/* attempt to read as many of the pages as possible from the cache
* - this returns -ENOBUFS immediately if the cookie is negative
*/
- ret = nfs_readpages_from_fscache(desc.ctx, inode, mapping,
+ ret = nfs_fscache_read_pages(desc.ctx, inode, mapping,
pages, &nr_pages);
if (ret == 0)
goto read_complete; /* all pages were read */
--
1.8.3.1


2021-11-17 22:17:40

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 7/7] NFS: Remove remaining usages of NFSDBG_FSCACHE

The NFS fscache interface has removed all dfprintks so remove the
NFSDBG_FSCACHE defines.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache-index.c | 2 --
fs/nfs/fscache.c | 2 --
include/uapi/linux/nfs_fs.h | 2 +-
3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/nfs/fscache-index.c b/fs/nfs/fscache-index.c
index 573b1da9342c..4205fb031fae 100644
--- a/fs/nfs/fscache-index.c
+++ b/fs/nfs/fscache-index.c
@@ -17,8 +17,6 @@
#include "internal.h"
#include "fscache.h"

-#define NFSDBG_FACILITY NFSDBG_FSCACHE
-
/*
* Define the NFS filesystem for FS-Cache. Upon registration FS-Cache sticks
* the cookie for the top-level index object for NFS into here. The top-level
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index e62629feeebf..e21ab23fa8d0 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
-
static struct rb_root nfs_fscache_keys = RB_ROOT;
static DEFINE_SPINLOCK(nfs_fscache_keys_lock);

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


2021-11-17 22:17:40

by David Wysochanski

[permalink] [raw]
Subject: [PATCH 6/7] NFS: Remove remaining dfprintks related to fscache cookies

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.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index f4147a7915bd..e62629feeebf 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -21,7 +21,7 @@
#include "fscache.h"
#include "nfstrace.h"

-#define NFSDBG_FACILITY NFSDBG_FSCACHE
+#define NFSDBG_FACILITY NFSDBG_FSCACHE

static struct rb_root nfs_fscache_keys = RB_ROOT;
static DEFINE_SPINLOCK(nfs_fscache_keys_lock);
@@ -86,8 +86,6 @@ void nfs_fscache_get_client_cookie(struct nfs_client *clp)
&key, len,
NULL, 0,
clp, 0, true);
- dfprintk(FSCACHE, "NFS: get client cookie (0x%p/0x%p)\n",
- clp, clp->fscache);
}

/*
@@ -95,9 +93,6 @@ void nfs_fscache_get_client_cookie(struct nfs_client *clp)
*/
void nfs_fscache_release_client_cookie(struct nfs_client *clp)
{
- dfprintk(FSCACHE, "NFS: releasing client cookie (0x%p/0x%p)\n",
- clp, clp->fscache);
-
fscache_relinquish_cookie(clp->fscache, NULL, false);
clp->fscache = NULL;
}
@@ -191,8 +186,6 @@ void nfs_fscache_get_super_cookie(struct super_block *sb, const char *uniq, int
sizeof(key->key) + ulen,
NULL, 0,
nfss, 0, true);
- dfprintk(FSCACHE, "NFS: get superblock cookie (0x%p/0x%p)\n",
- nfss, nfss->fscache);
return;

non_unique:
@@ -211,9 +204,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_cookie(nfss->fscache, NULL, false);
nfss->fscache = NULL;

@@ -270,8 +260,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);
-
nfs_fscache_update_auxdata(&auxdata, inode);
fscache_relinquish_cookie(cookie, &auxdata, false);
nfsi->fscache = NULL;
--
1.8.3.1


2021-11-18 16:27:26

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 0/7] Cleanups for NFS fscache and convert from dfprintk to trace events



> On Nov 17, 2021, at 5:17 PM, Dave Wysochanski <[email protected]> wrote:
>
> The first 3 patches are cleanups and refactorings of the NFS fscache code.
> The last 4 patches convert dfprintks to trace events in the NFS fscache code.
> These patches were built / tested on 5.16.0-rc1.
>
> Dave Wysochanski (7):
> NFS: Use nfs_i_fscache() consistently within NFS fscache code
> NFS: Cleanup usage of nfs_inode in fscache interface and handle i_size
> properly
> NFS: Rename fscache read and write pages functions
> NFS: Convert NFS fscache enable/disable dfprintks to tracepoints
> NFS: Replace dfprintks with tracepoints in fscache read and write page
> functions
> NFS: Remove remaining dfprintks related to fscache cookies
> NFS: Remove remaining usages of NFSDBG_FSCACHE
>
> fs/nfs/fscache-index.c | 2 -
> fs/nfs/fscache.c | 106 ++++++++++++++++----------------------------
> fs/nfs/fscache.h | 33 +++++++-------
> fs/nfs/nfstrace.h | 103 ++++++++++++++++++++++++++++++++++++++++++
> fs/nfs/read.c | 6 +--
> include/uapi/linux/nfs_fs.h | 2 +-
> 6 files changed, 162 insertions(+), 90 deletions(-)

Series Reviewed-by: Chuck Lever <[email protected]>


--
Chuck Lever