2021-10-07 22:34:37

by David Wysochanski

[permalink] [raw]
Subject: [PATCH v2 0/7] Various NFS fscache cleanups

This patchset is on top of David Howells patchset he just posted as
v3 of "fscache: Replace and remove old I/O API" and is based on his
fscache-remove-old-io branch at
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-remove-old-io
NOTE: fscache-remove-old-io was previously "fscache-iter-3" but it's been
renamed to better reflect the purpose.

The series is also at:
https://github.com/DaveWysochanskiRH/kernel.git
https://github.com/DaveWysochanskiRH/kernel/tree/fscache-remove-old-io-nfs-fixes

Testing is looking ok so far and is still ongoing at BakeAThon and in
my local testbed with tracepoints enabled via:
trace-cmd start -e fscache:* -e nfs:* -e nfs4:* -e cachefiles:*

Changes in v2 of this series
- Dropped first patch of v1 series (dhowells updated his patch)
- Don't rename or change the value of NFSDBG_FSCACHE (Trond)
- Rename nfs_readpage_from_fscache and nfs_readpage_to_fscache
- Rename enable/disable tracepoints to start with "nfs_fscache"
- Rename fscache IO tracepoints to better reflect the new function names
- Place the fscache IO tracepoints at begin and end of the functions

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: Convert NFS fscache enable/disable dfprintks to tracepoints
NFS: Rename fscache read and write pages functions
NFS: Replace dfprintks with tracepoints in 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 | 76 +++++++++++++----------------------
fs/nfs/fscache.h | 25 ++++++------
fs/nfs/nfstrace.h | 98 +++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/read.c | 4 +-
include/uapi/linux/nfs_fs.h | 2 +-
6 files changed, 140 insertions(+), 67 deletions(-)

--
1.8.3.1


2021-10-07 22:34:47

by David Wysochanski

[permalink] [raw]
Subject: [PATCH v2 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 7dbe3a404f34..06c4d8ee7281 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-10-07 22:35:13

by David Wysochanski

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

Rename NFS fscache functions to read from and write to the cache
to better reflect the new fscache fallback API naming.

Signed-off-by: Dave Wysochanski <[email protected]>
---
fs/nfs/fscache.c | 4 ++--
fs/nfs/fscache.h | 17 ++++++++---------
fs/nfs/read.c | 4 ++--
3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c
index 5f9be4a1b5f8..50d68cb946c8 100644
--- a/fs/nfs/fscache.c
+++ b/fs/nfs/fscache.c
@@ -331,7 +331,7 @@ void nfs_fscache_open_file(struct inode *inode, struct file *filp)
/*
* 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;

@@ -364,7 +364,7 @@ int __nfs_readpage_from_fscache(struct inode *inode, struct page *page)
/*
* Store a newly fetched page in fscache
*/
-void __nfs_readpage_to_fscache(struct inode *inode, struct page *page)
+void __nfs_fscache_write_page(struct inode *inode, struct page *page)
{
int ret;

diff --git a/fs/nfs/fscache.h b/fs/nfs/fscache.h
index f4deea2908e9..a2c669ce8217 100644
--- a/fs/nfs/fscache.h
+++ b/fs/nfs/fscache.h
@@ -94,19 +94,18 @@ struct nfs_fscache_inode_auxdata {
extern void nfs_fscache_clear_inode(struct inode *);
extern void nfs_fscache_open_file(struct inode *, struct file *);

-extern int __nfs_readpage_from_fscache(struct inode *, struct page *);
+extern int __nfs_fscache_read_page(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 void __nfs_fscache_write_page(struct inode *, struct page *);

/*
* 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_fscache(inode))
- return __nfs_readpage_from_fscache(inode, page);
+ return __nfs_fscache_read_page(inode, page);
return -ENOBUFS;
}

@@ -114,11 +113,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_fscache(inode))
- __nfs_readpage_to_fscache(inode, page);
+ __nfs_fscache_write_page(inode, page);
}

/*
@@ -161,12 +160,12 @@ static inline void nfs_fscache_clear_inode(struct inode *inode) {}
static inline void nfs_fscache_open_file(struct inode *inode,
struct file *filp) {}

-static inline int nfs_readpage_from_fscache(struct inode *inode,
+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,
+static inline void nfs_fscache_write_page(struct inode *inode,
struct page *page) {}


diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 06ed827a67e8..c65663f217a4 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

2021-10-13 15:42:50

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Various NFS fscache cleanups

On Thu, Oct 7, 2021 at 6:31 PM Dave Wysochanski <[email protected]> wrote:
>
> This patchset is on top of David Howells patchset he just posted as
> v3 of "fscache: Replace and remove old I/O API" and is based on his
> fscache-remove-old-io branch at
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-remove-old-io
> NOTE: fscache-remove-old-io was previously "fscache-iter-3" but it's been
> renamed to better reflect the purpose.
>
> The series is also at:
> https://github.com/DaveWysochanskiRH/kernel.git
> https://github.com/DaveWysochanskiRH/kernel/tree/fscache-remove-old-io-nfs-fixes
>
> Testing is looking ok so far and is still ongoing at BakeAThon and in
> my local testbed with tracepoints enabled via:
> trace-cmd start -e fscache:* -e nfs:* -e nfs4:* -e cachefiles:*
>
> Changes in v2 of this series
> - Dropped first patch of v1 series (dhowells updated his patch)
> - Don't rename or change the value of NFSDBG_FSCACHE (Trond)
> - Rename nfs_readpage_from_fscache and nfs_readpage_to_fscache
> - Rename enable/disable tracepoints to start with "nfs_fscache"
> - Rename fscache IO tracepoints to better reflect the new function names
> - Place the fscache IO tracepoints at begin and end of the functions
>
> 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: Convert NFS fscache enable/disable dfprintks to tracepoints
> NFS: Rename fscache read and write pages functions
> NFS: Replace dfprintks with tracepoints in 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 | 76 +++++++++++++----------------------
> fs/nfs/fscache.h | 25 ++++++------
> fs/nfs/nfstrace.h | 98 +++++++++++++++++++++++++++++++++++++++++++++
> fs/nfs/read.c | 4 +-
> include/uapi/linux/nfs_fs.h | 2 +-
> 6 files changed, 140 insertions(+), 67 deletions(-)
>
> --
> 1.8.3.1
>

Just a report on the testing of this patchset, which also tested
dhowells fscache-remove-old-io branch. Overall this looks very
stable.

I ran some custom unit tests as well as many xfstest runs. I saw
no oops or other significant problems during any of the runs.
I saw no differences in Failures on xfstest runs between 5.15.0-rc4
and this set.
I ran the following configurations of servers and NFS options for the
runs (5.15.0-rc4, then 5.15.0-rc4 + this patchset).
1. hammerspace (pnfs flexfiles): vers=4.1,fsc; vers=4.1,nofsc;
vers=4.2,fsc; vers=4.2,nofsc
2. ontap9.x (pnfs filelayout): vers=4.1,fsc; vers=4.1,nofsc
3. rhel7u8 (3.10.0-1127.8.2.el7): vers=3,nofsc; vers=4.0,nofsc;
vers=4.0,fsc; vers=4.2,fsc; vers=4.2,nofsc
4. rhel8 (4.18.0-193.28.1.el8): vers=4.2,fsc