linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates
@ 2022-01-18 13:52 David Howells
  2022-01-18 13:53 ` [PATCH 01/11] fscache: Fix the volume collision wait condition David Howells
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:52 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Shyam Prasad N, Jeff Layton, linux-cifs, Steve French, Jeffle Xu,
	dhowells, Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Jeff Layton, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel


Here's a set of fixes and minor updates for the fscache rewrite[1]:

 (1) Fix mishandling of volume collisions (the wait condition is inverted
     and so it was only waiting if the volume collision was already
     resolved).

And for cachefiles, including addressing some of Jeff Layton's review
comments:

 (2) Fix miscalculation of whether there's space available.

 (3) Make sure a default cache name is set on a cache if the user hasn't
     set one by the time they bind the cache.

 (4) Adjust the way the backing inode is presented in tracepoints, add a
     tracepoint for mkdir and trace directory lookup.

 (5) Trace failure to set the active file mark.

 (6) Add explanation of the checks made on the backing filesystem.

 (7) Check that the backing filesystem supports tmpfile.

 (8) Document how the page-release cancellation of the read-skip
     optimisation works.

 (9) Add an IS_KERNEL_FILE() check macro for the S_KERNEL_FILE inode flag.

And I've included a change for netfslib:

(10) Make ops->init_rreq() optional.

I've also added the patch to rewrite cifs's fscache indexing.


Link: https://lore.kernel.org/r/164021479106.640689.17404516570194656552.stgit@warthog.procyon.org.uk/ [1]

---
David Howells (9):
      fscache: Fix the volume collision wait condition
      cachefiles: Calculate the blockshift in terms of bytes, not pages
      cachefiles: Make some tracepoint adjustments
      cachefiles: Trace active-mark failure
      cachefiles: Explain checks in a comment
      cachefiles: Check that the backing filesystem supports tmpfiles
      fscache: Add a comment explaining how page-release optimisation works
      vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
      cifs: Support fscache indexing rewrite

Jeffle Xu (2):
      cachefiles: set default tag name if it's unspecified
      netfs: Make ops->init_rreq() optional


 fs/cachefiles/cache.c             |  17 +-
 fs/cachefiles/daemon.c            |  11 +
 fs/cachefiles/internal.h          |   2 +-
 fs/cachefiles/io.c                |   2 +-
 fs/cachefiles/namei.c             |  18 +-
 fs/ceph/addr.c                    |   5 -
 fs/cifs/Kconfig                   |   2 +-
 fs/cifs/Makefile                  |   2 +-
 fs/cifs/cache.c                   | 105 ----------
 fs/cifs/cifsfs.c                  |  19 +-
 fs/cifs/cifsglob.h                |   5 +-
 fs/cifs/connect.c                 |  15 +-
 fs/cifs/dir.c                     |   5 +
 fs/cifs/file.c                    |  70 ++++---
 fs/cifs/fscache.c                 | 333 +++++++-----------------------
 fs/cifs/fscache.h                 | 126 ++++-------
 fs/cifs/inode.c                   |  19 +-
 fs/namei.c                        |   2 +-
 fs/netfs/read_helper.c            |   3 +-
 include/linux/fs.h                |   1 +
 include/linux/fscache.h           |   5 +
 include/trace/events/cachefiles.h | 103 ++++++---
 22 files changed, 313 insertions(+), 557 deletions(-)
 delete mode 100644 fs/cifs/cache.c



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 01/11] fscache: Fix the volume collision wait condition
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
@ 2022-01-18 13:53 ` David Howells
  2022-01-18 13:53 ` [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages David Howells
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:53 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, dhowells, Trond Myklebust, Anna Schumaker,
	Steve French, Dominique Martinet, Jeff Layton, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

The condition that the waits in fscache_wait_on_volume_collision() are
waiting until are inverted.  This suddenly started happening on the
upstream kernel with something like the following appearing in dmesg when
running xfstests:

	CacheFiles: cachefiles: Inode already in use: Iexample.com,100055

Fix them by inverting the conditions.

Fixes: 62ab63352350 ("fscache: Implement volume registration")
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
---

 fs/fscache/volume.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fscache/volume.c b/fs/fscache/volume.c
index a57c6cbee858..f2aa7dbad766 100644
--- a/fs/fscache/volume.c
+++ b/fs/fscache/volume.c
@@ -142,12 +142,12 @@ static void fscache_wait_on_volume_collision(struct fscache_volume *candidate,
 					     unsigned int collidee_debug_id)
 {
 	wait_var_event_timeout(&candidate->flags,
-			       fscache_is_acquire_pending(candidate), 20 * HZ);
+			       !fscache_is_acquire_pending(candidate), 20 * HZ);
 	if (!fscache_is_acquire_pending(candidate)) {
 		pr_notice("Potential volume collision new=%08x old=%08x",
 			  candidate->debug_id, collidee_debug_id);
 		fscache_stat(&fscache_n_volumes_collision);
-		wait_var_event(&candidate->flags, fscache_is_acquire_pending(candidate));
+		wait_var_event(&candidate->flags, !fscache_is_acquire_pending(candidate));
 	}
 }
 



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
  2022-01-18 13:53 ` [PATCH 01/11] fscache: Fix the volume collision wait condition David Howells
@ 2022-01-18 13:53 ` David Howells
  2022-01-21 17:47   ` Jeff Layton
  2022-01-18 13:53 ` [PATCH 03/11] cachefiles: set default tag name if it's unspecified David Howells
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2022-01-18 13:53 UTC (permalink / raw)
  To: linux-cachefs
  Cc: dhowells, Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Jeff Layton, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

Cachefiles keeps track of how much space is available on the backing
filesystem and refuses new writes permission to start if there isn't enough
(we especially don't want ENOSPC happening).  It also tracks the amount of
data pending in DIO writes (cache->b_writing) and reduces the amount of
free space available by this amount before deciding if it can set up a new
write.

However, the old fscache I/O API was very much page-granularity dependent
and, as such, cachefiles's cache->bshift was meant to be a multiplier to
get from PAGE_SIZE to block size (ie. a blocksize of 512 would give a shift
of 3 for a 4KiB page) - and this was incorrectly being used to turn the
number of bytes in a DIO write into a number of blocks, leading to a
massive over estimation of the amount of data in flight.

Fix this by changing cache->bshift to be a multiplier from bytes to
blocksize and deal with quantities of blocks, not quantities of pages.

Fix also the rounding in the calculation in cachefiles_write() which needs
a "- 1" inserting.

Fixes: 047487c947e8 ("cachefiles: Implement the I/O routines")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-cachefs@redhat.com
---

 fs/cachefiles/cache.c    |    7 ++-----
 fs/cachefiles/internal.h |    2 +-
 fs/cachefiles/io.c       |    2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/cachefiles/cache.c b/fs/cachefiles/cache.c
index ce4d4785003c..1e9c71666c6a 100644
--- a/fs/cachefiles/cache.c
+++ b/fs/cachefiles/cache.c
@@ -84,9 +84,7 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
 		goto error_unsupported;
 
 	cache->bsize = stats.f_bsize;
-	cache->bshift = 0;
-	if (stats.f_bsize < PAGE_SIZE)
-		cache->bshift = PAGE_SHIFT - ilog2(stats.f_bsize);
+	cache->bshift = ilog2(stats.f_bsize);
 
 	_debug("blksize %u (shift %u)",
 	       cache->bsize, cache->bshift);
@@ -106,7 +104,6 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
 	       (unsigned long long) cache->fcull,
 	       (unsigned long long) cache->fstop);
 
-	stats.f_blocks >>= cache->bshift;
 	do_div(stats.f_blocks, 100);
 	cache->bstop = stats.f_blocks * cache->bstop_percent;
 	cache->bcull = stats.f_blocks * cache->bcull_percent;
@@ -209,7 +206,7 @@ int cachefiles_has_space(struct cachefiles_cache *cache,
 		return ret;
 	}
 
-	b_avail = stats.f_bavail >> cache->bshift;
+	b_avail = stats.f_bavail;
 	b_writing = atomic_long_read(&cache->b_writing);
 	if (b_avail > b_writing)
 		b_avail -= b_writing;
diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 8dd54d9375b6..c793d33b0224 100644
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -86,7 +86,7 @@ struct cachefiles_cache {
 	unsigned			bcull_percent;	/* when to start culling (% blocks) */
 	unsigned			bstop_percent;	/* when to stop allocating (% blocks) */
 	unsigned			bsize;		/* cache's block size */
-	unsigned			bshift;		/* min(ilog2(PAGE_SIZE / bsize), 0) */
+	unsigned			bshift;		/* ilog2(bsize) */
 	uint64_t			frun;		/* when to stop culling */
 	uint64_t			fcull;		/* when to start culling */
 	uint64_t			fstop;		/* when to stop allocating */
diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index 60b1eac2ce78..04eb52736990 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -264,7 +264,7 @@ static int cachefiles_write(struct netfs_cache_resources *cres,
 	ki->term_func		= term_func;
 	ki->term_func_priv	= term_func_priv;
 	ki->was_async		= true;
-	ki->b_writing		= (len + (1 << cache->bshift)) >> cache->bshift;
+	ki->b_writing		= (len + (1 << cache->bshift) - 1) >> cache->bshift;
 
 	if (ki->term_func)
 		ki->iocb.ki_complete = cachefiles_write_complete;



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 03/11] cachefiles: set default tag name if it's unspecified
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
  2022-01-18 13:53 ` [PATCH 01/11] fscache: Fix the volume collision wait condition David Howells
  2022-01-18 13:53 ` [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages David Howells
@ 2022-01-18 13:53 ` David Howells
  2022-01-21 17:51   ` Jeff Layton
  2022-01-18 13:53 ` [PATCH 04/11] cachefiles: Make some tracepoint adjustments David Howells
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2022-01-18 13:53 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeffle Xu, dhowells, Trond Myklebust, Anna Schumaker,
	Steve French, Dominique Martinet, Jeff Layton, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

From: Jeffle Xu <jefflexu@linux.alibaba.com>

fscache_acquire_cache() requires a non-empty name, while 'tag <name>'
command is optional for cachefilesd.

Thus set default tag name if it's unspecified to avoid the regression of
cachefilesd. The logic is the same with that before rewritten.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-cachefs@redhat.com
---

 fs/cachefiles/daemon.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 40a792421fc1..7ac04ee2c0a0 100644
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -703,6 +703,17 @@ static int cachefiles_daemon_bind(struct cachefiles_cache *cache, char *args)
 		return -EBUSY;
 	}
 
+	/* Make sure we have copies of the tag string */
+	if (!cache->tag) {
+		/*
+		 * The tag string is released by the fops->release()
+		 * function, so we don't release it on error here
+		 */
+		cache->tag = kstrdup("CacheFiles", GFP_KERNEL);
+		if (!cache->tag)
+			return -ENOMEM;
+	}
+
 	return cachefiles_add_cache(cache);
 }
 



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 04/11] cachefiles: Make some tracepoint adjustments
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (2 preceding siblings ...)
  2022-01-18 13:53 ` [PATCH 03/11] cachefiles: set default tag name if it's unspecified David Howells
@ 2022-01-18 13:53 ` David Howells
  2022-01-21 17:52   ` Jeff Layton
  2022-01-18 13:54 ` [PATCH 05/11] cachefiles: Trace active-mark failure David Howells
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2022-01-18 13:53 UTC (permalink / raw)
  To: linux-cachefs
  Cc: dhowells, Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Jeff Layton, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

Make some adjustments to tracepoints to make the tracing a bit more
followable:

 (1) Standardise on displaying the backing inode number as "B=<hex>" with
     no leading zeros.

 (2) Make the cachefiles_lookup tracepoint log the directory inode number
     as well as the looked-up inode number.

 (3) Add a cachefiles_lookup tracepoint into cachefiles_get_directory() to
     log directory lookup.

 (4) Add a new cachefiles_mkdir tracepoint and use that to log a successful
     mkdir from cachefiles_get_directory().

 (5) Make the cachefiles_unlink and cachefiles_rename tracepoints log the
     inode number of the affected file/dir rather than dentry struct
     pointers.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-cachefs@redhat.com
---

 fs/cachefiles/namei.c             |    8 ++--
 include/trace/events/cachefiles.h |   82 +++++++++++++++++++++++--------------
 2 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 9bd692870617..52c9f0864a87 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -101,6 +101,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 		subdir = lookup_one_len(dirname, dir, strlen(dirname));
 	else
 		subdir = ERR_PTR(ret);
+	trace_cachefiles_lookup(NULL, dir, subdir);
 	if (IS_ERR(subdir)) {
 		trace_cachefiles_vfs_error(NULL, d_backing_inode(dir),
 					   PTR_ERR(subdir),
@@ -135,6 +136,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
 						   cachefiles_trace_mkdir_error);
 			goto mkdir_error;
 		}
+		trace_cachefiles_mkdir(dir, subdir);
 
 		if (unlikely(d_unhashed(subdir))) {
 			cachefiles_put_directory(subdir);
@@ -233,7 +235,7 @@ static int cachefiles_unlink(struct cachefiles_cache *cache,
 	};
 	int ret;
 
-	trace_cachefiles_unlink(object, dentry, why);
+	trace_cachefiles_unlink(object, d_inode(dentry)->i_ino, why);
 	ret = security_path_unlink(&path, dentry);
 	if (ret < 0) {
 		cachefiles_io_error(cache, "Unlink security error");
@@ -386,7 +388,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
 			.new_dir	= d_inode(cache->graveyard),
 			.new_dentry	= grave,
 		};
-		trace_cachefiles_rename(object, rep, grave, why);
+		trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
 		ret = cachefiles_inject_read_error();
 		if (ret == 0)
 			ret = vfs_rename(&rd);
@@ -617,7 +619,7 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
 						  object->d_name_len);
 	else
 		dentry = ERR_PTR(ret);
-	trace_cachefiles_lookup(object, dentry);
+	trace_cachefiles_lookup(object, fan, dentry);
 	if (IS_ERR(dentry)) {
 		if (dentry == ERR_PTR(-ENOENT))
 			goto new_file;
diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index 1172529b5b49..093c4acb7a3a 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -233,25 +233,48 @@ TRACE_EVENT(cachefiles_ref,
 
 TRACE_EVENT(cachefiles_lookup,
 	    TP_PROTO(struct cachefiles_object *obj,
+		     struct dentry *dir,
 		     struct dentry *de),
 
-	    TP_ARGS(obj, de),
+	    TP_ARGS(obj, dir, de),
 
 	    TP_STRUCT__entry(
 		    __field(unsigned int,		obj	)
 		    __field(short,			error	)
+		    __field(unsigned long,		dino	)
 		    __field(unsigned long,		ino	)
 			     ),
 
 	    TP_fast_assign(
-		    __entry->obj	= obj->debug_id;
+		    __entry->obj	= obj ? obj->debug_id : 0;
+		    __entry->dino	= d_backing_inode(dir)->i_ino;
 		    __entry->ino	= (!IS_ERR(de) && d_backing_inode(de) ?
 					   d_backing_inode(de)->i_ino : 0);
 		    __entry->error	= IS_ERR(de) ? PTR_ERR(de) : 0;
 			   ),
 
-	    TP_printk("o=%08x i=%lx e=%d",
-		      __entry->obj, __entry->ino, __entry->error)
+	    TP_printk("o=%08x dB=%lx B=%lx e=%d",
+		      __entry->obj, __entry->dino, __entry->ino, __entry->error)
+	    );
+
+TRACE_EVENT(cachefiles_mkdir,
+	    TP_PROTO(struct dentry *dir, struct dentry *subdir),
+
+	    TP_ARGS(dir, subdir),
+
+	    TP_STRUCT__entry(
+		    __field(unsigned int,			dir	)
+		    __field(unsigned int,			subdir	)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->dir	= d_backing_inode(dir)->i_ino;
+		    __entry->subdir	= d_backing_inode(subdir)->i_ino;
+			   ),
+
+	    TP_printk("dB=%x sB=%x",
+		      __entry->dir,
+		      __entry->subdir)
 	    );
 
 TRACE_EVENT(cachefiles_tmpfile,
@@ -269,7 +292,7 @@ TRACE_EVENT(cachefiles_tmpfile,
 		    __entry->backer	= backer->i_ino;
 			   ),
 
-	    TP_printk("o=%08x b=%08x",
+	    TP_printk("o=%08x B=%x",
 		      __entry->obj,
 		      __entry->backer)
 	    );
@@ -289,61 +312,58 @@ TRACE_EVENT(cachefiles_link,
 		    __entry->backer	= backer->i_ino;
 			   ),
 
-	    TP_printk("o=%08x b=%08x",
+	    TP_printk("o=%08x B=%x",
 		      __entry->obj,
 		      __entry->backer)
 	    );
 
 TRACE_EVENT(cachefiles_unlink,
 	    TP_PROTO(struct cachefiles_object *obj,
-		     struct dentry *de,
+		     ino_t ino,
 		     enum fscache_why_object_killed why),
 
-	    TP_ARGS(obj, de, why),
+	    TP_ARGS(obj, ino, why),
 
 	    /* Note that obj may be NULL */
 	    TP_STRUCT__entry(
 		    __field(unsigned int,		obj		)
-		    __field(struct dentry *,		de		)
+		    __field(unsigned int,		ino		)
 		    __field(enum fscache_why_object_killed, why		)
 			     ),
 
 	    TP_fast_assign(
 		    __entry->obj	= obj ? obj->debug_id : UINT_MAX;
-		    __entry->de		= de;
+		    __entry->ino	= ino;
 		    __entry->why	= why;
 			   ),
 
-	    TP_printk("o=%08x d=%p w=%s",
-		      __entry->obj, __entry->de,
+	    TP_printk("o=%08x B=%x w=%s",
+		      __entry->obj, __entry->ino,
 		      __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
 	    );
 
 TRACE_EVENT(cachefiles_rename,
 	    TP_PROTO(struct cachefiles_object *obj,
-		     struct dentry *de,
-		     struct dentry *to,
+		     ino_t ino,
 		     enum fscache_why_object_killed why),
 
-	    TP_ARGS(obj, de, to, why),
+	    TP_ARGS(obj, ino, why),
 
 	    /* Note that obj may be NULL */
 	    TP_STRUCT__entry(
 		    __field(unsigned int,		obj		)
-		    __field(struct dentry *,		de		)
-		    __field(struct dentry *,		to		)
+		    __field(unsigned int,		ino		)
 		    __field(enum fscache_why_object_killed, why		)
 			     ),
 
 	    TP_fast_assign(
 		    __entry->obj	= obj ? obj->debug_id : UINT_MAX;
-		    __entry->de		= de;
-		    __entry->to		= to;
+		    __entry->ino	= ino;
 		    __entry->why	= why;
 			   ),
 
-	    TP_printk("o=%08x d=%p t=%p w=%s",
-		      __entry->obj, __entry->de, __entry->to,
+	    TP_printk("o=%08x B=%x w=%s",
+		      __entry->obj, __entry->ino,
 		      __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
 	    );
 
@@ -370,7 +390,7 @@ TRACE_EVENT(cachefiles_coherency,
 		    __entry->ino	= ino;
 			   ),
 
-	    TP_printk("o=%08x %s i=%llx c=%u",
+	    TP_printk("o=%08x %s B=%llx c=%u",
 		      __entry->obj,
 		      __print_symbolic(__entry->why, cachefiles_coherency_traces),
 		      __entry->ino,
@@ -397,7 +417,7 @@ TRACE_EVENT(cachefiles_vol_coherency,
 		    __entry->ino	= ino;
 			   ),
 
-	    TP_printk("V=%08x %s i=%llx",
+	    TP_printk("V=%08x %s B=%llx",
 		      __entry->vol,
 		      __print_symbolic(__entry->why, cachefiles_coherency_traces),
 		      __entry->ino)
@@ -435,7 +455,7 @@ TRACE_EVENT(cachefiles_prep_read,
 		    __entry->cache_inode = cache_inode;
 			   ),
 
-	    TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x b=%x",
+	    TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x B=%x",
 		      __entry->rreq, __entry->index,
 		      __print_symbolic(__entry->source, netfs_sreq_sources),
 		      __print_symbolic(__entry->why, cachefiles_prepare_read_traces),
@@ -466,7 +486,7 @@ TRACE_EVENT(cachefiles_read,
 		    __entry->len	= len;
 			   ),
 
-	    TP_printk("o=%08x b=%08x s=%llx l=%zx",
+	    TP_printk("o=%08x B=%x s=%llx l=%zx",
 		      __entry->obj,
 		      __entry->backer,
 		      __entry->start,
@@ -495,7 +515,7 @@ TRACE_EVENT(cachefiles_write,
 		    __entry->len	= len;
 			   ),
 
-	    TP_printk("o=%08x b=%08x s=%llx l=%zx",
+	    TP_printk("o=%08x B=%x s=%llx l=%zx",
 		      __entry->obj,
 		      __entry->backer,
 		      __entry->start,
@@ -524,7 +544,7 @@ TRACE_EVENT(cachefiles_trunc,
 		    __entry->why	= why;
 			   ),
 
-	    TP_printk("o=%08x b=%08x %s l=%llx->%llx",
+	    TP_printk("o=%08x B=%x %s l=%llx->%llx",
 		      __entry->obj,
 		      __entry->backer,
 		      __print_symbolic(__entry->why, cachefiles_trunc_traces),
@@ -549,7 +569,7 @@ TRACE_EVENT(cachefiles_mark_active,
 		    __entry->inode	= inode->i_ino;
 			   ),
 
-	    TP_printk("o=%08x i=%lx",
+	    TP_printk("o=%08x B=%lx",
 		      __entry->obj, __entry->inode)
 	    );
 
@@ -570,7 +590,7 @@ TRACE_EVENT(cachefiles_mark_inactive,
 		    __entry->inode	= inode->i_ino;
 			   ),
 
-	    TP_printk("o=%08x i=%lx",
+	    TP_printk("o=%08x B=%lx",
 		      __entry->obj, __entry->inode)
 	    );
 
@@ -594,7 +614,7 @@ TRACE_EVENT(cachefiles_vfs_error,
 		    __entry->where	= where;
 			   ),
 
-	    TP_printk("o=%08x b=%08x %s e=%d",
+	    TP_printk("o=%08x B=%x %s e=%d",
 		      __entry->obj,
 		      __entry->backer,
 		      __print_symbolic(__entry->where, cachefiles_error_traces),
@@ -621,7 +641,7 @@ TRACE_EVENT(cachefiles_io_error,
 		    __entry->where	= where;
 			   ),
 
-	    TP_printk("o=%08x b=%08x %s e=%d",
+	    TP_printk("o=%08x B=%x %s e=%d",
 		      __entry->obj,
 		      __entry->backer,
 		      __print_symbolic(__entry->where, cachefiles_error_traces),



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 05/11] cachefiles: Trace active-mark failure
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (3 preceding siblings ...)
  2022-01-18 13:53 ` [PATCH 04/11] cachefiles: Make some tracepoint adjustments David Howells
@ 2022-01-18 13:54 ` David Howells
  2022-01-21 17:53   ` Jeff Layton
  2022-01-18 13:54 ` [PATCH 06/11] cachefiles: Explain checks in a comment David Howells
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2022-01-18 13:54 UTC (permalink / raw)
  To: linux-cachefs
  Cc: dhowells, Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Jeff Layton, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

Add a tracepoint to log failure to apply an active mark to a file in
addition to tracing successfully setting and unsetting the mark.

Also include the backing file inode number in the message logged to dmesg.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-cachefs@redhat.com
---

 fs/cachefiles/namei.c             |    4 +++-
 include/trace/events/cachefiles.h |   21 +++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 52c9f0864a87..f256c8aff7bb 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -25,7 +25,9 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 		trace_cachefiles_mark_active(object, inode);
 		can_use = true;
 	} else {
-		pr_notice("cachefiles: Inode already in use: %pd\n", dentry);
+		trace_cachefiles_mark_failed(object, inode);
+		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
+			  dentry, inode->i_ino);
 	}
 
 	return can_use;
diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index 093c4acb7a3a..c6f5aa74db89 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -573,6 +573,27 @@ TRACE_EVENT(cachefiles_mark_active,
 		      __entry->obj, __entry->inode)
 	    );
 
+TRACE_EVENT(cachefiles_mark_failed,
+	    TP_PROTO(struct cachefiles_object *obj,
+		     struct inode *inode),
+
+	    TP_ARGS(obj, inode),
+
+	    /* Note that obj may be NULL */
+	    TP_STRUCT__entry(
+		    __field(unsigned int,		obj		)
+		    __field(ino_t,			inode		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->obj	= obj ? obj->debug_id : 0;
+		    __entry->inode	= inode->i_ino;
+			   ),
+
+	    TP_printk("o=%08x B=%lx",
+		      __entry->obj, __entry->inode)
+	    );
+
 TRACE_EVENT(cachefiles_mark_inactive,
 	    TP_PROTO(struct cachefiles_object *obj,
 		     struct inode *inode),



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 06/11] cachefiles: Explain checks in a comment
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (4 preceding siblings ...)
  2022-01-18 13:54 ` [PATCH 05/11] cachefiles: Trace active-mark failure David Howells
@ 2022-01-18 13:54 ` David Howells
  2022-01-18 13:54 ` [PATCH 07/11] cachefiles: Check that the backing filesystem supports tmpfiles David Howells
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:54 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, Jeff Layton, dhowells, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Jeff Layton,
	Matthew Wilcox, Alexander Viro, Omar Sandoval, JeffleXu,
	Linus Torvalds, linux-afs, linux-nfs, linux-cifs, ceph-devel,
	v9fs-developer, linux-fsdevel, linux-kernel

Add a comment to explain the checks that cachefiles is making of the
backing filesystem[1].

Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/568749bd7cc02908ecf6f3d6a611b6f9cf5c4afd.camel@kernel.org/ [1]
---

 fs/cachefiles/cache.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/cachefiles/cache.c b/fs/cachefiles/cache.c
index 1e9c71666c6a..2b2879c5d1d2 100644
--- a/fs/cachefiles/cache.c
+++ b/fs/cachefiles/cache.c
@@ -49,7 +49,13 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
 		goto error_unsupported;
 	}
 
-	/* check parameters */
+	/* Check features of the backing filesystem:
+	 * - Directories must support looking up and directory creation
+	 * - We use xattrs to store metadata
+	 * - We need to be able to query the amount of space available
+	 * - We want to be able to sync the filesystem when stopping the cache
+	 * - We use DIO to/from pages, so the blocksize mustn't be too big.
+	 */
 	ret = -EOPNOTSUPP;
 	if (d_is_negative(root) ||
 	    !d_backing_inode(root)->i_op->lookup ||



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 07/11] cachefiles: Check that the backing filesystem supports tmpfiles
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (5 preceding siblings ...)
  2022-01-18 13:54 ` [PATCH 06/11] cachefiles: Explain checks in a comment David Howells
@ 2022-01-18 13:54 ` David Howells
  2022-01-18 13:54 ` [PATCH 08/11] fscache: Add a comment explaining how page-release optimisation works David Howells
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:54 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, Jeff Layton, dhowells, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Jeff Layton,
	Matthew Wilcox, Alexander Viro, Omar Sandoval, JeffleXu,
	Linus Torvalds, linux-afs, linux-nfs, linux-cifs, ceph-devel,
	v9fs-developer, linux-fsdevel, linux-kernel

Add a check that the backing filesystem supports the creation of
tmpfiles[1].

Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/568749bd7cc02908ecf6f3d6a611b6f9cf5c4afd.camel@kernel.org/ [1]
---

 fs/cachefiles/cache.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/cachefiles/cache.c b/fs/cachefiles/cache.c
index 2b2879c5d1d2..7077f72e6f47 100644
--- a/fs/cachefiles/cache.c
+++ b/fs/cachefiles/cache.c
@@ -51,6 +51,7 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
 
 	/* Check features of the backing filesystem:
 	 * - Directories must support looking up and directory creation
+	 * - We create tmpfiles to handle invalidation
 	 * - We use xattrs to store metadata
 	 * - We need to be able to query the amount of space available
 	 * - We want to be able to sync the filesystem when stopping the cache
@@ -60,6 +61,7 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
 	if (d_is_negative(root) ||
 	    !d_backing_inode(root)->i_op->lookup ||
 	    !d_backing_inode(root)->i_op->mkdir ||
+	    !d_backing_inode(root)->i_op->tmpfile ||
 	    !(d_backing_inode(root)->i_opflags & IOP_XATTR) ||
 	    !root->d_sb->s_op->statfs ||
 	    !root->d_sb->s_op->sync_fs ||



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 08/11] fscache: Add a comment explaining how page-release optimisation works
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (6 preceding siblings ...)
  2022-01-18 13:54 ` [PATCH 07/11] cachefiles: Check that the backing filesystem supports tmpfiles David Howells
@ 2022-01-18 13:54 ` David Howells
  2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:54 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, dhowells, Trond Myklebust, Anna Schumaker,
	Steve French, Dominique Martinet, Jeff Layton, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

Add a comment into fscache_note_page_release() to explain how the
page-release optimisation logic works[1].  It's not entirely obvious as it
has nothing to do with whether or not the netfs file contains data.

FSCACHE_COOKIE_NO_DATA_TO_READ is set if we have no data in the cache yet
(ie. the backing file lookup was negative, the file is 0 length or the
cookie got invalidated).  It means that we have no data in the cache, not
that the file is necessarily empty on the server.

FSCACHE_COOKIE_HAVE_DATA is set once we've stored data in the backing file.
From that point on, we have data we *could* read - however, it's covered by
pages in the netfs pagecache until at such time one of those covering pages
is released.

So if we've written data to the cache (HAVE_DATA) and there wasn't any data
in the cache when we started (NO_DATA_TO_READ), it may no longer be true
that we can skip reading from the cache.

Read skipping is done by cachefiles_prepare_read().

Note that tracking is not done on a per-page basis, but only on a per-file
basis.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/043a206f03929c2667a465314144e518070a9b2d.camel@kernel.org/ [1]
---

 include/linux/fscache.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/fscache.h b/include/linux/fscache.h
index ede50406bcb0..296c5f1d9f35 100644
--- a/include/linux/fscache.h
+++ b/include/linux/fscache.h
@@ -665,6 +665,11 @@ static inline void fscache_clear_inode_writeback(struct fscache_cookie *cookie,
 static inline
 void fscache_note_page_release(struct fscache_cookie *cookie)
 {
+	/* If we've written data to the cache (HAVE_DATA) and there wasn't any
+	 * data in the cache when we started (NO_DATA_TO_READ), it may no
+	 * longer be true that we can skip reading from the cache - so clear
+	 * the flag that causes reads to be skipped.
+	 */
 	if (cookie &&
 	    test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) &&
 	    test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags))



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (7 preceding siblings ...)
  2022-01-18 13:54 ` [PATCH 08/11] fscache: Add a comment explaining how page-release optimisation works David Howells
@ 2022-01-18 13:54 ` David Howells
  2022-01-18 16:23   ` Christoph Hellwig
  2022-01-18 17:40   ` David Howells
  2022-01-18 13:55 ` [PATCH 10/11] netfs: Make ops->init_rreq() optional David Howells
  2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
  10 siblings, 2 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:54 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, Jeff Layton, dhowells, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Jeff Layton,
	Matthew Wilcox, Alexander Viro, Omar Sandoval, JeffleXu,
	Linus Torvalds, linux-afs, linux-nfs, linux-cifs, ceph-devel,
	v9fs-developer, linux-fsdevel, linux-kernel

Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
common practice for the other inode flags[1].

Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/88d7f8970dcc0fd0ead891b1f42f160b8d17d60e.camel@kernel.org/ [1]
---

 fs/cachefiles/namei.c |    6 +++---
 fs/namei.c            |    2 +-
 include/linux/fs.h    |    1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index f256c8aff7bb..04563f759e99 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -20,7 +20,7 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 	struct inode *inode = d_backing_inode(dentry);
 	bool can_use = false;
 
-	if (!(inode->i_flags & S_KERNEL_FILE)) {
+	if (!IS_KERNEL_FILE(inode)) {
 		inode->i_flags |= S_KERNEL_FILE;
 		trace_cachefiles_mark_active(object, inode);
 		can_use = true;
@@ -746,7 +746,7 @@ static struct dentry *cachefiles_lookup_for_cull(struct cachefiles_cache *cache,
 		goto lookup_error;
 	if (d_is_negative(victim))
 		goto lookup_put;
-	if (d_inode(victim)->i_flags & S_KERNEL_FILE)
+	if (IS_KERNEL_FILE(d_inode(victim)))
 		goto lookup_busy;
 	return victim;
 
@@ -793,7 +793,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	/* check to see if someone is using this object */
 	inode = d_inode(victim);
 	inode_lock(inode);
-	if (inode->i_flags & S_KERNEL_FILE) {
+	if (IS_KERNEL_FILE(inode)) {
 		ret = -EBUSY;
 	} else {
 		/* Stop the cache from picking it back up */
diff --git a/fs/namei.c b/fs/namei.c
index d81f04f8d818..c2175ab3849d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3959,7 +3959,7 @@ int vfs_rmdir(struct user_namespace *mnt_userns, struct inode *dir,
 
 	error = -EBUSY;
 	if (is_local_mountpoint(dentry) ||
-	    (dentry->d_inode->i_flags & S_KERNEL_FILE))
+	    IS_KERNEL_FILE(dentry->d_inode))
 		goto out;
 
 	error = security_inode_rmdir(dir, dentry);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f5d3bf5b69a6..227497793282 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2216,6 +2216,7 @@ static inline bool sb_rdonly(const struct super_block *sb) { return sb->s_flags
 #define IS_ENCRYPTED(inode)	((inode)->i_flags & S_ENCRYPTED)
 #define IS_CASEFOLDED(inode)	((inode)->i_flags & S_CASEFOLD)
 #define IS_VERITY(inode)	((inode)->i_flags & S_VERITY)
+#define IS_KERNEL_FILE(inode)	((inode)->i_flags & S_KERNEL_FILE)
 
 #define IS_WHITEOUT(inode)	(S_ISCHR(inode->i_mode) && \
 				 (inode)->i_rdev == WHITEOUT_DEV)



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 10/11] netfs: Make ops->init_rreq() optional
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (8 preceding siblings ...)
  2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
@ 2022-01-18 13:55 ` David Howells
  2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
  10 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:55 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeffle Xu, Jeff Layton, dhowells, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Jeff Layton,
	Matthew Wilcox, Alexander Viro, Omar Sandoval, JeffleXu,
	Linus Torvalds, linux-afs, linux-nfs, linux-cifs, ceph-devel,
	v9fs-developer, linux-fsdevel, linux-kernel

From: Jeffle Xu <jefflexu@linux.alibaba.com>

Make the ops->init_rreq() callback optional.  This isn't required for the
erofs changes I'm implementing to do on-demand read through fscache[1].
Further, ceph has an empty init_rreq method that can then be removed and
it's marked optional in the documentation.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/20211227125444.21187-1-jefflexu@linux.alibaba.com/ [1]
Link: https://lore.kernel.org/r/20211228124419.103020-1-jefflexu@linux.alibaba.com
---

 fs/ceph/addr.c         |    5 -----
 fs/netfs/read_helper.c |    3 ++-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index b3d9459c9bbd..c98e5238a1b6 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -297,10 +297,6 @@ static void ceph_netfs_issue_op(struct netfs_read_subrequest *subreq)
 	dout("%s: result %d\n", __func__, err);
 }
 
-static void ceph_init_rreq(struct netfs_read_request *rreq, struct file *file)
-{
-}
-
 static void ceph_readahead_cleanup(struct address_space *mapping, void *priv)
 {
 	struct inode *inode = mapping->host;
@@ -312,7 +308,6 @@ static void ceph_readahead_cleanup(struct address_space *mapping, void *priv)
 }
 
 static const struct netfs_read_request_ops ceph_netfs_read_ops = {
-	.init_rreq		= ceph_init_rreq,
 	.is_cache_enabled	= ceph_is_cache_enabled,
 	.begin_cache_operation	= ceph_begin_cache_operation,
 	.issue_op		= ceph_netfs_issue_op,
diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c
index 6169659857b3..501da990c259 100644
--- a/fs/netfs/read_helper.c
+++ b/fs/netfs/read_helper.c
@@ -55,7 +55,8 @@ static struct netfs_read_request *netfs_alloc_read_request(
 		INIT_WORK(&rreq->work, netfs_rreq_work);
 		refcount_set(&rreq->usage, 1);
 		__set_bit(NETFS_RREQ_IN_PROGRESS, &rreq->flags);
-		ops->init_rreq(rreq, file);
+		if (ops->init_rreq)
+			ops->init_rreq(rreq, file);
 		netfs_stat(&netfs_n_rh_rreq);
 	}
 



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH 11/11] cifs: Support fscache indexing rewrite
  2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
                   ` (9 preceding siblings ...)
  2022-01-18 13:55 ` [PATCH 10/11] netfs: Make ops->init_rreq() optional David Howells
@ 2022-01-18 13:55 ` David Howells
  2022-01-19  5:31   ` Steve French
  2022-01-19  8:32   ` David Howells
  10 siblings, 2 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 13:55 UTC (permalink / raw)
  To: linux-cachefs
  Cc: Jeff Layton, Steve French, Shyam Prasad N, linux-cifs, dhowells,
	Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Jeff Layton, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

Change the cifs filesystem to take account of the changes to fscache's
indexing rewrite and reenable caching in cifs.

The following changes have been made:

 (1) The fscache_netfs struct is no more, and there's no need to register
     the filesystem as a whole.

 (2) The session cookie is now an fscache_volume cookie, allocated with
     fscache_acquire_volume().  That takes three parameters: a string
     representing the "volume" in the index, a string naming the cache to
     use (or NULL) and a u64 that conveys coherency metadata for the
     volume.

     For cifs, I've made it render the volume name string as:

	"cifs,<ipaddress>,<sharename>"

     where the sharename has '/' characters replaced with ';'.

     This probably needs rethinking a bit as the total name could exceed
     the maximum filename component length.

     Further, the coherency data is currently just set to 0.  It needs
     something else doing with it - I wonder if it would suffice simply to
     sum the resource_id, vol_create_time and vol_serial_number or maybe
     hash them.

 (3) The fscache_cookie_def is no more and needed information is passed
     directly to fscache_acquire_cookie().  The cache no longer calls back
     into the filesystem, but rather metadata changes are indicated at
     other times.

     fscache_acquire_cookie() is passed the same keying and coherency
     information as before.

 (4) The functions to set/reset cookies are removed and
     fscache_use_cookie() and fscache_unuse_cookie() are used instead.

     fscache_use_cookie() is passed a flag to indicate if the cookie is
     opened for writing.  fscache_unuse_cookie() is passed updates for the
     metadata if we changed it (ie. if the file was opened for writing).

     These are called when the file is opened or closed.

 (5) cifs_setattr_*() are made to call fscache_resize() to change the size
     of the cache object.

 (6) The functions to read and write data are stubbed out pending a
     conversion to use netfslib.

Changes
=======
ver #7:
 - Removed the accidentally added-back call to get the super cookie in
   cifs_root_iget().
 - Fixed the right call to cifs_fscache_get_super_cookie() to take account
   of the "-o fsc" mount flag.

ver #6:
 - Moved the change of gfpflags_allow_blocking() to current_is_kswapd() for
   cifs here.
 - Fixed one of the error paths in cifs_atomic_open() to jump around the
   call to use the cookie.
 - Fixed an additional successful return in the middle of cifs_open() to
   use the cookie on the way out.
 - Only get a volume cookie (and thus inode cookies) when "-o fsc" is
   supplied to mount.

ver #5:
 - Fixed a couple of bits of cookie handling[2]:
   - The cookie should be released in cifs_evict_inode(), not
     cifsFileInfo_put_final().  The cookie needs to persist beyond file
     closure so that writepages will be able to write to it.
   - fscache_use_cookie() needs to be called in cifs_atomic_open() as it is
     for cifs_open().

ver #4:
 - Fixed the use of sizeof with memset.
 - tcon->vol_create_time is __le64 so doesn't need cpu_to_le64().

ver #3:
 - Canonicalise the cifs coherency data to make the cache portable.
 - Set volume coherency data.

ver #2:
 - Use gfpflags_allow_blocking() rather than using flag directly.
 - Upgraded to -rc4 to allow for upstream changes[1].
 - fscache_acquire_volume() now returns errors.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Jeff Layton <jlayton@kernel.org>
cc: Steve French <smfrench@gmail.com>
cc: Shyam Prasad N <nspmangalore@gmail.com>
cc: linux-cifs@vger.kernel.org
cc: linux-cachefs@redhat.com
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23b55d673d7527b093cd97b7c217c82e70cd1af0 [1]
Link: https://lore.kernel.org/r/3419813.1641592362@warthog.procyon.org.uk/ [2]
Link: https://lore.kernel.org/r/163819671009.215744.11230627184193298714.stgit@warthog.procyon.org.uk/ # v1
Link: https://lore.kernel.org/r/163906982979.143852.10672081929614953210.stgit@warthog.procyon.org.uk/ # v2
Link: https://lore.kernel.org/r/163967187187.1823006.247415138444991444.stgit@warthog.procyon.org.uk/ # v3
Link: https://lore.kernel.org/r/164021579335.640689.2681324337038770579.stgit@warthog.procyon.org.uk/ # v4
Link: https://lore.kernel.org/r/3462849.1641593783@warthog.procyon.org.uk/ # v5
Link: https://lore.kernel.org/r/1318953.1642024578@warthog.procyon.org.uk/ # v6
---

 fs/cifs/Kconfig    |    2 
 fs/cifs/Makefile   |    2 
 fs/cifs/cache.c    |  105 ----------------
 fs/cifs/cifsfs.c   |   19 ++-
 fs/cifs/cifsglob.h |    5 -
 fs/cifs/connect.c  |   15 --
 fs/cifs/dir.c      |    5 +
 fs/cifs/file.c     |   70 +++++++----
 fs/cifs/fscache.c  |  333 ++++++++++++----------------------------------------
 fs/cifs/fscache.h  |  126 ++++++--------------
 fs/cifs/inode.c    |   19 ++-
 11 files changed, 197 insertions(+), 504 deletions(-)
 delete mode 100644 fs/cifs/cache.c

diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
index 346ae8716deb..3b7e3b9e4fd2 100644
--- a/fs/cifs/Kconfig
+++ b/fs/cifs/Kconfig
@@ -188,7 +188,7 @@ config CIFS_SMB_DIRECT
 
 config CIFS_FSCACHE
 	bool "Provide CIFS client caching support"
-	depends on CIFS=m && FSCACHE_OLD_API || CIFS=y && FSCACHE_OLD_API=y
+	depends on CIFS=m && FSCACHE || CIFS=y && FSCACHE=y
 	help
 	  Makes CIFS FS-Cache capable. Say Y here if you want your CIFS data
 	  to be cached locally on disk through the general filesystem cache
diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile
index 87fcacdf3de7..cc8fdcb35b71 100644
--- a/fs/cifs/Makefile
+++ b/fs/cifs/Makefile
@@ -25,7 +25,7 @@ cifs-$(CONFIG_CIFS_DFS_UPCALL) += cifs_dfs_ref.o dfs_cache.o
 
 cifs-$(CONFIG_CIFS_SWN_UPCALL) += netlink.o cifs_swn.o
 
-cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o cache.o
+cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o
 
 cifs-$(CONFIG_CIFS_SMB_DIRECT) += smbdirect.o
 
diff --git a/fs/cifs/cache.c b/fs/cifs/cache.c
deleted file mode 100644
index 8be57aaedab6..000000000000
--- a/fs/cifs/cache.c
+++ /dev/null
@@ -1,105 +0,0 @@
-// SPDX-License-Identifier: LGPL-2.1
-/*
- *   CIFS filesystem cache index structure definitions
- *
- *   Copyright (c) 2010 Novell, Inc.
- *   Authors(s): Suresh Jayaraman (sjayaraman@suse.de>
- *
- */
-#include "fscache.h"
-#include "cifs_debug.h"
-
-/*
- * CIFS filesystem definition for FS-Cache
- */
-struct fscache_netfs cifs_fscache_netfs = {
-	.name = "cifs",
-	.version = 0,
-};
-
-/*
- * Register CIFS for caching with FS-Cache
- */
-int cifs_fscache_register(void)
-{
-	return fscache_register_netfs(&cifs_fscache_netfs);
-}
-
-/*
- * Unregister CIFS for caching
- */
-void cifs_fscache_unregister(void)
-{
-	fscache_unregister_netfs(&cifs_fscache_netfs);
-}
-
-/*
- * Server object for FS-Cache
- */
-const struct fscache_cookie_def cifs_fscache_server_index_def = {
-	.name = "CIFS.server",
-	.type = FSCACHE_COOKIE_TYPE_INDEX,
-};
-
-static enum
-fscache_checkaux cifs_fscache_super_check_aux(void *cookie_netfs_data,
-					      const void *data,
-					      uint16_t datalen,
-					      loff_t object_size)
-{
-	struct cifs_fscache_super_auxdata auxdata;
-	const struct cifs_tcon *tcon = cookie_netfs_data;
-
-	if (datalen != sizeof(auxdata))
-		return FSCACHE_CHECKAUX_OBSOLETE;
-
-	memset(&auxdata, 0, sizeof(auxdata));
-	auxdata.resource_id = tcon->resource_id;
-	auxdata.vol_create_time = tcon->vol_create_time;
-	auxdata.vol_serial_number = tcon->vol_serial_number;
-
-	if (memcmp(data, &auxdata, datalen) != 0)
-		return FSCACHE_CHECKAUX_OBSOLETE;
-
-	return FSCACHE_CHECKAUX_OKAY;
-}
-
-/*
- * Superblock object for FS-Cache
- */
-const struct fscache_cookie_def cifs_fscache_super_index_def = {
-	.name = "CIFS.super",
-	.type = FSCACHE_COOKIE_TYPE_INDEX,
-	.check_aux = cifs_fscache_super_check_aux,
-};
-
-static enum
-fscache_checkaux cifs_fscache_inode_check_aux(void *cookie_netfs_data,
-					      const void *data,
-					      uint16_t datalen,
-					      loff_t object_size)
-{
-	struct cifs_fscache_inode_auxdata auxdata;
-	struct cifsInodeInfo *cifsi = cookie_netfs_data;
-
-	if (datalen != sizeof(auxdata))
-		return FSCACHE_CHECKAUX_OBSOLETE;
-
-	memset(&auxdata, 0, sizeof(auxdata));
-	auxdata.eof = cifsi->server_eof;
-	auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
-	auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
-	auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
-	auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
-
-	if (memcmp(data, &auxdata, datalen) != 0)
-		return FSCACHE_CHECKAUX_OBSOLETE;
-
-	return FSCACHE_CHECKAUX_OKAY;
-}
-
-const struct fscache_cookie_def cifs_fscache_inode_object_def = {
-	.name		= "CIFS.uniqueid",
-	.type		= FSCACHE_COOKIE_TYPE_DATAFILE,
-	.check_aux	= cifs_fscache_inode_check_aux,
-};
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index dca42aa87d30..26cf2193c9a2 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -396,6 +396,9 @@ static void
 cifs_evict_inode(struct inode *inode)
 {
 	truncate_inode_pages_final(&inode->i_data);
+	if (inode->i_state & I_PINNING_FSCACHE_WB)
+		cifs_fscache_unuse_inode_cookie(inode, true);
+	cifs_fscache_release_inode_cookie(inode);
 	clear_inode(inode);
 }
 
@@ -720,6 +723,12 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
 }
 #endif
 
+static int cifs_write_inode(struct inode *inode, struct writeback_control *wbc)
+{
+	fscache_unpin_writeback(wbc, cifs_inode_cookie(inode));
+	return 0;
+}
+
 static int cifs_drop_inode(struct inode *inode)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
@@ -732,6 +741,7 @@ static int cifs_drop_inode(struct inode *inode)
 static const struct super_operations cifs_super_ops = {
 	.statfs = cifs_statfs,
 	.alloc_inode = cifs_alloc_inode,
+	.write_inode	= cifs_write_inode,
 	.free_inode = cifs_free_inode,
 	.drop_inode	= cifs_drop_inode,
 	.evict_inode	= cifs_evict_inode,
@@ -1624,13 +1634,9 @@ init_cifs(void)
 		goto out_destroy_cifsoplockd_wq;
 	}
 
-	rc = cifs_fscache_register();
-	if (rc)
-		goto out_destroy_deferredclose_wq;
-
 	rc = cifs_init_inodecache();
 	if (rc)
-		goto out_unreg_fscache;
+		goto out_destroy_deferredclose_wq;
 
 	rc = cifs_init_mids();
 	if (rc)
@@ -1692,8 +1698,6 @@ init_cifs(void)
 	cifs_destroy_mids();
 out_destroy_inodecache:
 	cifs_destroy_inodecache();
-out_unreg_fscache:
-	cifs_fscache_unregister();
 out_destroy_deferredclose_wq:
 	destroy_workqueue(deferredclose_wq);
 out_destroy_cifsoplockd_wq:
@@ -1729,7 +1733,6 @@ exit_cifs(void)
 	cifs_destroy_request_bufs();
 	cifs_destroy_mids();
 	cifs_destroy_inodecache();
-	cifs_fscache_unregister();
 	destroy_workqueue(deferredclose_wq);
 	destroy_workqueue(cifsoplockd_wq);
 	destroy_workqueue(decrypt_wq);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index be74606724c7..ba6fbb1ad8f3 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -659,9 +659,6 @@ struct TCP_Server_Info {
 	unsigned int total_read; /* total amount of data read in this pass */
 	atomic_t in_send; /* requests trying to send */
 	atomic_t num_waiters;   /* blocked waiting to get in sendrecv */
-#ifdef CONFIG_CIFS_FSCACHE
-	struct fscache_cookie   *fscache; /* client index cache cookie */
-#endif
 #ifdef CONFIG_CIFS_STATS2
 	atomic_t num_cmds[NUMBER_OF_SMB2_COMMANDS]; /* total requests by cmd */
 	atomic_t smb2slowcmd[NUMBER_OF_SMB2_COMMANDS]; /* count resps > 1 sec */
@@ -1117,7 +1114,7 @@ struct cifs_tcon {
 	__u32 max_bytes_copy;
 #ifdef CONFIG_CIFS_FSCACHE
 	u64 resource_id;		/* server resource id */
-	struct fscache_cookie *fscache;	/* cookie for share */
+	struct fscache_volume *fscache;	/* cookie for share */
 #endif
 	struct list_head pending_opens;	/* list of incomplete opens */
 	struct cached_fid crfid; /* Cached root fid */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 1060164b984a..598be9890f2a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1396,10 +1396,6 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
 
 	cifs_crypto_secmech_release(server);
 
-	/* fscache server cookies are based on primary channel only */
-	if (!CIFS_SERVER_IS_CHAN(server))
-		cifs_fscache_release_client_cookie(server);
-
 	kfree(server->session_key.response);
 	server->session_key.response = NULL;
 	server->session_key.len = 0;
@@ -1559,14 +1555,6 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
 	list_add(&tcp_ses->tcp_ses_list, &cifs_tcp_ses_list);
 	spin_unlock(&cifs_tcp_ses_lock);
 
-	/* fscache server cookies are based on primary channel only */
-	if (!CIFS_SERVER_IS_CHAN(tcp_ses))
-		cifs_fscache_get_client_cookie(tcp_ses);
-#ifdef CONFIG_CIFS_FSCACHE
-	else
-		tcp_ses->fscache = tcp_ses->primary_server->fscache;
-#endif /* CONFIG_CIFS_FSCACHE */
-
 	/* queue echo request delayed work */
 	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
 
@@ -3069,7 +3057,8 @@ static int mount_get_conns(struct mount_ctx *mnt_ctx)
 	 * Inside cifs_fscache_get_super_cookie it checks
 	 * that we do not get super cookie twice.
 	 */
-	cifs_fscache_get_super_cookie(tcon);
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_FSCACHE)
+		cifs_fscache_get_super_cookie(tcon);
 
 out:
 	mnt_ctx->server = server;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 6e8e7cc26ae2..ce9b22aecfba 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -22,6 +22,7 @@
 #include "cifs_unicode.h"
 #include "fs_context.h"
 #include "cifs_ioctl.h"
+#include "fscache.h"
 
 static void
 renew_parental_timestamps(struct dentry *direntry)
@@ -507,8 +508,12 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
 			server->ops->close(xid, tcon, &fid);
 		cifs_del_pending_open(&open);
 		rc = -ENOMEM;
+		goto out;
 	}
 
+	fscache_use_cookie(cifs_inode_cookie(file_inode(file)),
+			   file->f_mode & FMODE_WRITE);
+
 out:
 	cifs_put_tlink(tlink);
 out_free_xid:
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 9fee3af83a73..fb77ca1a15d8 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -376,8 +376,6 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
 	struct cifsLockInfo *li, *tmp;
 	struct super_block *sb = inode->i_sb;
 
-	cifs_fscache_release_inode_cookie(inode);
-
 	/*
 	 * Delete any outstanding lock records. We'll lose them when the file
 	 * is closed anyway.
@@ -570,7 +568,7 @@ int cifs_open(struct inode *inode, struct file *file)
 			spin_lock(&CIFS_I(inode)->deferred_lock);
 			cifs_del_deferred_close(cfile);
 			spin_unlock(&CIFS_I(inode)->deferred_lock);
-			goto out;
+			goto use_cache;
 		} else {
 			_cifsFileInfo_put(cfile, true, false);
 		}
@@ -632,8 +630,6 @@ int cifs_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	cifs_fscache_set_inode_cookie(inode, file);
-
 	if ((oplock & CIFS_CREATE_ACTION) && !posix_open_ok && tcon->unix_ext) {
 		/*
 		 * Time to set mode which we can not set earlier due to
@@ -652,6 +648,19 @@ int cifs_open(struct inode *inode, struct file *file)
 				       cfile->pid);
 	}
 
+use_cache:
+	fscache_use_cookie(cifs_inode_cookie(file_inode(file)),
+			   file->f_mode & FMODE_WRITE);
+	if (file->f_flags & O_DIRECT &&
+	    (!((file->f_flags & O_ACCMODE) != O_RDONLY) ||
+	     file->f_flags & O_APPEND)) {
+		struct cifs_fscache_inode_coherency_data cd;
+		cifs_fscache_fill_coherency(file_inode(file), &cd);
+		fscache_invalidate(cifs_inode_cookie(file_inode(file)),
+				   &cd, i_size_read(file_inode(file)),
+				   FSCACHE_INVAL_DIO_WRITE);
+	}
+
 out:
 	free_dentry_path(page);
 	free_xid(xid);
@@ -876,6 +885,8 @@ int cifs_close(struct inode *inode, struct file *file)
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifs_deferred_close *dclose;
 
+	cifs_fscache_unuse_inode_cookie(inode, file->f_mode & FMODE_WRITE);
+
 	if (file->private_data != NULL) {
 		cfile = file->private_data;
 		file->private_data = NULL;
@@ -886,7 +897,6 @@ int cifs_close(struct inode *inode, struct file *file)
 		    dclose) {
 			if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
 				inode->i_ctime = inode->i_mtime = current_time(inode);
-				cifs_fscache_update_inode_cookie(inode);
 			}
 			spin_lock(&cinode->deferred_lock);
 			cifs_add_deferred_close(cfile, dclose);
@@ -4198,10 +4208,12 @@ static vm_fault_t
 cifs_page_mkwrite(struct vm_fault *vmf)
 {
 	struct page *page = vmf->page;
-	struct file *file = vmf->vma->vm_file;
-	struct inode *inode = file_inode(file);
 
-	cifs_fscache_wait_on_page_write(inode, page);
+#ifdef CONFIG_CIFS_FSCACHE
+	if (PageFsCache(page) &&
+	    wait_on_page_fscache_killable(page) < 0)
+		return VM_FAULT_RETRY;
+#endif
 
 	lock_page(page);
 	return VM_FAULT_LOCKED;
@@ -4275,8 +4287,6 @@ cifs_readv_complete(struct work_struct *work)
 		if (rdata->result == 0 ||
 		    (rdata->result == -EAGAIN && got_bytes))
 			cifs_readpage_to_fscache(rdata->mapping->host, page);
-		else
-			cifs_fscache_uncache_page(rdata->mapping->host, page);
 
 		got_bytes -= min_t(unsigned int, PAGE_SIZE, got_bytes);
 
@@ -4593,11 +4603,6 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 		kref_put(&rdata->refcount, cifs_readdata_release);
 	}
 
-	/* Any pages that have been shown to fscache but didn't get added to
-	 * the pagecache must be uncached before they get returned to the
-	 * allocator.
-	 */
-	cifs_fscache_readpages_cancel(mapping->host, page_list);
 	free_xid(xid);
 	return rc;
 }
@@ -4801,17 +4806,19 @@ static int cifs_release_page(struct page *page, gfp_t gfp)
 {
 	if (PagePrivate(page))
 		return 0;
-
-	return cifs_fscache_release_page(page, gfp);
+	if (PageFsCache(page)) {
+		if (current_is_kswapd() || !(gfp & __GFP_FS))
+			return false;
+		wait_on_page_fscache(page);
+	}
+	fscache_note_page_release(cifs_inode_cookie(page->mapping->host));
+	return true;
 }
 
 static void cifs_invalidate_page(struct page *page, unsigned int offset,
 				 unsigned int length)
 {
-	struct cifsInodeInfo *cifsi = CIFS_I(page->mapping->host);
-
-	if (offset == 0 && length == PAGE_SIZE)
-		cifs_fscache_invalidate_page(page, &cifsi->vfs_inode);
+	wait_on_page_fscache(page);
 }
 
 static int cifs_launder_page(struct page *page)
@@ -4831,7 +4838,7 @@ static int cifs_launder_page(struct page *page)
 	if (clear_page_dirty_for_io(page))
 		rc = cifs_writepage_locked(page, &wbc);
 
-	cifs_fscache_invalidate_page(page, page->mapping->host);
+	wait_on_page_fscache(page);
 	return rc;
 }
 
@@ -4988,6 +4995,19 @@ static void cifs_swap_deactivate(struct file *file)
 	/* do we need to unpin (or unlock) the file */
 }
 
+/*
+ * Mark a page as having been made dirty and thus needing writeback.  We also
+ * need to pin the cache object to write back to.
+ */
+#ifdef CONFIG_CIFS_FSCACHE
+static int cifs_set_page_dirty(struct page *page)
+{
+	return fscache_set_page_dirty(page, cifs_inode_cookie(page->mapping->host));
+}
+#else
+#define cifs_set_page_dirty __set_page_dirty_nobuffers
+#endif
+
 const struct address_space_operations cifs_addr_ops = {
 	.readpage = cifs_readpage,
 	.readpages = cifs_readpages,
@@ -4995,7 +5015,7 @@ const struct address_space_operations cifs_addr_ops = {
 	.writepages = cifs_writepages,
 	.write_begin = cifs_write_begin,
 	.write_end = cifs_write_end,
-	.set_page_dirty = __set_page_dirty_nobuffers,
+	.set_page_dirty = cifs_set_page_dirty,
 	.releasepage = cifs_release_page,
 	.direct_IO = cifs_direct_io,
 	.invalidatepage = cifs_invalidate_page,
@@ -5020,7 +5040,7 @@ const struct address_space_operations cifs_addr_ops_smallbuf = {
 	.writepages = cifs_writepages,
 	.write_begin = cifs_write_begin,
 	.write_end = cifs_write_end,
-	.set_page_dirty = __set_page_dirty_nobuffers,
+	.set_page_dirty = cifs_set_page_dirty,
 	.releasepage = cifs_release_page,
 	.invalidatepage = cifs_invalidate_page,
 	.launder_page = cifs_launder_page,
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index 003c5f1f4dfb..efaac4d5ff55 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -12,250 +12,136 @@
 #include "cifs_fs_sb.h"
 #include "cifsproto.h"
 
-/*
- * Key layout of CIFS server cache index object
- */
-struct cifs_server_key {
-	__u64 conn_id;
-} __packed;
-
-/*
- * Get a cookie for a server object keyed by {IPaddress,port,family} tuple
- */
-void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
-{
-	struct cifs_server_key key;
-
-	/*
-	 * Check if cookie was already initialized so don't reinitialize it.
-	 * In the future, as we integrate with newer fscache features,
-	 * we may want to instead add a check if cookie has changed
-	 */
-	if (server->fscache)
-		return;
-
-	memset(&key, 0, sizeof(key));
-	key.conn_id = server->conn_id;
-
-	server->fscache =
-		fscache_acquire_cookie(cifs_fscache_netfs.primary_index,
-				       &cifs_fscache_server_index_def,
-				       &key, sizeof(key),
-				       NULL, 0,
-				       server, 0, true);
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
-		 __func__, server, server->fscache);
-}
-
-void cifs_fscache_release_client_cookie(struct TCP_Server_Info *server)
+static void cifs_fscache_fill_volume_coherency(
+	struct cifs_tcon *tcon,
+	struct cifs_fscache_volume_coherency_data *cd)
 {
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
-		 __func__, server, server->fscache);
-	fscache_relinquish_cookie(server->fscache, NULL, false);
-	server->fscache = NULL;
+	memset(cd, 0, sizeof(*cd));
+	cd->resource_id		= cpu_to_le64(tcon->resource_id);
+	cd->vol_create_time	= tcon->vol_create_time;
+	cd->vol_serial_number	= cpu_to_le32(tcon->vol_serial_number);
 }
 
-void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
+int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
 {
+	struct cifs_fscache_volume_coherency_data cd;
 	struct TCP_Server_Info *server = tcon->ses->server;
+	struct fscache_volume *vcookie;
+	const struct sockaddr *sa = (struct sockaddr *)&server->dstaddr;
+	size_t slen, i;
 	char *sharename;
-	struct cifs_fscache_super_auxdata auxdata;
+	char *key;
+	int ret = -ENOMEM;
 
-	/*
-	 * Check if cookie was already initialized so don't reinitialize it.
-	 * In the future, as we integrate with newer fscache features,
-	 * we may want to instead add a check if cookie has changed
-	 */
-	if (tcon->fscache)
-		return;
+	tcon->fscache = NULL;
+	switch (sa->sa_family) {
+	case AF_INET:
+	case AF_INET6:
+		break;
+	default:
+		cifs_dbg(VFS, "Unknown network family '%d'\n", sa->sa_family);
+		return -EINVAL;
+	}
+
+	memset(&key, 0, sizeof(key));
 
 	sharename = extract_sharename(tcon->treeName);
 	if (IS_ERR(sharename)) {
 		cifs_dbg(FYI, "%s: couldn't extract sharename\n", __func__);
-		tcon->fscache = NULL;
-		return;
+		return -EINVAL;
 	}
 
-	memset(&auxdata, 0, sizeof(auxdata));
-	auxdata.resource_id = tcon->resource_id;
-	auxdata.vol_create_time = tcon->vol_create_time;
-	auxdata.vol_serial_number = tcon->vol_serial_number;
+	slen = strlen(sharename);
+	for (i = 0; i < slen; i++)
+		if (sharename[i] == '/')
+			sharename[i] = ';';
+
+	key = kasprintf(GFP_KERNEL, "cifs,%pISpc,%s", sa, sharename);
+	if (!key)
+		goto out;
+
+	cifs_fscache_fill_volume_coherency(tcon, &cd);
+	vcookie = fscache_acquire_volume(key,
+					 NULL, /* preferred_cache */
+					 &cd, sizeof(cd));
+	cifs_dbg(FYI, "%s: (%s/0x%p)\n", __func__, key, vcookie);
+	if (IS_ERR(vcookie)) {
+		if (vcookie != ERR_PTR(-EBUSY)) {
+			ret = PTR_ERR(vcookie);
+			goto out_2;
+		}
+		pr_err("Cache volume key already in use (%s)\n", key);
+		vcookie = NULL;
+	}
 
-	tcon->fscache =
-		fscache_acquire_cookie(server->fscache,
-				       &cifs_fscache_super_index_def,
-				       sharename, strlen(sharename),
-				       &auxdata, sizeof(auxdata),
-				       tcon, 0, true);
+	tcon->fscache = vcookie;
+	ret = 0;
+out_2:
+	kfree(key);
+out:
 	kfree(sharename);
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
-		 __func__, server->fscache, tcon->fscache);
+	return ret;
 }
 
 void cifs_fscache_release_super_cookie(struct cifs_tcon *tcon)
 {
-	struct cifs_fscache_super_auxdata auxdata;
-
-	memset(&auxdata, 0, sizeof(auxdata));
-	auxdata.resource_id = tcon->resource_id;
-	auxdata.vol_create_time = tcon->vol_create_time;
-	auxdata.vol_serial_number = tcon->vol_serial_number;
+	struct cifs_fscache_volume_coherency_data cd;
 
 	cifs_dbg(FYI, "%s: (0x%p)\n", __func__, tcon->fscache);
-	fscache_relinquish_cookie(tcon->fscache, &auxdata, false);
-	tcon->fscache = NULL;
-}
-
-static void cifs_fscache_acquire_inode_cookie(struct cifsInodeInfo *cifsi,
-					      struct cifs_tcon *tcon)
-{
-	struct cifs_fscache_inode_auxdata auxdata;
 
-	memset(&auxdata, 0, sizeof(auxdata));
-	auxdata.eof = cifsi->server_eof;
-	auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
-	auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
-	auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
-	auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
-
-	cifsi->fscache =
-		fscache_acquire_cookie(tcon->fscache,
-				       &cifs_fscache_inode_object_def,
-				       &cifsi->uniqueid, sizeof(cifsi->uniqueid),
-				       &auxdata, sizeof(auxdata),
-				       cifsi, cifsi->vfs_inode.i_size, true);
+	cifs_fscache_fill_volume_coherency(tcon, &cd);
+	fscache_relinquish_volume(tcon->fscache, &cd, false);
+	tcon->fscache = NULL;
 }
 
-static void cifs_fscache_enable_inode_cookie(struct inode *inode)
+void cifs_fscache_get_inode_cookie(struct inode *inode)
 {
+	struct cifs_fscache_inode_coherency_data cd;
 	struct cifsInodeInfo *cifsi = CIFS_I(inode);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
 	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
 
-	if (cifsi->fscache)
-		return;
-
-	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_FSCACHE))
-		return;
-
-	cifs_fscache_acquire_inode_cookie(cifsi, tcon);
+	cifs_fscache_fill_coherency(&cifsi->vfs_inode, &cd);
 
-	cifs_dbg(FYI, "%s: got FH cookie (0x%p/0x%p)\n",
-		 __func__, tcon->fscache, cifsi->fscache);
+	cifsi->fscache =
+		fscache_acquire_cookie(tcon->fscache, 0,
+				       &cifsi->uniqueid, sizeof(cifsi->uniqueid),
+				       &cd, sizeof(cd),
+				       i_size_read(&cifsi->vfs_inode));
 }
 
-void cifs_fscache_release_inode_cookie(struct inode *inode)
+void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update)
 {
-	struct cifs_fscache_inode_auxdata auxdata;
-	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-
-	if (cifsi->fscache) {
-		memset(&auxdata, 0, sizeof(auxdata));
-		auxdata.eof = cifsi->server_eof;
-		auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
-		auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
-		auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
-		auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
+	if (update) {
+		struct cifs_fscache_inode_coherency_data cd;
+		loff_t i_size = i_size_read(inode);
 
-		cifs_dbg(FYI, "%s: (0x%p)\n", __func__, cifsi->fscache);
-		/* fscache_relinquish_cookie does not seem to update auxdata */
-		fscache_update_cookie(cifsi->fscache, &auxdata);
-		fscache_relinquish_cookie(cifsi->fscache, &auxdata, false);
-		cifsi->fscache = NULL;
+		cifs_fscache_fill_coherency(inode, &cd);
+		fscache_unuse_cookie(cifs_inode_cookie(inode), &cd, &i_size);
+	} else {
+		fscache_unuse_cookie(cifs_inode_cookie(inode), NULL, NULL);
 	}
 }
 
-void cifs_fscache_update_inode_cookie(struct inode *inode)
+void cifs_fscache_release_inode_cookie(struct inode *inode)
 {
-	struct cifs_fscache_inode_auxdata auxdata;
 	struct cifsInodeInfo *cifsi = CIFS_I(inode);
 
 	if (cifsi->fscache) {
-		memset(&auxdata, 0, sizeof(auxdata));
-		auxdata.eof = cifsi->server_eof;
-		auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
-		auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
-		auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
-		auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
-
 		cifs_dbg(FYI, "%s: (0x%p)\n", __func__, cifsi->fscache);
-		fscache_update_cookie(cifsi->fscache, &auxdata);
-	}
-}
-
-void cifs_fscache_set_inode_cookie(struct inode *inode, struct file *filp)
-{
-	cifs_fscache_enable_inode_cookie(inode);
-}
-
-void cifs_fscache_reset_inode_cookie(struct inode *inode)
-{
-	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
-	struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
-	struct fscache_cookie *old = cifsi->fscache;
-
-	if (cifsi->fscache) {
-		/* retire the current fscache cache and get a new one */
-		fscache_relinquish_cookie(cifsi->fscache, NULL, true);
-
-		cifs_fscache_acquire_inode_cookie(cifsi, tcon);
-		cifs_dbg(FYI, "%s: new cookie 0x%p oldcookie 0x%p\n",
-			 __func__, cifsi->fscache, old);
+		fscache_relinquish_cookie(cifsi->fscache, false);
+		cifsi->fscache = NULL;
 	}
 }
 
-int cifs_fscache_release_page(struct page *page, gfp_t gfp)
-{
-	if (PageFsCache(page)) {
-		struct inode *inode = page->mapping->host;
-		struct cifsInodeInfo *cifsi = CIFS_I(inode);
-
-		cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
-			 __func__, page, cifsi->fscache);
-		if (!fscache_maybe_release_page(cifsi->fscache, page, gfp))
-			return 0;
-	}
-
-	return 1;
-}
-
-static void cifs_readpage_from_fscache_complete(struct page *page, void *ctx,
-						int error)
-{
-	cifs_dbg(FYI, "%s: (0x%p/%d)\n", __func__, page, error);
-	if (!error)
-		SetPageUptodate(page);
-	unlock_page(page);
-}
-
 /*
  * Retrieve a page from FS-Cache
  */
 int __cifs_readpage_from_fscache(struct inode *inode, struct page *page)
 {
-	int ret;
-
 	cifs_dbg(FYI, "%s: (fsc:%p, p:%p, i:0x%p\n",
 		 __func__, CIFS_I(inode)->fscache, page, inode);
-	ret = fscache_read_or_alloc_page(CIFS_I(inode)->fscache, page,
-					 cifs_readpage_from_fscache_complete,
-					 NULL,
-					 GFP_KERNEL);
-	switch (ret) {
-
-	case 0: /* page found in fscache, read submitted */
-		cifs_dbg(FYI, "%s: submitted\n", __func__);
-		return ret;
-	case -ENOBUFS:	/* page won't be cached */
-	case -ENODATA:	/* page not in cache */
-		cifs_dbg(FYI, "%s: %d\n", __func__, ret);
-		return 1;
-
-	default:
-		cifs_dbg(VFS, "unknown error ret = %d\n", ret);
-	}
-	return ret;
+	return -ENOBUFS; // Needs conversion to using netfslib
 }
 
 /*
@@ -266,78 +152,19 @@ int __cifs_readpages_from_fscache(struct inode *inode,
 				struct list_head *pages,
 				unsigned *nr_pages)
 {
-	int ret;
-
 	cifs_dbg(FYI, "%s: (0x%p/%u/0x%p)\n",
 		 __func__, CIFS_I(inode)->fscache, *nr_pages, inode);
-	ret = fscache_read_or_alloc_pages(CIFS_I(inode)->fscache, mapping,
-					  pages, nr_pages,
-					  cifs_readpage_from_fscache_complete,
-					  NULL,
-					  mapping_gfp_mask(mapping));
-	switch (ret) {
-	case 0:	/* read submitted to the cache for all pages */
-		cifs_dbg(FYI, "%s: submitted\n", __func__);
-		return ret;
-
-	case -ENOBUFS:	/* some pages are not cached and can't be */
-	case -ENODATA:	/* some pages are not cached */
-		cifs_dbg(FYI, "%s: no page\n", __func__);
-		return 1;
-
-	default:
-		cifs_dbg(FYI, "unknown error ret = %d\n", ret);
-	}
-
-	return ret;
+	return -ENOBUFS; // Needs conversion to using netfslib
 }
 
 void __cifs_readpage_to_fscache(struct inode *inode, struct page *page)
 {
 	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-	int ret;
 
 	WARN_ON(!cifsi->fscache);
 
 	cifs_dbg(FYI, "%s: (fsc: %p, p: %p, i: %p)\n",
 		 __func__, cifsi->fscache, page, inode);
-	ret = fscache_write_page(cifsi->fscache, page,
-				 cifsi->vfs_inode.i_size, GFP_KERNEL);
-	if (ret != 0)
-		fscache_uncache_page(cifsi->fscache, page);
-}
-
-void __cifs_fscache_readpages_cancel(struct inode *inode, struct list_head *pages)
-{
-	cifs_dbg(FYI, "%s: (fsc: %p, i: %p)\n",
-		 __func__, CIFS_I(inode)->fscache, inode);
-	fscache_readpages_cancel(CIFS_I(inode)->fscache, pages);
-}
-
-void __cifs_fscache_invalidate_page(struct page *page, struct inode *inode)
-{
-	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-	struct fscache_cookie *cookie = cifsi->fscache;
-
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
-	fscache_wait_on_page_write(cookie, page);
-	fscache_uncache_page(cookie, page);
-}
-
-void __cifs_fscache_wait_on_page_write(struct inode *inode, struct page *page)
-{
-	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-	struct fscache_cookie *cookie = cifsi->fscache;
-
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
-	fscache_wait_on_page_write(cookie, page);
-}
-
-void __cifs_fscache_uncache_page(struct inode *inode, struct page *page)
-{
-	struct cifsInodeInfo *cifsi = CIFS_I(inode);
-	struct fscache_cookie *cookie = cifsi->fscache;
 
-	cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
-	fscache_uncache_page(cookie, page);
+	// Needs conversion to using netfslib
 }
diff --git a/fs/cifs/fscache.h b/fs/cifs/fscache.h
index 9baa1d0f22bd..0fc3f9252c84 100644
--- a/fs/cifs/fscache.h
+++ b/fs/cifs/fscache.h
@@ -13,84 +13,62 @@
 
 #include "cifsglob.h"
 
-#ifdef CONFIG_CIFS_FSCACHE
-
 /*
- * Auxiliary data attached to CIFS superblock within the cache
+ * Coherency data attached to CIFS volume within the cache
  */
-struct cifs_fscache_super_auxdata {
-	u64	resource_id;		/* unique server resource id */
+struct cifs_fscache_volume_coherency_data {
+	__le64	resource_id;		/* unique server resource id */
 	__le64	vol_create_time;
-	u32	vol_serial_number;
+	__le32	vol_serial_number;
 } __packed;
 
 /*
- * Auxiliary data attached to CIFS inode within the cache
+ * Coherency data attached to CIFS inode within the cache.
  */
-struct cifs_fscache_inode_auxdata {
-	u64 last_write_time_sec;
-	u64 last_change_time_sec;
-	u32 last_write_time_nsec;
-	u32 last_change_time_nsec;
-	u64 eof;
+struct cifs_fscache_inode_coherency_data {
+	__le64 last_write_time_sec;
+	__le64 last_change_time_sec;
+	__le32 last_write_time_nsec;
+	__le32 last_change_time_nsec;
 };
 
-/*
- * cache.c
- */
-extern struct fscache_netfs cifs_fscache_netfs;
-extern const struct fscache_cookie_def cifs_fscache_server_index_def;
-extern const struct fscache_cookie_def cifs_fscache_super_index_def;
-extern const struct fscache_cookie_def cifs_fscache_inode_object_def;
-
-extern int cifs_fscache_register(void);
-extern void cifs_fscache_unregister(void);
+#ifdef CONFIG_CIFS_FSCACHE
 
 /*
  * fscache.c
  */
-extern void cifs_fscache_get_client_cookie(struct TCP_Server_Info *);
-extern void cifs_fscache_release_client_cookie(struct TCP_Server_Info *);
-extern void cifs_fscache_get_super_cookie(struct cifs_tcon *);
+extern int cifs_fscache_get_super_cookie(struct cifs_tcon *);
 extern void cifs_fscache_release_super_cookie(struct cifs_tcon *);
 
+extern void cifs_fscache_get_inode_cookie(struct inode *);
 extern void cifs_fscache_release_inode_cookie(struct inode *);
-extern void cifs_fscache_update_inode_cookie(struct inode *inode);
-extern void cifs_fscache_set_inode_cookie(struct inode *, struct file *);
-extern void cifs_fscache_reset_inode_cookie(struct inode *);
+extern void cifs_fscache_unuse_inode_cookie(struct inode *, bool);
+
+static inline
+void cifs_fscache_fill_coherency(struct inode *inode,
+				 struct cifs_fscache_inode_coherency_data *cd)
+{
+	struct cifsInodeInfo *cifsi = CIFS_I(inode);
+
+	memset(cd, 0, sizeof(*cd));
+	cd->last_write_time_sec   = cpu_to_le64(cifsi->vfs_inode.i_mtime.tv_sec);
+	cd->last_write_time_nsec  = cpu_to_le32(cifsi->vfs_inode.i_mtime.tv_nsec);
+	cd->last_change_time_sec  = cpu_to_le64(cifsi->vfs_inode.i_ctime.tv_sec);
+	cd->last_change_time_nsec = cpu_to_le32(cifsi->vfs_inode.i_ctime.tv_nsec);
+}
+
 
-extern void __cifs_fscache_invalidate_page(struct page *, struct inode *);
-extern void __cifs_fscache_wait_on_page_write(struct inode *inode, struct page *page);
-extern void __cifs_fscache_uncache_page(struct inode *inode, struct page *page);
 extern int cifs_fscache_release_page(struct page *page, gfp_t gfp);
 extern int __cifs_readpage_from_fscache(struct inode *, struct page *);
 extern int __cifs_readpages_from_fscache(struct inode *,
 					 struct address_space *,
 					 struct list_head *,
 					 unsigned *);
-extern void __cifs_fscache_readpages_cancel(struct inode *, struct list_head *);
-
 extern void __cifs_readpage_to_fscache(struct inode *, struct page *);
 
-static inline void cifs_fscache_invalidate_page(struct page *page,
-					       struct inode *inode)
+static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode)
 {
-	if (PageFsCache(page))
-		__cifs_fscache_invalidate_page(page, inode);
-}
-
-static inline void cifs_fscache_wait_on_page_write(struct inode *inode,
-						   struct page *page)
-{
-	if (PageFsCache(page))
-		__cifs_fscache_wait_on_page_write(inode, page);
-}
-
-static inline void cifs_fscache_uncache_page(struct inode *inode,
-						   struct page *page)
-{
-	if (PageFsCache(page))
-		__cifs_fscache_uncache_page(inode, page);
+	return CIFS_I(inode)->fscache;
 }
 
 static inline int cifs_readpage_from_fscache(struct inode *inode,
@@ -120,41 +98,20 @@ static inline void cifs_readpage_to_fscache(struct inode *inode,
 		__cifs_readpage_to_fscache(inode, page);
 }
 
-static inline void cifs_fscache_readpages_cancel(struct inode *inode,
-						 struct list_head *pages)
+#else /* CONFIG_CIFS_FSCACHE */
+static inline
+void cifs_fscache_fill_coherency(struct inode *inode,
+				 struct cifs_fscache_inode_coherency_data *cd)
 {
-	if (CIFS_I(inode)->fscache)
-		return __cifs_fscache_readpages_cancel(inode, pages);
 }
 
-#else /* CONFIG_CIFS_FSCACHE */
-static inline int cifs_fscache_register(void) { return 0; }
-static inline void cifs_fscache_unregister(void) {}
-
-static inline void
-cifs_fscache_get_client_cookie(struct TCP_Server_Info *server) {}
-static inline void
-cifs_fscache_release_client_cookie(struct TCP_Server_Info *server) {}
-static inline void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon) {}
-static inline void
-cifs_fscache_release_super_cookie(struct cifs_tcon *tcon) {}
+static inline int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon) { return 0; }
+static inline void cifs_fscache_release_super_cookie(struct cifs_tcon *tcon) {}
 
+static inline void cifs_fscache_get_inode_cookie(struct inode *inode) {}
 static inline void cifs_fscache_release_inode_cookie(struct inode *inode) {}
-static inline void cifs_fscache_update_inode_cookie(struct inode *inode) {}
-static inline void cifs_fscache_set_inode_cookie(struct inode *inode,
-						 struct file *filp) {}
-static inline void cifs_fscache_reset_inode_cookie(struct inode *inode) {}
-static inline int cifs_fscache_release_page(struct page *page, gfp_t gfp)
-{
-	return 1; /* May release page */
-}
-
-static inline void cifs_fscache_invalidate_page(struct page *page,
-			struct inode *inode) {}
-static inline void cifs_fscache_wait_on_page_write(struct inode *inode,
-						   struct page *page) {}
-static inline void cifs_fscache_uncache_page(struct inode *inode,
-						   struct page *page) {}
+static inline void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update) {}
+static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode) { return NULL; }
 
 static inline int
 cifs_readpage_from_fscache(struct inode *inode, struct page *page)
@@ -173,11 +130,6 @@ static inline int cifs_readpages_from_fscache(struct inode *inode,
 static inline void cifs_readpage_to_fscache(struct inode *inode,
 			struct page *page) {}
 
-static inline void cifs_fscache_readpages_cancel(struct inode *inode,
-						 struct list_head *pages)
-{
-}
-
 #endif /* CONFIG_CIFS_FSCACHE */
 
 #endif /* _CIFS_FSCACHE_H */
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 279622e4eb1c..9b93e7d3e0e1 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1298,10 +1298,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
 			inode->i_flags |= S_NOATIME | S_NOCMTIME;
 		if (inode->i_state & I_NEW) {
 			inode->i_ino = hash;
-#ifdef CONFIG_CIFS_FSCACHE
-			/* initialize per-inode cache cookie pointer */
-			CIFS_I(inode)->fscache = NULL;
-#endif
+			cifs_fscache_get_inode_cookie(inode);
 			unlock_new_inode(inode);
 		}
 	}
@@ -1370,6 +1367,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 		iget_failed(inode);
 		inode = ERR_PTR(rc);
 	}
+
 out:
 	kfree(path);
 	free_xid(xid);
@@ -2257,6 +2255,8 @@ cifs_dentry_needs_reval(struct dentry *dentry)
 int
 cifs_invalidate_mapping(struct inode *inode)
 {
+	struct cifs_fscache_inode_coherency_data cd;
+	struct cifsInodeInfo *cifsi = CIFS_I(inode);
 	int rc = 0;
 
 	if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
@@ -2266,7 +2266,8 @@ cifs_invalidate_mapping(struct inode *inode)
 				 __func__, inode);
 	}
 
-	cifs_fscache_reset_inode_cookie(inode);
+	cifs_fscache_fill_coherency(&cifsi->vfs_inode, &cd);
+	fscache_invalidate(cifs_inode_cookie(inode), &cd, i_size_read(inode), 0);
 	return rc;
 }
 
@@ -2771,8 +2772,10 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 		goto out;
 
 	if ((attrs->ia_valid & ATTR_SIZE) &&
-	    attrs->ia_size != i_size_read(inode))
+	    attrs->ia_size != i_size_read(inode)) {
 		truncate_setsize(inode, attrs->ia_size);
+		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
+	}
 
 	setattr_copy(&init_user_ns, inode, attrs);
 	mark_inode_dirty(inode);
@@ -2967,8 +2970,10 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 		goto cifs_setattr_exit;
 
 	if ((attrs->ia_valid & ATTR_SIZE) &&
-	    attrs->ia_size != i_size_read(inode))
+	    attrs->ia_size != i_size_read(inode)) {
 		truncate_setsize(inode, attrs->ia_size);
+		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
+	}
 
 	setattr_copy(&init_user_ns, inode, attrs);
 	mark_inode_dirty(inode);



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
@ 2022-01-18 16:23   ` Christoph Hellwig
  2022-01-18 17:40   ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2022-01-18 16:23 UTC (permalink / raw)
  To: David Howells
  Cc: linux-cachefs, Jeff Layton, Trond Myklebust, Anna Schumaker,
	Steve French, Dominique Martinet, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

On Tue, Jan 18, 2022 at 01:54:54PM +0000, David Howells wrote:
> Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
> common practice for the other inode flags[1].

Please fix the flag to have a sensible name first, as the naming of the
flag and this new helper is utterly wrong as we already discussed.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
  2022-01-18 16:23   ` Christoph Hellwig
@ 2022-01-18 17:40   ` David Howells
  2022-01-19  5:20     ` Christoph Hellwig
  2022-01-19  9:18     ` David Howells
  1 sibling, 2 replies; 27+ messages in thread
From: David Howells @ 2022-01-18 17:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Jan 18, 2022 at 01:54:54PM +0000, David Howells wrote:
> > Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
> > common practice for the other inode flags[1].
> 
> Please fix the flag to have a sensible name first, as the naming of the
> flag and this new helper is utterly wrong as we already discussed.

And I suggested a new name, which you didn't comment on.

David


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-18 17:40   ` David Howells
@ 2022-01-19  5:20     ` Christoph Hellwig
  2022-01-19  9:18     ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2022-01-19  5:20 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

On Tue, Jan 18, 2022 at 05:40:14PM +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Tue, Jan 18, 2022 at 01:54:54PM +0000, David Howells wrote:
> > > Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
> > > common practice for the other inode flags[1].
> > 
> > Please fix the flag to have a sensible name first, as the naming of the
> > flag and this new helper is utterly wrong as we already discussed.
> 
> And I suggested a new name, which you didn't comment on.

Again, look at the semantics of the flag:  The only thing it does in the
VFS is to prevent a rmdir.  So you might want to name it after that.

Or in fact drop the flag entirely.  We don't have that kind of
protection for other in-kernel file use or important userspace daemons
either.  I can't see why cachefiles is the magic snowflake here that
suddenly needs semantics no one else has.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/11] cifs: Support fscache indexing rewrite
  2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
@ 2022-01-19  5:31   ` Steve French
  2022-01-19  8:32   ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Steve French @ 2022-01-19  5:31 UTC (permalink / raw)
  To: David Howells, CIFS; +Cc: Jeff Layton, Shyam Prasad N

Noticed minor checkpatch issues - but no obvious issues (yet) but I
need to run tests on it.

$ scripts/checkpatch.pl 0001-cifs-Support-fscache-indexing-rewrite.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#153:
deleted file mode 100644

WARNING: Missing a blank line after declarations
#460: FILE: fs/cifs/file.c:658:
+ struct cifs_fscache_inode_coherency_data cd;
+ cifs_fscache_fill_coherency(file_inode(file), &cd);

WARNING: function definition argument 'struct inode *' should also
have an identifier name
#1071: FILE: fs/cifs/fscache.h:43:
+extern void cifs_fscache_get_inode_cookie(struct inode *);

WARNING: function definition argument 'struct inode *' should also
have an identifier name
#1076: FILE: fs/cifs/fscache.h:45:
+extern void cifs_fscache_unuse_inode_cookie(struct inode *, bool);

WARNING: function definition argument 'bool' should also have an identifier name
#1076: FILE: fs/cifs/fscache.h:45:
+extern void cifs_fscache_unuse_inode_cookie(struct inode *, bool);

On Tue, Jan 18, 2022 at 7:55 AM David Howells <dhowells@redhat.com> wrote:
>
> Change the cifs filesystem to take account of the changes to fscache's
> indexing rewrite and reenable caching in cifs.
>
> The following changes have been made:
>
>  (1) The fscache_netfs struct is no more, and there's no need to register
>      the filesystem as a whole.
>
>  (2) The session cookie is now an fscache_volume cookie, allocated with
>      fscache_acquire_volume().  That takes three parameters: a string
>      representing the "volume" in the index, a string naming the cache to
>      use (or NULL) and a u64 that conveys coherency metadata for the
>      volume.
>
>      For cifs, I've made it render the volume name string as:
>
>         "cifs,<ipaddress>,<sharename>"
>
>      where the sharename has '/' characters replaced with ';'.
>
>      This probably needs rethinking a bit as the total name could exceed
>      the maximum filename component length.
>
>      Further, the coherency data is currently just set to 0.  It needs
>      something else doing with it - I wonder if it would suffice simply to
>      sum the resource_id, vol_create_time and vol_serial_number or maybe
>      hash them.
>
>  (3) The fscache_cookie_def is no more and needed information is passed
>      directly to fscache_acquire_cookie().  The cache no longer calls back
>      into the filesystem, but rather metadata changes are indicated at
>      other times.
>
>      fscache_acquire_cookie() is passed the same keying and coherency
>      information as before.
>
>  (4) The functions to set/reset cookies are removed and
>      fscache_use_cookie() and fscache_unuse_cookie() are used instead.
>
>      fscache_use_cookie() is passed a flag to indicate if the cookie is
>      opened for writing.  fscache_unuse_cookie() is passed updates for the
>      metadata if we changed it (ie. if the file was opened for writing).
>
>      These are called when the file is opened or closed.
>
>  (5) cifs_setattr_*() are made to call fscache_resize() to change the size
>      of the cache object.
>
>  (6) The functions to read and write data are stubbed out pending a
>      conversion to use netfslib.
>
> Changes
> =======
> ver #7:
>  - Removed the accidentally added-back call to get the super cookie in
>    cifs_root_iget().
>  - Fixed the right call to cifs_fscache_get_super_cookie() to take account
>    of the "-o fsc" mount flag.
>
> ver #6:
>  - Moved the change of gfpflags_allow_blocking() to current_is_kswapd() for
>    cifs here.
>  - Fixed one of the error paths in cifs_atomic_open() to jump around the
>    call to use the cookie.
>  - Fixed an additional successful return in the middle of cifs_open() to
>    use the cookie on the way out.
>  - Only get a volume cookie (and thus inode cookies) when "-o fsc" is
>    supplied to mount.
>
> ver #5:
>  - Fixed a couple of bits of cookie handling[2]:
>    - The cookie should be released in cifs_evict_inode(), not
>      cifsFileInfo_put_final().  The cookie needs to persist beyond file
>      closure so that writepages will be able to write to it.
>    - fscache_use_cookie() needs to be called in cifs_atomic_open() as it is
>      for cifs_open().
>
> ver #4:
>  - Fixed the use of sizeof with memset.
>  - tcon->vol_create_time is __le64 so doesn't need cpu_to_le64().
>
> ver #3:
>  - Canonicalise the cifs coherency data to make the cache portable.
>  - Set volume coherency data.
>
> ver #2:
>  - Use gfpflags_allow_blocking() rather than using flag directly.
>  - Upgraded to -rc4 to allow for upstream changes[1].
>  - fscache_acquire_volume() now returns errors.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Acked-by: Jeff Layton <jlayton@kernel.org>
> cc: Steve French <smfrench@gmail.com>
> cc: Shyam Prasad N <nspmangalore@gmail.com>
> cc: linux-cifs@vger.kernel.org
> cc: linux-cachefs@redhat.com
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23b55d673d7527b093cd97b7c217c82e70cd1af0 [1]
> Link: https://lore.kernel.org/r/3419813.1641592362@warthog.procyon.org.uk/ [2]
> Link: https://lore.kernel.org/r/163819671009.215744.11230627184193298714.stgit@warthog.procyon.org.uk/ # v1
> Link: https://lore.kernel.org/r/163906982979.143852.10672081929614953210.stgit@warthog.procyon.org.uk/ # v2
> Link: https://lore.kernel.org/r/163967187187.1823006.247415138444991444.stgit@warthog.procyon.org.uk/ # v3
> Link: https://lore.kernel.org/r/164021579335.640689.2681324337038770579.stgit@warthog.procyon.org.uk/ # v4
> Link: https://lore.kernel.org/r/3462849.1641593783@warthog.procyon.org.uk/ # v5
> Link: https://lore.kernel.org/r/1318953.1642024578@warthog.procyon.org.uk/ # v6
> ---
>
>  fs/cifs/Kconfig    |    2
>  fs/cifs/Makefile   |    2
>  fs/cifs/cache.c    |  105 ----------------
>  fs/cifs/cifsfs.c   |   19 ++-
>  fs/cifs/cifsglob.h |    5 -
>  fs/cifs/connect.c  |   15 --
>  fs/cifs/dir.c      |    5 +
>  fs/cifs/file.c     |   70 +++++++----
>  fs/cifs/fscache.c  |  333 ++++++++++++----------------------------------------
>  fs/cifs/fscache.h  |  126 ++++++--------------
>  fs/cifs/inode.c    |   19 ++-
>  11 files changed, 197 insertions(+), 504 deletions(-)
>  delete mode 100644 fs/cifs/cache.c
>
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 346ae8716deb..3b7e3b9e4fd2 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -188,7 +188,7 @@ config CIFS_SMB_DIRECT
>
>  config CIFS_FSCACHE
>         bool "Provide CIFS client caching support"
> -       depends on CIFS=m && FSCACHE_OLD_API || CIFS=y && FSCACHE_OLD_API=y
> +       depends on CIFS=m && FSCACHE || CIFS=y && FSCACHE=y
>         help
>           Makes CIFS FS-Cache capable. Say Y here if you want your CIFS data
>           to be cached locally on disk through the general filesystem cache
> diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile
> index 87fcacdf3de7..cc8fdcb35b71 100644
> --- a/fs/cifs/Makefile
> +++ b/fs/cifs/Makefile
> @@ -25,7 +25,7 @@ cifs-$(CONFIG_CIFS_DFS_UPCALL) += cifs_dfs_ref.o dfs_cache.o
>
>  cifs-$(CONFIG_CIFS_SWN_UPCALL) += netlink.o cifs_swn.o
>
> -cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o cache.o
> +cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o
>
>  cifs-$(CONFIG_CIFS_SMB_DIRECT) += smbdirect.o
>
> diff --git a/fs/cifs/cache.c b/fs/cifs/cache.c
> deleted file mode 100644
> index 8be57aaedab6..000000000000
> --- a/fs/cifs/cache.c
> +++ /dev/null
> @@ -1,105 +0,0 @@
> -// SPDX-License-Identifier: LGPL-2.1
> -/*
> - *   CIFS filesystem cache index structure definitions
> - *
> - *   Copyright (c) 2010 Novell, Inc.
> - *   Authors(s): Suresh Jayaraman (sjayaraman@suse.de>
> - *
> - */
> -#include "fscache.h"
> -#include "cifs_debug.h"
> -
> -/*
> - * CIFS filesystem definition for FS-Cache
> - */
> -struct fscache_netfs cifs_fscache_netfs = {
> -       .name = "cifs",
> -       .version = 0,
> -};
> -
> -/*
> - * Register CIFS for caching with FS-Cache
> - */
> -int cifs_fscache_register(void)
> -{
> -       return fscache_register_netfs(&cifs_fscache_netfs);
> -}
> -
> -/*
> - * Unregister CIFS for caching
> - */
> -void cifs_fscache_unregister(void)
> -{
> -       fscache_unregister_netfs(&cifs_fscache_netfs);
> -}
> -
> -/*
> - * Server object for FS-Cache
> - */
> -const struct fscache_cookie_def cifs_fscache_server_index_def = {
> -       .name = "CIFS.server",
> -       .type = FSCACHE_COOKIE_TYPE_INDEX,
> -};
> -
> -static enum
> -fscache_checkaux cifs_fscache_super_check_aux(void *cookie_netfs_data,
> -                                             const void *data,
> -                                             uint16_t datalen,
> -                                             loff_t object_size)
> -{
> -       struct cifs_fscache_super_auxdata auxdata;
> -       const struct cifs_tcon *tcon = cookie_netfs_data;
> -
> -       if (datalen != sizeof(auxdata))
> -               return FSCACHE_CHECKAUX_OBSOLETE;
> -
> -       memset(&auxdata, 0, sizeof(auxdata));
> -       auxdata.resource_id = tcon->resource_id;
> -       auxdata.vol_create_time = tcon->vol_create_time;
> -       auxdata.vol_serial_number = tcon->vol_serial_number;
> -
> -       if (memcmp(data, &auxdata, datalen) != 0)
> -               return FSCACHE_CHECKAUX_OBSOLETE;
> -
> -       return FSCACHE_CHECKAUX_OKAY;
> -}
> -
> -/*
> - * Superblock object for FS-Cache
> - */
> -const struct fscache_cookie_def cifs_fscache_super_index_def = {
> -       .name = "CIFS.super",
> -       .type = FSCACHE_COOKIE_TYPE_INDEX,
> -       .check_aux = cifs_fscache_super_check_aux,
> -};
> -
> -static enum
> -fscache_checkaux cifs_fscache_inode_check_aux(void *cookie_netfs_data,
> -                                             const void *data,
> -                                             uint16_t datalen,
> -                                             loff_t object_size)
> -{
> -       struct cifs_fscache_inode_auxdata auxdata;
> -       struct cifsInodeInfo *cifsi = cookie_netfs_data;
> -
> -       if (datalen != sizeof(auxdata))
> -               return FSCACHE_CHECKAUX_OBSOLETE;
> -
> -       memset(&auxdata, 0, sizeof(auxdata));
> -       auxdata.eof = cifsi->server_eof;
> -       auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
> -       auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
> -       auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
> -       auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
> -
> -       if (memcmp(data, &auxdata, datalen) != 0)
> -               return FSCACHE_CHECKAUX_OBSOLETE;
> -
> -       return FSCACHE_CHECKAUX_OKAY;
> -}
> -
> -const struct fscache_cookie_def cifs_fscache_inode_object_def = {
> -       .name           = "CIFS.uniqueid",
> -       .type           = FSCACHE_COOKIE_TYPE_DATAFILE,
> -       .check_aux      = cifs_fscache_inode_check_aux,
> -};
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index dca42aa87d30..26cf2193c9a2 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -396,6 +396,9 @@ static void
>  cifs_evict_inode(struct inode *inode)
>  {
>         truncate_inode_pages_final(&inode->i_data);
> +       if (inode->i_state & I_PINNING_FSCACHE_WB)
> +               cifs_fscache_unuse_inode_cookie(inode, true);
> +       cifs_fscache_release_inode_cookie(inode);
>         clear_inode(inode);
>  }
>
> @@ -720,6 +723,12 @@ static int cifs_show_stats(struct seq_file *s, struct dentry *root)
>  }
>  #endif
>
> +static int cifs_write_inode(struct inode *inode, struct writeback_control *wbc)
> +{
> +       fscache_unpin_writeback(wbc, cifs_inode_cookie(inode));
> +       return 0;
> +}
> +
>  static int cifs_drop_inode(struct inode *inode)
>  {
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> @@ -732,6 +741,7 @@ static int cifs_drop_inode(struct inode *inode)
>  static const struct super_operations cifs_super_ops = {
>         .statfs = cifs_statfs,
>         .alloc_inode = cifs_alloc_inode,
> +       .write_inode    = cifs_write_inode,
>         .free_inode = cifs_free_inode,
>         .drop_inode     = cifs_drop_inode,
>         .evict_inode    = cifs_evict_inode,
> @@ -1624,13 +1634,9 @@ init_cifs(void)
>                 goto out_destroy_cifsoplockd_wq;
>         }
>
> -       rc = cifs_fscache_register();
> -       if (rc)
> -               goto out_destroy_deferredclose_wq;
> -
>         rc = cifs_init_inodecache();
>         if (rc)
> -               goto out_unreg_fscache;
> +               goto out_destroy_deferredclose_wq;
>
>         rc = cifs_init_mids();
>         if (rc)
> @@ -1692,8 +1698,6 @@ init_cifs(void)
>         cifs_destroy_mids();
>  out_destroy_inodecache:
>         cifs_destroy_inodecache();
> -out_unreg_fscache:
> -       cifs_fscache_unregister();
>  out_destroy_deferredclose_wq:
>         destroy_workqueue(deferredclose_wq);
>  out_destroy_cifsoplockd_wq:
> @@ -1729,7 +1733,6 @@ exit_cifs(void)
>         cifs_destroy_request_bufs();
>         cifs_destroy_mids();
>         cifs_destroy_inodecache();
> -       cifs_fscache_unregister();
>         destroy_workqueue(deferredclose_wq);
>         destroy_workqueue(cifsoplockd_wq);
>         destroy_workqueue(decrypt_wq);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index be74606724c7..ba6fbb1ad8f3 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -659,9 +659,6 @@ struct TCP_Server_Info {
>         unsigned int total_read; /* total amount of data read in this pass */
>         atomic_t in_send; /* requests trying to send */
>         atomic_t num_waiters;   /* blocked waiting to get in sendrecv */
> -#ifdef CONFIG_CIFS_FSCACHE
> -       struct fscache_cookie   *fscache; /* client index cache cookie */
> -#endif
>  #ifdef CONFIG_CIFS_STATS2
>         atomic_t num_cmds[NUMBER_OF_SMB2_COMMANDS]; /* total requests by cmd */
>         atomic_t smb2slowcmd[NUMBER_OF_SMB2_COMMANDS]; /* count resps > 1 sec */
> @@ -1117,7 +1114,7 @@ struct cifs_tcon {
>         __u32 max_bytes_copy;
>  #ifdef CONFIG_CIFS_FSCACHE
>         u64 resource_id;                /* server resource id */
> -       struct fscache_cookie *fscache; /* cookie for share */
> +       struct fscache_volume *fscache; /* cookie for share */
>  #endif
>         struct list_head pending_opens; /* list of incomplete opens */
>         struct cached_fid crfid; /* Cached root fid */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1060164b984a..598be9890f2a 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1396,10 +1396,6 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
>
>         cifs_crypto_secmech_release(server);
>
> -       /* fscache server cookies are based on primary channel only */
> -       if (!CIFS_SERVER_IS_CHAN(server))
> -               cifs_fscache_release_client_cookie(server);
> -
>         kfree(server->session_key.response);
>         server->session_key.response = NULL;
>         server->session_key.len = 0;
> @@ -1559,14 +1555,6 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
>         list_add(&tcp_ses->tcp_ses_list, &cifs_tcp_ses_list);
>         spin_unlock(&cifs_tcp_ses_lock);
>
> -       /* fscache server cookies are based on primary channel only */
> -       if (!CIFS_SERVER_IS_CHAN(tcp_ses))
> -               cifs_fscache_get_client_cookie(tcp_ses);
> -#ifdef CONFIG_CIFS_FSCACHE
> -       else
> -               tcp_ses->fscache = tcp_ses->primary_server->fscache;
> -#endif /* CONFIG_CIFS_FSCACHE */
> -
>         /* queue echo request delayed work */
>         queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
>
> @@ -3069,7 +3057,8 @@ static int mount_get_conns(struct mount_ctx *mnt_ctx)
>          * Inside cifs_fscache_get_super_cookie it checks
>          * that we do not get super cookie twice.
>          */
> -       cifs_fscache_get_super_cookie(tcon);
> +       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_FSCACHE)
> +               cifs_fscache_get_super_cookie(tcon);
>
>  out:
>         mnt_ctx->server = server;
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 6e8e7cc26ae2..ce9b22aecfba 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -22,6 +22,7 @@
>  #include "cifs_unicode.h"
>  #include "fs_context.h"
>  #include "cifs_ioctl.h"
> +#include "fscache.h"
>
>  static void
>  renew_parental_timestamps(struct dentry *direntry)
> @@ -507,8 +508,12 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
>                         server->ops->close(xid, tcon, &fid);
>                 cifs_del_pending_open(&open);
>                 rc = -ENOMEM;
> +               goto out;
>         }
>
> +       fscache_use_cookie(cifs_inode_cookie(file_inode(file)),
> +                          file->f_mode & FMODE_WRITE);
> +
>  out:
>         cifs_put_tlink(tlink);
>  out_free_xid:
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 9fee3af83a73..fb77ca1a15d8 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -376,8 +376,6 @@ static void cifsFileInfo_put_final(struct cifsFileInfo *cifs_file)
>         struct cifsLockInfo *li, *tmp;
>         struct super_block *sb = inode->i_sb;
>
> -       cifs_fscache_release_inode_cookie(inode);
> -
>         /*
>          * Delete any outstanding lock records. We'll lose them when the file
>          * is closed anyway.
> @@ -570,7 +568,7 @@ int cifs_open(struct inode *inode, struct file *file)
>                         spin_lock(&CIFS_I(inode)->deferred_lock);
>                         cifs_del_deferred_close(cfile);
>                         spin_unlock(&CIFS_I(inode)->deferred_lock);
> -                       goto out;
> +                       goto use_cache;
>                 } else {
>                         _cifsFileInfo_put(cfile, true, false);
>                 }
> @@ -632,8 +630,6 @@ int cifs_open(struct inode *inode, struct file *file)
>                 goto out;
>         }
>
> -       cifs_fscache_set_inode_cookie(inode, file);
> -
>         if ((oplock & CIFS_CREATE_ACTION) && !posix_open_ok && tcon->unix_ext) {
>                 /*
>                  * Time to set mode which we can not set earlier due to
> @@ -652,6 +648,19 @@ int cifs_open(struct inode *inode, struct file *file)
>                                        cfile->pid);
>         }
>
> +use_cache:
> +       fscache_use_cookie(cifs_inode_cookie(file_inode(file)),
> +                          file->f_mode & FMODE_WRITE);
> +       if (file->f_flags & O_DIRECT &&
> +           (!((file->f_flags & O_ACCMODE) != O_RDONLY) ||
> +            file->f_flags & O_APPEND)) {
> +               struct cifs_fscache_inode_coherency_data cd;
> +               cifs_fscache_fill_coherency(file_inode(file), &cd);
> +               fscache_invalidate(cifs_inode_cookie(file_inode(file)),
> +                                  &cd, i_size_read(file_inode(file)),
> +                                  FSCACHE_INVAL_DIO_WRITE);
> +       }
> +
>  out:
>         free_dentry_path(page);
>         free_xid(xid);
> @@ -876,6 +885,8 @@ int cifs_close(struct inode *inode, struct file *file)
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct cifs_deferred_close *dclose;
>
> +       cifs_fscache_unuse_inode_cookie(inode, file->f_mode & FMODE_WRITE);
> +
>         if (file->private_data != NULL) {
>                 cfile = file->private_data;
>                 file->private_data = NULL;
> @@ -886,7 +897,6 @@ int cifs_close(struct inode *inode, struct file *file)
>                     dclose) {
>                         if (test_and_clear_bit(CIFS_INO_MODIFIED_ATTR, &cinode->flags)) {
>                                 inode->i_ctime = inode->i_mtime = current_time(inode);
> -                               cifs_fscache_update_inode_cookie(inode);
>                         }
>                         spin_lock(&cinode->deferred_lock);
>                         cifs_add_deferred_close(cfile, dclose);
> @@ -4198,10 +4208,12 @@ static vm_fault_t
>  cifs_page_mkwrite(struct vm_fault *vmf)
>  {
>         struct page *page = vmf->page;
> -       struct file *file = vmf->vma->vm_file;
> -       struct inode *inode = file_inode(file);
>
> -       cifs_fscache_wait_on_page_write(inode, page);
> +#ifdef CONFIG_CIFS_FSCACHE
> +       if (PageFsCache(page) &&
> +           wait_on_page_fscache_killable(page) < 0)
> +               return VM_FAULT_RETRY;
> +#endif
>
>         lock_page(page);
>         return VM_FAULT_LOCKED;
> @@ -4275,8 +4287,6 @@ cifs_readv_complete(struct work_struct *work)
>                 if (rdata->result == 0 ||
>                     (rdata->result == -EAGAIN && got_bytes))
>                         cifs_readpage_to_fscache(rdata->mapping->host, page);
> -               else
> -                       cifs_fscache_uncache_page(rdata->mapping->host, page);
>
>                 got_bytes -= min_t(unsigned int, PAGE_SIZE, got_bytes);
>
> @@ -4593,11 +4603,6 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
>                 kref_put(&rdata->refcount, cifs_readdata_release);
>         }
>
> -       /* Any pages that have been shown to fscache but didn't get added to
> -        * the pagecache must be uncached before they get returned to the
> -        * allocator.
> -        */
> -       cifs_fscache_readpages_cancel(mapping->host, page_list);
>         free_xid(xid);
>         return rc;
>  }
> @@ -4801,17 +4806,19 @@ static int cifs_release_page(struct page *page, gfp_t gfp)
>  {
>         if (PagePrivate(page))
>                 return 0;
> -
> -       return cifs_fscache_release_page(page, gfp);
> +       if (PageFsCache(page)) {
> +               if (current_is_kswapd() || !(gfp & __GFP_FS))
> +                       return false;
> +               wait_on_page_fscache(page);
> +       }
> +       fscache_note_page_release(cifs_inode_cookie(page->mapping->host));
> +       return true;
>  }
>
>  static void cifs_invalidate_page(struct page *page, unsigned int offset,
>                                  unsigned int length)
>  {
> -       struct cifsInodeInfo *cifsi = CIFS_I(page->mapping->host);
> -
> -       if (offset == 0 && length == PAGE_SIZE)
> -               cifs_fscache_invalidate_page(page, &cifsi->vfs_inode);
> +       wait_on_page_fscache(page);
>  }
>
>  static int cifs_launder_page(struct page *page)
> @@ -4831,7 +4838,7 @@ static int cifs_launder_page(struct page *page)
>         if (clear_page_dirty_for_io(page))
>                 rc = cifs_writepage_locked(page, &wbc);
>
> -       cifs_fscache_invalidate_page(page, page->mapping->host);
> +       wait_on_page_fscache(page);
>         return rc;
>  }
>
> @@ -4988,6 +4995,19 @@ static void cifs_swap_deactivate(struct file *file)
>         /* do we need to unpin (or unlock) the file */
>  }
>
> +/*
> + * Mark a page as having been made dirty and thus needing writeback.  We also
> + * need to pin the cache object to write back to.
> + */
> +#ifdef CONFIG_CIFS_FSCACHE
> +static int cifs_set_page_dirty(struct page *page)
> +{
> +       return fscache_set_page_dirty(page, cifs_inode_cookie(page->mapping->host));
> +}
> +#else
> +#define cifs_set_page_dirty __set_page_dirty_nobuffers
> +#endif
> +
>  const struct address_space_operations cifs_addr_ops = {
>         .readpage = cifs_readpage,
>         .readpages = cifs_readpages,
> @@ -4995,7 +5015,7 @@ const struct address_space_operations cifs_addr_ops = {
>         .writepages = cifs_writepages,
>         .write_begin = cifs_write_begin,
>         .write_end = cifs_write_end,
> -       .set_page_dirty = __set_page_dirty_nobuffers,
> +       .set_page_dirty = cifs_set_page_dirty,
>         .releasepage = cifs_release_page,
>         .direct_IO = cifs_direct_io,
>         .invalidatepage = cifs_invalidate_page,
> @@ -5020,7 +5040,7 @@ const struct address_space_operations cifs_addr_ops_smallbuf = {
>         .writepages = cifs_writepages,
>         .write_begin = cifs_write_begin,
>         .write_end = cifs_write_end,
> -       .set_page_dirty = __set_page_dirty_nobuffers,
> +       .set_page_dirty = cifs_set_page_dirty,
>         .releasepage = cifs_release_page,
>         .invalidatepage = cifs_invalidate_page,
>         .launder_page = cifs_launder_page,
> diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
> index 003c5f1f4dfb..efaac4d5ff55 100644
> --- a/fs/cifs/fscache.c
> +++ b/fs/cifs/fscache.c
> @@ -12,250 +12,136 @@
>  #include "cifs_fs_sb.h"
>  #include "cifsproto.h"
>
> -/*
> - * Key layout of CIFS server cache index object
> - */
> -struct cifs_server_key {
> -       __u64 conn_id;
> -} __packed;
> -
> -/*
> - * Get a cookie for a server object keyed by {IPaddress,port,family} tuple
> - */
> -void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
> -{
> -       struct cifs_server_key key;
> -
> -       /*
> -        * Check if cookie was already initialized so don't reinitialize it.
> -        * In the future, as we integrate with newer fscache features,
> -        * we may want to instead add a check if cookie has changed
> -        */
> -       if (server->fscache)
> -               return;
> -
> -       memset(&key, 0, sizeof(key));
> -       key.conn_id = server->conn_id;
> -
> -       server->fscache =
> -               fscache_acquire_cookie(cifs_fscache_netfs.primary_index,
> -                                      &cifs_fscache_server_index_def,
> -                                      &key, sizeof(key),
> -                                      NULL, 0,
> -                                      server, 0, true);
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
> -                __func__, server, server->fscache);
> -}
> -
> -void cifs_fscache_release_client_cookie(struct TCP_Server_Info *server)
> +static void cifs_fscache_fill_volume_coherency(
> +       struct cifs_tcon *tcon,
> +       struct cifs_fscache_volume_coherency_data *cd)
>  {
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
> -                __func__, server, server->fscache);
> -       fscache_relinquish_cookie(server->fscache, NULL, false);
> -       server->fscache = NULL;
> +       memset(cd, 0, sizeof(*cd));
> +       cd->resource_id         = cpu_to_le64(tcon->resource_id);
> +       cd->vol_create_time     = tcon->vol_create_time;
> +       cd->vol_serial_number   = cpu_to_le32(tcon->vol_serial_number);
>  }
>
> -void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
> +int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
>  {
> +       struct cifs_fscache_volume_coherency_data cd;
>         struct TCP_Server_Info *server = tcon->ses->server;
> +       struct fscache_volume *vcookie;
> +       const struct sockaddr *sa = (struct sockaddr *)&server->dstaddr;
> +       size_t slen, i;
>         char *sharename;
> -       struct cifs_fscache_super_auxdata auxdata;
> +       char *key;
> +       int ret = -ENOMEM;
>
> -       /*
> -        * Check if cookie was already initialized so don't reinitialize it.
> -        * In the future, as we integrate with newer fscache features,
> -        * we may want to instead add a check if cookie has changed
> -        */
> -       if (tcon->fscache)
> -               return;
> +       tcon->fscache = NULL;
> +       switch (sa->sa_family) {
> +       case AF_INET:
> +       case AF_INET6:
> +               break;
> +       default:
> +               cifs_dbg(VFS, "Unknown network family '%d'\n", sa->sa_family);
> +               return -EINVAL;
> +       }
> +
> +       memset(&key, 0, sizeof(key));
>
>         sharename = extract_sharename(tcon->treeName);
>         if (IS_ERR(sharename)) {
>                 cifs_dbg(FYI, "%s: couldn't extract sharename\n", __func__);
> -               tcon->fscache = NULL;
> -               return;
> +               return -EINVAL;
>         }
>
> -       memset(&auxdata, 0, sizeof(auxdata));
> -       auxdata.resource_id = tcon->resource_id;
> -       auxdata.vol_create_time = tcon->vol_create_time;
> -       auxdata.vol_serial_number = tcon->vol_serial_number;
> +       slen = strlen(sharename);
> +       for (i = 0; i < slen; i++)
> +               if (sharename[i] == '/')
> +                       sharename[i] = ';';
> +
> +       key = kasprintf(GFP_KERNEL, "cifs,%pISpc,%s", sa, sharename);
> +       if (!key)
> +               goto out;
> +
> +       cifs_fscache_fill_volume_coherency(tcon, &cd);
> +       vcookie = fscache_acquire_volume(key,
> +                                        NULL, /* preferred_cache */
> +                                        &cd, sizeof(cd));
> +       cifs_dbg(FYI, "%s: (%s/0x%p)\n", __func__, key, vcookie);
> +       if (IS_ERR(vcookie)) {
> +               if (vcookie != ERR_PTR(-EBUSY)) {
> +                       ret = PTR_ERR(vcookie);
> +                       goto out_2;
> +               }
> +               pr_err("Cache volume key already in use (%s)\n", key);
> +               vcookie = NULL;
> +       }
>
> -       tcon->fscache =
> -               fscache_acquire_cookie(server->fscache,
> -                                      &cifs_fscache_super_index_def,
> -                                      sharename, strlen(sharename),
> -                                      &auxdata, sizeof(auxdata),
> -                                      tcon, 0, true);
> +       tcon->fscache = vcookie;
> +       ret = 0;
> +out_2:
> +       kfree(key);
> +out:
>         kfree(sharename);
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
> -                __func__, server->fscache, tcon->fscache);
> +       return ret;
>  }
>
>  void cifs_fscache_release_super_cookie(struct cifs_tcon *tcon)
>  {
> -       struct cifs_fscache_super_auxdata auxdata;
> -
> -       memset(&auxdata, 0, sizeof(auxdata));
> -       auxdata.resource_id = tcon->resource_id;
> -       auxdata.vol_create_time = tcon->vol_create_time;
> -       auxdata.vol_serial_number = tcon->vol_serial_number;
> +       struct cifs_fscache_volume_coherency_data cd;
>
>         cifs_dbg(FYI, "%s: (0x%p)\n", __func__, tcon->fscache);
> -       fscache_relinquish_cookie(tcon->fscache, &auxdata, false);
> -       tcon->fscache = NULL;
> -}
> -
> -static void cifs_fscache_acquire_inode_cookie(struct cifsInodeInfo *cifsi,
> -                                             struct cifs_tcon *tcon)
> -{
> -       struct cifs_fscache_inode_auxdata auxdata;
>
> -       memset(&auxdata, 0, sizeof(auxdata));
> -       auxdata.eof = cifsi->server_eof;
> -       auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
> -       auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
> -       auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
> -       auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
> -
> -       cifsi->fscache =
> -               fscache_acquire_cookie(tcon->fscache,
> -                                      &cifs_fscache_inode_object_def,
> -                                      &cifsi->uniqueid, sizeof(cifsi->uniqueid),
> -                                      &auxdata, sizeof(auxdata),
> -                                      cifsi, cifsi->vfs_inode.i_size, true);
> +       cifs_fscache_fill_volume_coherency(tcon, &cd);
> +       fscache_relinquish_volume(tcon->fscache, &cd, false);
> +       tcon->fscache = NULL;
>  }
>
> -static void cifs_fscache_enable_inode_cookie(struct inode *inode)
> +void cifs_fscache_get_inode_cookie(struct inode *inode)
>  {
> +       struct cifs_fscache_inode_coherency_data cd;
>         struct cifsInodeInfo *cifsi = CIFS_I(inode);
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
>
> -       if (cifsi->fscache)
> -               return;
> -
> -       if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_FSCACHE))
> -               return;
> -
> -       cifs_fscache_acquire_inode_cookie(cifsi, tcon);
> +       cifs_fscache_fill_coherency(&cifsi->vfs_inode, &cd);
>
> -       cifs_dbg(FYI, "%s: got FH cookie (0x%p/0x%p)\n",
> -                __func__, tcon->fscache, cifsi->fscache);
> +       cifsi->fscache =
> +               fscache_acquire_cookie(tcon->fscache, 0,
> +                                      &cifsi->uniqueid, sizeof(cifsi->uniqueid),
> +                                      &cd, sizeof(cd),
> +                                      i_size_read(&cifsi->vfs_inode));
>  }
>
> -void cifs_fscache_release_inode_cookie(struct inode *inode)
> +void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update)
>  {
> -       struct cifs_fscache_inode_auxdata auxdata;
> -       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -
> -       if (cifsi->fscache) {
> -               memset(&auxdata, 0, sizeof(auxdata));
> -               auxdata.eof = cifsi->server_eof;
> -               auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
> -               auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
> -               auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
> -               auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
> +       if (update) {
> +               struct cifs_fscache_inode_coherency_data cd;
> +               loff_t i_size = i_size_read(inode);
>
> -               cifs_dbg(FYI, "%s: (0x%p)\n", __func__, cifsi->fscache);
> -               /* fscache_relinquish_cookie does not seem to update auxdata */
> -               fscache_update_cookie(cifsi->fscache, &auxdata);
> -               fscache_relinquish_cookie(cifsi->fscache, &auxdata, false);
> -               cifsi->fscache = NULL;
> +               cifs_fscache_fill_coherency(inode, &cd);
> +               fscache_unuse_cookie(cifs_inode_cookie(inode), &cd, &i_size);
> +       } else {
> +               fscache_unuse_cookie(cifs_inode_cookie(inode), NULL, NULL);
>         }
>  }
>
> -void cifs_fscache_update_inode_cookie(struct inode *inode)
> +void cifs_fscache_release_inode_cookie(struct inode *inode)
>  {
> -       struct cifs_fscache_inode_auxdata auxdata;
>         struct cifsInodeInfo *cifsi = CIFS_I(inode);
>
>         if (cifsi->fscache) {
> -               memset(&auxdata, 0, sizeof(auxdata));
> -               auxdata.eof = cifsi->server_eof;
> -               auxdata.last_write_time_sec = cifsi->vfs_inode.i_mtime.tv_sec;
> -               auxdata.last_change_time_sec = cifsi->vfs_inode.i_ctime.tv_sec;
> -               auxdata.last_write_time_nsec = cifsi->vfs_inode.i_mtime.tv_nsec;
> -               auxdata.last_change_time_nsec = cifsi->vfs_inode.i_ctime.tv_nsec;
> -
>                 cifs_dbg(FYI, "%s: (0x%p)\n", __func__, cifsi->fscache);
> -               fscache_update_cookie(cifsi->fscache, &auxdata);
> -       }
> -}
> -
> -void cifs_fscache_set_inode_cookie(struct inode *inode, struct file *filp)
> -{
> -       cifs_fscache_enable_inode_cookie(inode);
> -}
> -
> -void cifs_fscache_reset_inode_cookie(struct inode *inode)
> -{
> -       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -       struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> -       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> -       struct fscache_cookie *old = cifsi->fscache;
> -
> -       if (cifsi->fscache) {
> -               /* retire the current fscache cache and get a new one */
> -               fscache_relinquish_cookie(cifsi->fscache, NULL, true);
> -
> -               cifs_fscache_acquire_inode_cookie(cifsi, tcon);
> -               cifs_dbg(FYI, "%s: new cookie 0x%p oldcookie 0x%p\n",
> -                        __func__, cifsi->fscache, old);
> +               fscache_relinquish_cookie(cifsi->fscache, false);
> +               cifsi->fscache = NULL;
>         }
>  }
>
> -int cifs_fscache_release_page(struct page *page, gfp_t gfp)
> -{
> -       if (PageFsCache(page)) {
> -               struct inode *inode = page->mapping->host;
> -               struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -
> -               cifs_dbg(FYI, "%s: (0x%p/0x%p)\n",
> -                        __func__, page, cifsi->fscache);
> -               if (!fscache_maybe_release_page(cifsi->fscache, page, gfp))
> -                       return 0;
> -       }
> -
> -       return 1;
> -}
> -
> -static void cifs_readpage_from_fscache_complete(struct page *page, void *ctx,
> -                                               int error)
> -{
> -       cifs_dbg(FYI, "%s: (0x%p/%d)\n", __func__, page, error);
> -       if (!error)
> -               SetPageUptodate(page);
> -       unlock_page(page);
> -}
> -
>  /*
>   * Retrieve a page from FS-Cache
>   */
>  int __cifs_readpage_from_fscache(struct inode *inode, struct page *page)
>  {
> -       int ret;
> -
>         cifs_dbg(FYI, "%s: (fsc:%p, p:%p, i:0x%p\n",
>                  __func__, CIFS_I(inode)->fscache, page, inode);
> -       ret = fscache_read_or_alloc_page(CIFS_I(inode)->fscache, page,
> -                                        cifs_readpage_from_fscache_complete,
> -                                        NULL,
> -                                        GFP_KERNEL);
> -       switch (ret) {
> -
> -       case 0: /* page found in fscache, read submitted */
> -               cifs_dbg(FYI, "%s: submitted\n", __func__);
> -               return ret;
> -       case -ENOBUFS:  /* page won't be cached */
> -       case -ENODATA:  /* page not in cache */
> -               cifs_dbg(FYI, "%s: %d\n", __func__, ret);
> -               return 1;
> -
> -       default:
> -               cifs_dbg(VFS, "unknown error ret = %d\n", ret);
> -       }
> -       return ret;
> +       return -ENOBUFS; // Needs conversion to using netfslib
>  }
>
>  /*
> @@ -266,78 +152,19 @@ int __cifs_readpages_from_fscache(struct inode *inode,
>                                 struct list_head *pages,
>                                 unsigned *nr_pages)
>  {
> -       int ret;
> -
>         cifs_dbg(FYI, "%s: (0x%p/%u/0x%p)\n",
>                  __func__, CIFS_I(inode)->fscache, *nr_pages, inode);
> -       ret = fscache_read_or_alloc_pages(CIFS_I(inode)->fscache, mapping,
> -                                         pages, nr_pages,
> -                                         cifs_readpage_from_fscache_complete,
> -                                         NULL,
> -                                         mapping_gfp_mask(mapping));
> -       switch (ret) {
> -       case 0: /* read submitted to the cache for all pages */
> -               cifs_dbg(FYI, "%s: submitted\n", __func__);
> -               return ret;
> -
> -       case -ENOBUFS:  /* some pages are not cached and can't be */
> -       case -ENODATA:  /* some pages are not cached */
> -               cifs_dbg(FYI, "%s: no page\n", __func__);
> -               return 1;
> -
> -       default:
> -               cifs_dbg(FYI, "unknown error ret = %d\n", ret);
> -       }
> -
> -       return ret;
> +       return -ENOBUFS; // Needs conversion to using netfslib
>  }
>
>  void __cifs_readpage_to_fscache(struct inode *inode, struct page *page)
>  {
>         struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -       int ret;
>
>         WARN_ON(!cifsi->fscache);
>
>         cifs_dbg(FYI, "%s: (fsc: %p, p: %p, i: %p)\n",
>                  __func__, cifsi->fscache, page, inode);
> -       ret = fscache_write_page(cifsi->fscache, page,
> -                                cifsi->vfs_inode.i_size, GFP_KERNEL);
> -       if (ret != 0)
> -               fscache_uncache_page(cifsi->fscache, page);
> -}
> -
> -void __cifs_fscache_readpages_cancel(struct inode *inode, struct list_head *pages)
> -{
> -       cifs_dbg(FYI, "%s: (fsc: %p, i: %p)\n",
> -                __func__, CIFS_I(inode)->fscache, inode);
> -       fscache_readpages_cancel(CIFS_I(inode)->fscache, pages);
> -}
> -
> -void __cifs_fscache_invalidate_page(struct page *page, struct inode *inode)
> -{
> -       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -       struct fscache_cookie *cookie = cifsi->fscache;
> -
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
> -       fscache_wait_on_page_write(cookie, page);
> -       fscache_uncache_page(cookie, page);
> -}
> -
> -void __cifs_fscache_wait_on_page_write(struct inode *inode, struct page *page)
> -{
> -       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -       struct fscache_cookie *cookie = cifsi->fscache;
> -
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
> -       fscache_wait_on_page_write(cookie, page);
> -}
> -
> -void __cifs_fscache_uncache_page(struct inode *inode, struct page *page)
> -{
> -       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> -       struct fscache_cookie *cookie = cifsi->fscache;
>
> -       cifs_dbg(FYI, "%s: (0x%p/0x%p)\n", __func__, page, cookie);
> -       fscache_uncache_page(cookie, page);
> +       // Needs conversion to using netfslib
>  }
> diff --git a/fs/cifs/fscache.h b/fs/cifs/fscache.h
> index 9baa1d0f22bd..0fc3f9252c84 100644
> --- a/fs/cifs/fscache.h
> +++ b/fs/cifs/fscache.h
> @@ -13,84 +13,62 @@
>
>  #include "cifsglob.h"
>
> -#ifdef CONFIG_CIFS_FSCACHE
> -
>  /*
> - * Auxiliary data attached to CIFS superblock within the cache
> + * Coherency data attached to CIFS volume within the cache
>   */
> -struct cifs_fscache_super_auxdata {
> -       u64     resource_id;            /* unique server resource id */
> +struct cifs_fscache_volume_coherency_data {
> +       __le64  resource_id;            /* unique server resource id */
>         __le64  vol_create_time;
> -       u32     vol_serial_number;
> +       __le32  vol_serial_number;
>  } __packed;
>
>  /*
> - * Auxiliary data attached to CIFS inode within the cache
> + * Coherency data attached to CIFS inode within the cache.
>   */
> -struct cifs_fscache_inode_auxdata {
> -       u64 last_write_time_sec;
> -       u64 last_change_time_sec;
> -       u32 last_write_time_nsec;
> -       u32 last_change_time_nsec;
> -       u64 eof;
> +struct cifs_fscache_inode_coherency_data {
> +       __le64 last_write_time_sec;
> +       __le64 last_change_time_sec;
> +       __le32 last_write_time_nsec;
> +       __le32 last_change_time_nsec;
>  };
>
> -/*
> - * cache.c
> - */
> -extern struct fscache_netfs cifs_fscache_netfs;
> -extern const struct fscache_cookie_def cifs_fscache_server_index_def;
> -extern const struct fscache_cookie_def cifs_fscache_super_index_def;
> -extern const struct fscache_cookie_def cifs_fscache_inode_object_def;
> -
> -extern int cifs_fscache_register(void);
> -extern void cifs_fscache_unregister(void);
> +#ifdef CONFIG_CIFS_FSCACHE
>
>  /*
>   * fscache.c
>   */
> -extern void cifs_fscache_get_client_cookie(struct TCP_Server_Info *);
> -extern void cifs_fscache_release_client_cookie(struct TCP_Server_Info *);
> -extern void cifs_fscache_get_super_cookie(struct cifs_tcon *);
> +extern int cifs_fscache_get_super_cookie(struct cifs_tcon *);
>  extern void cifs_fscache_release_super_cookie(struct cifs_tcon *);
>
> +extern void cifs_fscache_get_inode_cookie(struct inode *);
>  extern void cifs_fscache_release_inode_cookie(struct inode *);
> -extern void cifs_fscache_update_inode_cookie(struct inode *inode);
> -extern void cifs_fscache_set_inode_cookie(struct inode *, struct file *);
> -extern void cifs_fscache_reset_inode_cookie(struct inode *);
> +extern void cifs_fscache_unuse_inode_cookie(struct inode *, bool);
> +
> +static inline
> +void cifs_fscache_fill_coherency(struct inode *inode,
> +                                struct cifs_fscache_inode_coherency_data *cd)
> +{
> +       struct cifsInodeInfo *cifsi = CIFS_I(inode);
> +
> +       memset(cd, 0, sizeof(*cd));
> +       cd->last_write_time_sec   = cpu_to_le64(cifsi->vfs_inode.i_mtime.tv_sec);
> +       cd->last_write_time_nsec  = cpu_to_le32(cifsi->vfs_inode.i_mtime.tv_nsec);
> +       cd->last_change_time_sec  = cpu_to_le64(cifsi->vfs_inode.i_ctime.tv_sec);
> +       cd->last_change_time_nsec = cpu_to_le32(cifsi->vfs_inode.i_ctime.tv_nsec);
> +}
> +
>
> -extern void __cifs_fscache_invalidate_page(struct page *, struct inode *);
> -extern void __cifs_fscache_wait_on_page_write(struct inode *inode, struct page *page);
> -extern void __cifs_fscache_uncache_page(struct inode *inode, struct page *page);
>  extern int cifs_fscache_release_page(struct page *page, gfp_t gfp);
>  extern int __cifs_readpage_from_fscache(struct inode *, struct page *);
>  extern int __cifs_readpages_from_fscache(struct inode *,
>                                          struct address_space *,
>                                          struct list_head *,
>                                          unsigned *);
> -extern void __cifs_fscache_readpages_cancel(struct inode *, struct list_head *);
> -
>  extern void __cifs_readpage_to_fscache(struct inode *, struct page *);
>
> -static inline void cifs_fscache_invalidate_page(struct page *page,
> -                                              struct inode *inode)
> +static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode)
>  {
> -       if (PageFsCache(page))
> -               __cifs_fscache_invalidate_page(page, inode);
> -}
> -
> -static inline void cifs_fscache_wait_on_page_write(struct inode *inode,
> -                                                  struct page *page)
> -{
> -       if (PageFsCache(page))
> -               __cifs_fscache_wait_on_page_write(inode, page);
> -}
> -
> -static inline void cifs_fscache_uncache_page(struct inode *inode,
> -                                                  struct page *page)
> -{
> -       if (PageFsCache(page))
> -               __cifs_fscache_uncache_page(inode, page);
> +       return CIFS_I(inode)->fscache;
>  }
>
>  static inline int cifs_readpage_from_fscache(struct inode *inode,
> @@ -120,41 +98,20 @@ static inline void cifs_readpage_to_fscache(struct inode *inode,
>                 __cifs_readpage_to_fscache(inode, page);
>  }
>
> -static inline void cifs_fscache_readpages_cancel(struct inode *inode,
> -                                                struct list_head *pages)
> +#else /* CONFIG_CIFS_FSCACHE */
> +static inline
> +void cifs_fscache_fill_coherency(struct inode *inode,
> +                                struct cifs_fscache_inode_coherency_data *cd)
>  {
> -       if (CIFS_I(inode)->fscache)
> -               return __cifs_fscache_readpages_cancel(inode, pages);
>  }
>
> -#else /* CONFIG_CIFS_FSCACHE */
> -static inline int cifs_fscache_register(void) { return 0; }
> -static inline void cifs_fscache_unregister(void) {}
> -
> -static inline void
> -cifs_fscache_get_client_cookie(struct TCP_Server_Info *server) {}
> -static inline void
> -cifs_fscache_release_client_cookie(struct TCP_Server_Info *server) {}
> -static inline void cifs_fscache_get_super_cookie(struct cifs_tcon *tcon) {}
> -static inline void
> -cifs_fscache_release_super_cookie(struct cifs_tcon *tcon) {}
> +static inline int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon) { return 0; }
> +static inline void cifs_fscache_release_super_cookie(struct cifs_tcon *tcon) {}
>
> +static inline void cifs_fscache_get_inode_cookie(struct inode *inode) {}
>  static inline void cifs_fscache_release_inode_cookie(struct inode *inode) {}
> -static inline void cifs_fscache_update_inode_cookie(struct inode *inode) {}
> -static inline void cifs_fscache_set_inode_cookie(struct inode *inode,
> -                                                struct file *filp) {}
> -static inline void cifs_fscache_reset_inode_cookie(struct inode *inode) {}
> -static inline int cifs_fscache_release_page(struct page *page, gfp_t gfp)
> -{
> -       return 1; /* May release page */
> -}
> -
> -static inline void cifs_fscache_invalidate_page(struct page *page,
> -                       struct inode *inode) {}
> -static inline void cifs_fscache_wait_on_page_write(struct inode *inode,
> -                                                  struct page *page) {}
> -static inline void cifs_fscache_uncache_page(struct inode *inode,
> -                                                  struct page *page) {}
> +static inline void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update) {}
> +static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode) { return NULL; }
>
>  static inline int
>  cifs_readpage_from_fscache(struct inode *inode, struct page *page)
> @@ -173,11 +130,6 @@ static inline int cifs_readpages_from_fscache(struct inode *inode,
>  static inline void cifs_readpage_to_fscache(struct inode *inode,
>                         struct page *page) {}
>
> -static inline void cifs_fscache_readpages_cancel(struct inode *inode,
> -                                                struct list_head *pages)
> -{
> -}
> -
>  #endif /* CONFIG_CIFS_FSCACHE */
>
>  #endif /* _CIFS_FSCACHE_H */
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 279622e4eb1c..9b93e7d3e0e1 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1298,10 +1298,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
>                         inode->i_flags |= S_NOATIME | S_NOCMTIME;
>                 if (inode->i_state & I_NEW) {
>                         inode->i_ino = hash;
> -#ifdef CONFIG_CIFS_FSCACHE
> -                       /* initialize per-inode cache cookie pointer */
> -                       CIFS_I(inode)->fscache = NULL;
> -#endif
> +                       cifs_fscache_get_inode_cookie(inode);
>                         unlock_new_inode(inode);
>                 }
>         }
> @@ -1370,6 +1367,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
>                 iget_failed(inode);
>                 inode = ERR_PTR(rc);
>         }
> +
>  out:
>         kfree(path);
>         free_xid(xid);
> @@ -2257,6 +2255,8 @@ cifs_dentry_needs_reval(struct dentry *dentry)
>  int
>  cifs_invalidate_mapping(struct inode *inode)
>  {
> +       struct cifs_fscache_inode_coherency_data cd;
> +       struct cifsInodeInfo *cifsi = CIFS_I(inode);
>         int rc = 0;
>
>         if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
> @@ -2266,7 +2266,8 @@ cifs_invalidate_mapping(struct inode *inode)
>                                  __func__, inode);
>         }
>
> -       cifs_fscache_reset_inode_cookie(inode);
> +       cifs_fscache_fill_coherency(&cifsi->vfs_inode, &cd);
> +       fscache_invalidate(cifs_inode_cookie(inode), &cd, i_size_read(inode), 0);
>         return rc;
>  }
>
> @@ -2771,8 +2772,10 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
>                 goto out;
>
>         if ((attrs->ia_valid & ATTR_SIZE) &&
> -           attrs->ia_size != i_size_read(inode))
> +           attrs->ia_size != i_size_read(inode)) {
>                 truncate_setsize(inode, attrs->ia_size);
> +               fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
> +       }
>
>         setattr_copy(&init_user_ns, inode, attrs);
>         mark_inode_dirty(inode);
> @@ -2967,8 +2970,10 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
>                 goto cifs_setattr_exit;
>
>         if ((attrs->ia_valid & ATTR_SIZE) &&
> -           attrs->ia_size != i_size_read(inode))
> +           attrs->ia_size != i_size_read(inode)) {
>                 truncate_setsize(inode, attrs->ia_size);
> +               fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
> +       }
>
>         setattr_copy(&init_user_ns, inode, attrs);
>         mark_inode_dirty(inode);
>
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/11] cifs: Support fscache indexing rewrite
  2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
  2022-01-19  5:31   ` Steve French
@ 2022-01-19  8:32   ` David Howells
  2022-01-19  9:48     ` Shyam Prasad N
  2022-01-19 10:38     ` David Howells
  1 sibling, 2 replies; 27+ messages in thread
From: David Howells @ 2022-01-19  8:32 UTC (permalink / raw)
  To: Steve French; +Cc: dhowells, CIFS, Jeff Layton, Shyam Prasad N

Steve French <smfrench@gmail.com> wrote:

> WARNING: Missing a blank line after declarations
> #460: FILE: fs/cifs/file.c:658:
> + struct cifs_fscache_inode_coherency_data cd;
> + cifs_fscache_fill_coherency(file_inode(file), &cd);

I have a small patch to abstract cache invalidation for cifs into a helper
function (see attached) that I'll merge in that will also take care of this.

David
---
commit ff463eee039fbe119ae0d4185cb8a90aec10ec80
Author: David Howells <dhowells@redhat.com>
Date:   Fri Jan 7 18:08:37 2022 +0000

    cifs: Abstract cache invalidation into a helper function
    
    Abstract fscache invalidation for a cifs inode out into a helper function
    as there will be more than one caller of it.
    
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 27604eb01a94..015fd415e5ee 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -653,13 +653,9 @@ int cifs_open(struct inode *inode, struct file *file)
 			   file->f_mode & FMODE_WRITE);
 	if (file->f_flags & O_DIRECT &&
 	    (!((file->f_flags & O_ACCMODE) != O_RDONLY) ||
-	     file->f_flags & O_APPEND)) {
-		struct cifs_fscache_inode_coherency_data cd;
-		cifs_fscache_fill_coherency(file_inode(file), &cd);
-		fscache_invalidate(cifs_inode_cookie(file_inode(file)),
-				   &cd, i_size_read(file_inode(file)),
-				   FSCACHE_INVAL_DIO_WRITE);
-	}
+	     file->f_flags & O_APPEND))
+		cifs_invalidate_cache(file_inode(file),
+				      FSCACHE_INVAL_DIO_WRITE);
 
 out:
 	free_dentry_path(page);
diff --git a/fs/cifs/fscache.h b/fs/cifs/fscache.h
index e444445d0906..b741d38df6c8 100644
--- a/fs/cifs/fscache.h
+++ b/fs/cifs/fscache.h
@@ -71,6 +71,15 @@ static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode)
 	return netfs_i_cookie(inode);
 }
 
+static inline void cifs_invalidate_cache(struct inode *inode, unsigned int flags)
+{
+	struct cifs_fscache_inode_coherency_data cd;
+
+	cifs_fscache_fill_coherency(inode, &cd);
+	fscache_invalidate(cifs_inode_cookie(inode), &cd,
+			   i_size_read(inode), flags);
+}
+
 static inline int cifs_readpage_from_fscache(struct inode *inode,
 					     struct page *page)
 {
@@ -112,6 +121,7 @@ static inline void cifs_fscache_get_inode_cookie(struct inode *inode) {}
 static inline void cifs_fscache_release_inode_cookie(struct inode *inode) {}
 static inline void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update) {}
 static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode) { return NULL; }
+static inline void cifs_invalidate_cache(struct inode *inode, unsigned int flags) {}
 
 static inline int
 cifs_readpage_from_fscache(struct inode *inode, struct page *page)


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-18 17:40   ` David Howells
  2022-01-19  5:20     ` Christoph Hellwig
@ 2022-01-19  9:18     ` David Howells
  2022-01-19 11:15       ` Christian Brauner
                         ` (2 more replies)
  1 sibling, 3 replies; 27+ messages in thread
From: David Howells @ 2022-01-19  9:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Jan 18, 2022 at 05:40:14PM +0000, David Howells wrote:
> > Christoph Hellwig <hch@infradead.org> wrote:
> > 
> > > On Tue, Jan 18, 2022 at 01:54:54PM +0000, David Howells wrote:
> > > > Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
> > > > common practice for the other inode flags[1].
> > > 
> > > Please fix the flag to have a sensible name first, as the naming of the
> > > flag and this new helper is utterly wrong as we already discussed.
> > 
> > And I suggested a new name, which you didn't comment on.
> 
> Again, look at the semantics of the flag:  The only thing it does in the
> VFS is to prevent a rmdir.  So you might want to name it after that.
> 
> Or in fact drop the flag entirely.  We don't have that kind of
> protection for other in-kernel file use or important userspace daemons
> either.  I can't see why cachefiles is the magic snowflake here that
> suddenly needs semantics no one else has.

The flag cannot just be dropped - it's an important part of the interaction
with cachefilesd with regard to culling.  Culling to free up space is
offloaded to userspace rather than being done within the kernel.

Previously, cachefiles, the kernel module, had to maintain a huge tree of
records of every backing inode that it was currently using so that it could
forbid cachefilesd to cull one when cachefilesd asked.  I've reduced that to a
single bit flag on the inode struct, thereby saving both memory and time.  You
can argue whether it's worth sacrificing an inode flag bit for that, but the
flag can be reused for any other kernel service that wants to similarly mark
an inode in use.

Further, it's used as a mark to prevent cachefiles accidentally using an inode
twice - say someone misconfigures a second cache overlapping the first - and,
again, this works if some other kernel driver wants to mark inode it is using
in use.  Cachefiles will refuse to use them if it ever sees them, so no
problem there.

And it's not true that we don't have that kind of protection for other
in-kernel file use.  See S_SWAPFILE.  I did consider using that, but that has
other side effects.  I mentioned that perhaps I should make swapon set
S_KERNEL_FILE also.  Also blockdevs have some exclusion also, I think.

The rmdir thing should really apply to rename and unlink also.  That's to
prevent someone, cachefilesd included, causing cachefiles to malfunction by
removing the directories it created.  Possibly this should be a separate bit
to S_KERNEL_FILE, maybe S_NO_DELETE.

So I could change S_KERNEL_FILE to S_KERNEL_LOCK, say, or maybe S_EXCLUSIVE.

David


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/11] cifs: Support fscache indexing rewrite
  2022-01-19  8:32   ` David Howells
@ 2022-01-19  9:48     ` Shyam Prasad N
  2022-01-19 10:38     ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: Shyam Prasad N @ 2022-01-19  9:48 UTC (permalink / raw)
  To: David Howells; +Cc: Steve French, CIFS, Jeff Layton, rohiths msft

On Wed, Jan 19, 2022 at 2:02 PM David Howells <dhowells@redhat.com> wrote:
>
> Steve French <smfrench@gmail.com> wrote:
>
> > WARNING: Missing a blank line after declarations
> > #460: FILE: fs/cifs/file.c:658:
> > + struct cifs_fscache_inode_coherency_data cd;
> > + cifs_fscache_fill_coherency(file_inode(file), &cd);
>
> I have a small patch to abstract cache invalidation for cifs into a helper
> function (see attached) that I'll merge in that will also take care of this.
>
> David
> ---
> commit ff463eee039fbe119ae0d4185cb8a90aec10ec80
> Author: David Howells <dhowells@redhat.com>
> Date:   Fri Jan 7 18:08:37 2022 +0000
>
>     cifs: Abstract cache invalidation into a helper function
>
>     Abstract fscache invalidation for a cifs inode out into a helper function
>     as there will be more than one caller of it.
>
>     Signed-off-by: David Howells <dhowells@redhat.com>
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 27604eb01a94..015fd415e5ee 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -653,13 +653,9 @@ int cifs_open(struct inode *inode, struct file *file)
>                            file->f_mode & FMODE_WRITE);
>         if (file->f_flags & O_DIRECT &&
>             (!((file->f_flags & O_ACCMODE) != O_RDONLY) ||
> -            file->f_flags & O_APPEND)) {
> -               struct cifs_fscache_inode_coherency_data cd;
> -               cifs_fscache_fill_coherency(file_inode(file), &cd);
> -               fscache_invalidate(cifs_inode_cookie(file_inode(file)),
> -                                  &cd, i_size_read(file_inode(file)),
> -                                  FSCACHE_INVAL_DIO_WRITE);
> -       }
> +            file->f_flags & O_APPEND))
> +               cifs_invalidate_cache(file_inode(file),
> +                                     FSCACHE_INVAL_DIO_WRITE);
>
>  out:
>         free_dentry_path(page);
> diff --git a/fs/cifs/fscache.h b/fs/cifs/fscache.h
> index e444445d0906..b741d38df6c8 100644
> --- a/fs/cifs/fscache.h
> +++ b/fs/cifs/fscache.h
> @@ -71,6 +71,15 @@ static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode)
>         return netfs_i_cookie(inode);
>  }
>
> +static inline void cifs_invalidate_cache(struct inode *inode, unsigned int flags)
> +{
> +       struct cifs_fscache_inode_coherency_data cd;
> +
> +       cifs_fscache_fill_coherency(inode, &cd);
> +       fscache_invalidate(cifs_inode_cookie(inode), &cd,
> +                          i_size_read(inode), flags);
> +}
> +
>  static inline int cifs_readpage_from_fscache(struct inode *inode,
>                                              struct page *page)
>  {
> @@ -112,6 +121,7 @@ static inline void cifs_fscache_get_inode_cookie(struct inode *inode) {}
>  static inline void cifs_fscache_release_inode_cookie(struct inode *inode) {}
>  static inline void cifs_fscache_unuse_inode_cookie(struct inode *inode, bool update) {}
>  static inline struct fscache_cookie *cifs_inode_cookie(struct inode *inode) { return NULL; }
> +static inline void cifs_invalidate_cache(struct inode *inode, unsigned int flags) {}
>
>  static inline int
>  cifs_readpage_from_fscache(struct inode *inode, struct page *page)
>
Hi David,

Can you let us know the branch name that you're working on in your tree?
I do not see this last patch in fscache-rewrite branch. Is there
another branch we should be looking at?

-- 
Regards,
Shyam

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/11] cifs: Support fscache indexing rewrite
  2022-01-19  8:32   ` David Howells
  2022-01-19  9:48     ` Shyam Prasad N
@ 2022-01-19 10:38     ` David Howells
  1 sibling, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-19 10:38 UTC (permalink / raw)
  To: Shyam Prasad N; +Cc: dhowells, Steve French, CIFS, Jeff Layton, rohiths msft

Shyam Prasad N <nspmangalore@gmail.com> wrote:

> Can you let us know the branch name that you're working on in your tree?
> I do not see this last patch in fscache-rewrite branch. Is there
> another branch we should be looking at?

I hadn't got round to pushing it yet.  I've done that now.

David


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-19  9:18     ` David Howells
@ 2022-01-19 11:15       ` Christian Brauner
  2022-01-20  9:08       ` Christoph Hellwig
  2022-01-20  9:37       ` David Howells
  2 siblings, 0 replies; 27+ messages in thread
From: Christian Brauner @ 2022-01-19 11:15 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

On Wed, Jan 19, 2022 at 09:18:05AM +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > On Tue, Jan 18, 2022 at 05:40:14PM +0000, David Howells wrote:
> > > Christoph Hellwig <hch@infradead.org> wrote:
> > > 
> > > > On Tue, Jan 18, 2022 at 01:54:54PM +0000, David Howells wrote:
> > > > > Add an IS_KERNEL_FILE() macro to test the S_KERNEL_FILE inode flag as is
> > > > > common practice for the other inode flags[1].
> > > > 
> > > > Please fix the flag to have a sensible name first, as the naming of the
> > > > flag and this new helper is utterly wrong as we already discussed.
> > > 
> > > And I suggested a new name, which you didn't comment on.
> > 
> > Again, look at the semantics of the flag:  The only thing it does in the
> > VFS is to prevent a rmdir.  So you might want to name it after that.
> > 
> > Or in fact drop the flag entirely.  We don't have that kind of
> > protection for other in-kernel file use or important userspace daemons
> > either.  I can't see why cachefiles is the magic snowflake here that
> > suddenly needs semantics no one else has.
> 
> The flag cannot just be dropped - it's an important part of the interaction
> with cachefilesd with regard to culling.  Culling to free up space is
> offloaded to userspace rather than being done within the kernel.
> 
> Previously, cachefiles, the kernel module, had to maintain a huge tree of
> records of every backing inode that it was currently using so that it could
> forbid cachefilesd to cull one when cachefilesd asked.  I've reduced that to a
> single bit flag on the inode struct, thereby saving both memory and time.  You
> can argue whether it's worth sacrificing an inode flag bit for that, but the
> flag can be reused for any other kernel service that wants to similarly mark
> an inode in use.
> 
> Further, it's used as a mark to prevent cachefiles accidentally using an inode
> twice - say someone misconfigures a second cache overlapping the first - and,
> again, this works if some other kernel driver wants to mark inode it is using
> in use.  Cachefiles will refuse to use them if it ever sees them, so no
> problem there.
> 
> And it's not true that we don't have that kind of protection for other
> in-kernel file use.  See S_SWAPFILE.  I did consider using that, but that has
> other side effects.  I mentioned that perhaps I should make swapon set
> S_KERNEL_FILE also.  Also blockdevs have some exclusion also, I think.
> 
> The rmdir thing should really apply to rename and unlink also.  That's to
> prevent someone, cachefilesd included, causing cachefiles to malfunction by
> removing the directories it created.  Possibly this should be a separate bit
> to S_KERNEL_FILE, maybe S_NO_DELETE.
> 
> So I could change S_KERNEL_FILE to S_KERNEL_LOCK, say, or maybe S_EXCLUSIVE.

[ ] S_REMOVE_PROTECTED
[ ] S_UNREMOVABLE
[ ] S_HELD_BUSY
[ ] S_KERNEL_BUSY
[ ] S_BUSY_INTERNAL
[ ] S_BUSY
[ ] S_HELD

?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-19  9:18     ` David Howells
  2022-01-19 11:15       ` Christian Brauner
@ 2022-01-20  9:08       ` Christoph Hellwig
  2022-01-20  9:37       ` David Howells
  2 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2022-01-20  9:08 UTC (permalink / raw)
  To: David Howells
  Cc: Christoph Hellwig, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

On Wed, Jan 19, 2022 at 09:18:05AM +0000, David Howells wrote:
> The flag cannot just be dropped - it's an important part of the interaction
> with cachefilesd with regard to culling.  Culling to free up space is
> offloaded to userspace rather than being done within the kernel.
> 
> Previously, cachefiles, the kernel module, had to maintain a huge tree of
> records of every backing inode that it was currently using so that it could
> forbid cachefilesd to cull one when cachefilesd asked.  I've reduced that to a
> single bit flag on the inode struct, thereby saving both memory and time.  You
> can argue whether it's worth sacrificing an inode flag bit for that, but the
> flag can be reused for any other kernel service that wants to similarly mark
> an inode in use.

Which is a horrible interface.   But you tricked Linus into merging this
crap, so let's not pretent it is a "kernel file".  We have plenty of
those, basically every caller of filp_open is one.

It is something like "pinned for fscache/cachefiles", so name it that
way and add a big fat comment expaining the atrocities.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag
  2022-01-19  9:18     ` David Howells
  2022-01-19 11:15       ` Christian Brauner
  2022-01-20  9:08       ` Christoph Hellwig
@ 2022-01-20  9:37       ` David Howells
  2 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2022-01-20  9:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dhowells, linux-cachefs, Jeff Layton, Trond Myklebust,
	Anna Schumaker, Steve French, Dominique Martinet, Matthew Wilcox,
	Alexander Viro, Omar Sandoval, JeffleXu, Linus Torvalds,
	linux-afs, linux-nfs, linux-cifs, ceph-devel, v9fs-developer,
	linux-fsdevel, linux-kernel

Christoph Hellwig <hch@infradead.org> wrote:

> But you tricked Linus

Tricked?  I put a notice explicitly pointing out that I was adding it and
indicating that it might be controversial in the cover note and the pull
request and further explained the use in the patches that handle it.  I posted
the patches adding/using it a bunch of times to various mailing lists.  TYVM.

David


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages
  2022-01-18 13:53 ` [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages David Howells
@ 2022-01-21 17:47   ` Jeff Layton
  0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2022-01-21 17:47 UTC (permalink / raw)
  To: David Howells, linux-cachefs
  Cc: Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

On Tue, 2022-01-18 at 13:53 +0000, David Howells wrote:
> Cachefiles keeps track of how much space is available on the backing
> filesystem and refuses new writes permission to start if there isn't enough
> (we especially don't want ENOSPC happening).  It also tracks the amount of
> data pending in DIO writes (cache->b_writing) and reduces the amount of
> free space available by this amount before deciding if it can set up a new
> write.
> 
> However, the old fscache I/O API was very much page-granularity dependent
> and, as such, cachefiles's cache->bshift was meant to be a multiplier to
> get from PAGE_SIZE to block size (ie. a blocksize of 512 would give a shift
> of 3 for a 4KiB page) - and this was incorrectly being used to turn the
> number of bytes in a DIO write into a number of blocks, leading to a
> massive over estimation of the amount of data in flight.
> 
> Fix this by changing cache->bshift to be a multiplier from bytes to
> blocksize and deal with quantities of blocks, not quantities of pages.
> 
> Fix also the rounding in the calculation in cachefiles_write() which needs
> a "- 1" inserting.
> 
> Fixes: 047487c947e8 ("cachefiles: Implement the I/O routines")
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-cachefs@redhat.com
> ---
> 
>  fs/cachefiles/cache.c    |    7 ++-----
>  fs/cachefiles/internal.h |    2 +-
>  fs/cachefiles/io.c       |    2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/cachefiles/cache.c b/fs/cachefiles/cache.c
> index ce4d4785003c..1e9c71666c6a 100644
> --- a/fs/cachefiles/cache.c
> +++ b/fs/cachefiles/cache.c
> @@ -84,9 +84,7 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
>  		goto error_unsupported;
>  
>  	cache->bsize = stats.f_bsize;
> -	cache->bshift = 0;
> -	if (stats.f_bsize < PAGE_SIZE)
> -		cache->bshift = PAGE_SHIFT - ilog2(stats.f_bsize);
> +	cache->bshift = ilog2(stats.f_bsize);
>  
>  	_debug("blksize %u (shift %u)",
>  	       cache->bsize, cache->bshift);
> @@ -106,7 +104,6 @@ int cachefiles_add_cache(struct cachefiles_cache *cache)
>  	       (unsigned long long) cache->fcull,
>  	       (unsigned long long) cache->fstop);
>  
> -	stats.f_blocks >>= cache->bshift;
>  	do_div(stats.f_blocks, 100);
>  	cache->bstop = stats.f_blocks * cache->bstop_percent;
>  	cache->bcull = stats.f_blocks * cache->bcull_percent;
> @@ -209,7 +206,7 @@ int cachefiles_has_space(struct cachefiles_cache *cache,
>  		return ret;
>  	}
>  
> -	b_avail = stats.f_bavail >> cache->bshift;
> +	b_avail = stats.f_bavail;
>  	b_writing = atomic_long_read(&cache->b_writing);
>  	if (b_avail > b_writing)
>  		b_avail -= b_writing;
> diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
> index 8dd54d9375b6..c793d33b0224 100644
> --- a/fs/cachefiles/internal.h
> +++ b/fs/cachefiles/internal.h
> @@ -86,7 +86,7 @@ struct cachefiles_cache {
>  	unsigned			bcull_percent;	/* when to start culling (% blocks) */
>  	unsigned			bstop_percent;	/* when to stop allocating (% blocks) */
>  	unsigned			bsize;		/* cache's block size */
> -	unsigned			bshift;		/* min(ilog2(PAGE_SIZE / bsize), 0) */
> +	unsigned			bshift;		/* ilog2(bsize) */
>  	uint64_t			frun;		/* when to stop culling */
>  	uint64_t			fcull;		/* when to start culling */
>  	uint64_t			fstop;		/* when to stop allocating */
> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
> index 60b1eac2ce78..04eb52736990 100644
> --- a/fs/cachefiles/io.c
> +++ b/fs/cachefiles/io.c
> @@ -264,7 +264,7 @@ static int cachefiles_write(struct netfs_cache_resources *cres,
>  	ki->term_func		= term_func;
>  	ki->term_func_priv	= term_func_priv;
>  	ki->was_async		= true;
> -	ki->b_writing		= (len + (1 << cache->bshift)) >> cache->bshift;
> +	ki->b_writing		= (len + (1 << cache->bshift) - 1) >> cache->bshift;
>  
>  	if (ki->term_func)
>  		ki->iocb.ki_complete = cachefiles_write_complete;
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/11] cachefiles: set default tag name if it's unspecified
  2022-01-18 13:53 ` [PATCH 03/11] cachefiles: set default tag name if it's unspecified David Howells
@ 2022-01-21 17:51   ` Jeff Layton
  0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2022-01-21 17:51 UTC (permalink / raw)
  To: David Howells, linux-cachefs
  Cc: Jeffle Xu, Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, Linus Torvalds, linux-afs, linux-nfs, linux-cifs,
	ceph-devel, v9fs-developer, linux-fsdevel, linux-kernel

On Tue, 2022-01-18 at 13:53 +0000, David Howells wrote:
> From: Jeffle Xu <jefflexu@linux.alibaba.com>
> 
> fscache_acquire_cache() requires a non-empty name, while 'tag <name>'
> command is optional for cachefilesd.
> 
> Thus set default tag name if it's unspecified to avoid the regression of
> cachefilesd. The logic is the same with that before rewritten.
> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-cachefs@redhat.com
> ---
> 
>  fs/cachefiles/daemon.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
> index 40a792421fc1..7ac04ee2c0a0 100644
> --- a/fs/cachefiles/daemon.c
> +++ b/fs/cachefiles/daemon.c
> @@ -703,6 +703,17 @@ static int cachefiles_daemon_bind(struct cachefiles_cache *cache, char *args)
>  		return -EBUSY;
>  	}
>  
> +	/* Make sure we have copies of the tag string */
> +	if (!cache->tag) {
> +		/*
> +		 * The tag string is released by the fops->release()
> +		 * function, so we don't release it on error here
> +		 */
> +		cache->tag = kstrdup("CacheFiles", GFP_KERNEL);
> +		if (!cache->tag)
> +			return -ENOMEM;
> +	}
> +
>  	return cachefiles_add_cache(cache);
>  }
>  
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 04/11] cachefiles: Make some tracepoint adjustments
  2022-01-18 13:53 ` [PATCH 04/11] cachefiles: Make some tracepoint adjustments David Howells
@ 2022-01-21 17:52   ` Jeff Layton
  0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2022-01-21 17:52 UTC (permalink / raw)
  To: David Howells, linux-cachefs
  Cc: Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

On Tue, 2022-01-18 at 13:53 +0000, David Howells wrote:
> Make some adjustments to tracepoints to make the tracing a bit more
> followable:
> 
>  (1) Standardise on displaying the backing inode number as "B=<hex>" with
>      no leading zeros.
> 
>  (2) Make the cachefiles_lookup tracepoint log the directory inode number
>      as well as the looked-up inode number.
> 
>  (3) Add a cachefiles_lookup tracepoint into cachefiles_get_directory() to
>      log directory lookup.
> 
>  (4) Add a new cachefiles_mkdir tracepoint and use that to log a successful
>      mkdir from cachefiles_get_directory().
> 
>  (5) Make the cachefiles_unlink and cachefiles_rename tracepoints log the
>      inode number of the affected file/dir rather than dentry struct
>      pointers.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-cachefs@redhat.com
> ---
> 
>  fs/cachefiles/namei.c             |    8 ++--
>  include/trace/events/cachefiles.h |   82 +++++++++++++++++++++++--------------
>  2 files changed, 56 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 9bd692870617..52c9f0864a87 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -101,6 +101,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
>  		subdir = lookup_one_len(dirname, dir, strlen(dirname));
>  	else
>  		subdir = ERR_PTR(ret);
> +	trace_cachefiles_lookup(NULL, dir, subdir);
>  	if (IS_ERR(subdir)) {
>  		trace_cachefiles_vfs_error(NULL, d_backing_inode(dir),
>  					   PTR_ERR(subdir),
> @@ -135,6 +136,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
>  						   cachefiles_trace_mkdir_error);
>  			goto mkdir_error;
>  		}
> +		trace_cachefiles_mkdir(dir, subdir);
>  
>  		if (unlikely(d_unhashed(subdir))) {
>  			cachefiles_put_directory(subdir);
> @@ -233,7 +235,7 @@ static int cachefiles_unlink(struct cachefiles_cache *cache,
>  	};
>  	int ret;
>  
> -	trace_cachefiles_unlink(object, dentry, why);
> +	trace_cachefiles_unlink(object, d_inode(dentry)->i_ino, why);
>  	ret = security_path_unlink(&path, dentry);
>  	if (ret < 0) {
>  		cachefiles_io_error(cache, "Unlink security error");
> @@ -386,7 +388,7 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
>  			.new_dir	= d_inode(cache->graveyard),
>  			.new_dentry	= grave,
>  		};
> -		trace_cachefiles_rename(object, rep, grave, why);
> +		trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
>  		ret = cachefiles_inject_read_error();
>  		if (ret == 0)
>  			ret = vfs_rename(&rd);
> @@ -617,7 +619,7 @@ bool cachefiles_look_up_object(struct cachefiles_object *object)
>  						  object->d_name_len);
>  	else
>  		dentry = ERR_PTR(ret);
> -	trace_cachefiles_lookup(object, dentry);
> +	trace_cachefiles_lookup(object, fan, dentry);
>  	if (IS_ERR(dentry)) {
>  		if (dentry == ERR_PTR(-ENOENT))
>  			goto new_file;
> diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
> index 1172529b5b49..093c4acb7a3a 100644
> --- a/include/trace/events/cachefiles.h
> +++ b/include/trace/events/cachefiles.h
> @@ -233,25 +233,48 @@ TRACE_EVENT(cachefiles_ref,
>  
>  TRACE_EVENT(cachefiles_lookup,
>  	    TP_PROTO(struct cachefiles_object *obj,
> +		     struct dentry *dir,
>  		     struct dentry *de),
>  
> -	    TP_ARGS(obj, de),
> +	    TP_ARGS(obj, dir, de),
>  
>  	    TP_STRUCT__entry(
>  		    __field(unsigned int,		obj	)
>  		    __field(short,			error	)
> +		    __field(unsigned long,		dino	)
>  		    __field(unsigned long,		ino	)
>  			     ),
>  
>  	    TP_fast_assign(
> -		    __entry->obj	= obj->debug_id;
> +		    __entry->obj	= obj ? obj->debug_id : 0;
> +		    __entry->dino	= d_backing_inode(dir)->i_ino;
>  		    __entry->ino	= (!IS_ERR(de) && d_backing_inode(de) ?
>  					   d_backing_inode(de)->i_ino : 0);
>  		    __entry->error	= IS_ERR(de) ? PTR_ERR(de) : 0;
>  			   ),
>  
> -	    TP_printk("o=%08x i=%lx e=%d",
> -		      __entry->obj, __entry->ino, __entry->error)
> +	    TP_printk("o=%08x dB=%lx B=%lx e=%d",
> +		      __entry->obj, __entry->dino, __entry->ino, __entry->error)
> +	    );
> +
> +TRACE_EVENT(cachefiles_mkdir,
> +	    TP_PROTO(struct dentry *dir, struct dentry *subdir),
> +
> +	    TP_ARGS(dir, subdir),
> +
> +	    TP_STRUCT__entry(
> +		    __field(unsigned int,			dir	)
> +		    __field(unsigned int,			subdir	)
> +			     ),
> +
> +	    TP_fast_assign(
> +		    __entry->dir	= d_backing_inode(dir)->i_ino;
> +		    __entry->subdir	= d_backing_inode(subdir)->i_ino;
> +			   ),
> +
> +	    TP_printk("dB=%x sB=%x",
> +		      __entry->dir,
> +		      __entry->subdir)
>  	    );
>  
>  TRACE_EVENT(cachefiles_tmpfile,
> @@ -269,7 +292,7 @@ TRACE_EVENT(cachefiles_tmpfile,
>  		    __entry->backer	= backer->i_ino;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x",
> +	    TP_printk("o=%08x B=%x",
>  		      __entry->obj,
>  		      __entry->backer)
>  	    );
> @@ -289,61 +312,58 @@ TRACE_EVENT(cachefiles_link,
>  		    __entry->backer	= backer->i_ino;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x",
> +	    TP_printk("o=%08x B=%x",
>  		      __entry->obj,
>  		      __entry->backer)
>  	    );
>  
>  TRACE_EVENT(cachefiles_unlink,
>  	    TP_PROTO(struct cachefiles_object *obj,
> -		     struct dentry *de,
> +		     ino_t ino,
>  		     enum fscache_why_object_killed why),
>  
> -	    TP_ARGS(obj, de, why),
> +	    TP_ARGS(obj, ino, why),
>  
>  	    /* Note that obj may be NULL */
>  	    TP_STRUCT__entry(
>  		    __field(unsigned int,		obj		)
> -		    __field(struct dentry *,		de		)
> +		    __field(unsigned int,		ino		)
>  		    __field(enum fscache_why_object_killed, why		)
>  			     ),
>  
>  	    TP_fast_assign(
>  		    __entry->obj	= obj ? obj->debug_id : UINT_MAX;
> -		    __entry->de		= de;
> +		    __entry->ino	= ino;
>  		    __entry->why	= why;
>  			   ),
>  
> -	    TP_printk("o=%08x d=%p w=%s",
> -		      __entry->obj, __entry->de,
> +	    TP_printk("o=%08x B=%x w=%s",
> +		      __entry->obj, __entry->ino,
>  		      __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
>  	    );
>  
>  TRACE_EVENT(cachefiles_rename,
>  	    TP_PROTO(struct cachefiles_object *obj,
> -		     struct dentry *de,
> -		     struct dentry *to,
> +		     ino_t ino,
>  		     enum fscache_why_object_killed why),
>  
> -	    TP_ARGS(obj, de, to, why),
> +	    TP_ARGS(obj, ino, why),
>  
>  	    /* Note that obj may be NULL */
>  	    TP_STRUCT__entry(
>  		    __field(unsigned int,		obj		)
> -		    __field(struct dentry *,		de		)
> -		    __field(struct dentry *,		to		)
> +		    __field(unsigned int,		ino		)
>  		    __field(enum fscache_why_object_killed, why		)
>  			     ),
>  
>  	    TP_fast_assign(
>  		    __entry->obj	= obj ? obj->debug_id : UINT_MAX;
> -		    __entry->de		= de;
> -		    __entry->to		= to;
> +		    __entry->ino	= ino;
>  		    __entry->why	= why;
>  			   ),
>  
> -	    TP_printk("o=%08x d=%p t=%p w=%s",
> -		      __entry->obj, __entry->de, __entry->to,
> +	    TP_printk("o=%08x B=%x w=%s",
> +		      __entry->obj, __entry->ino,
>  		      __print_symbolic(__entry->why, cachefiles_obj_kill_traces))
>  	    );
>  
> @@ -370,7 +390,7 @@ TRACE_EVENT(cachefiles_coherency,
>  		    __entry->ino	= ino;
>  			   ),
>  
> -	    TP_printk("o=%08x %s i=%llx c=%u",
> +	    TP_printk("o=%08x %s B=%llx c=%u",
>  		      __entry->obj,
>  		      __print_symbolic(__entry->why, cachefiles_coherency_traces),
>  		      __entry->ino,
> @@ -397,7 +417,7 @@ TRACE_EVENT(cachefiles_vol_coherency,
>  		    __entry->ino	= ino;
>  			   ),
>  
> -	    TP_printk("V=%08x %s i=%llx",
> +	    TP_printk("V=%08x %s B=%llx",
>  		      __entry->vol,
>  		      __print_symbolic(__entry->why, cachefiles_coherency_traces),
>  		      __entry->ino)
> @@ -435,7 +455,7 @@ TRACE_EVENT(cachefiles_prep_read,
>  		    __entry->cache_inode = cache_inode;
>  			   ),
>  
> -	    TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x b=%x",
> +	    TP_printk("R=%08x[%u] %s %s f=%02x s=%llx %zx ni=%x B=%x",
>  		      __entry->rreq, __entry->index,
>  		      __print_symbolic(__entry->source, netfs_sreq_sources),
>  		      __print_symbolic(__entry->why, cachefiles_prepare_read_traces),
> @@ -466,7 +486,7 @@ TRACE_EVENT(cachefiles_read,
>  		    __entry->len	= len;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x s=%llx l=%zx",
> +	    TP_printk("o=%08x B=%x s=%llx l=%zx",
>  		      __entry->obj,
>  		      __entry->backer,
>  		      __entry->start,
> @@ -495,7 +515,7 @@ TRACE_EVENT(cachefiles_write,
>  		    __entry->len	= len;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x s=%llx l=%zx",
> +	    TP_printk("o=%08x B=%x s=%llx l=%zx",
>  		      __entry->obj,
>  		      __entry->backer,
>  		      __entry->start,
> @@ -524,7 +544,7 @@ TRACE_EVENT(cachefiles_trunc,
>  		    __entry->why	= why;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x %s l=%llx->%llx",
> +	    TP_printk("o=%08x B=%x %s l=%llx->%llx",
>  		      __entry->obj,
>  		      __entry->backer,
>  		      __print_symbolic(__entry->why, cachefiles_trunc_traces),
> @@ -549,7 +569,7 @@ TRACE_EVENT(cachefiles_mark_active,
>  		    __entry->inode	= inode->i_ino;
>  			   ),
>  
> -	    TP_printk("o=%08x i=%lx",
> +	    TP_printk("o=%08x B=%lx",
>  		      __entry->obj, __entry->inode)
>  	    );
>  
> @@ -570,7 +590,7 @@ TRACE_EVENT(cachefiles_mark_inactive,
>  		    __entry->inode	= inode->i_ino;
>  			   ),
>  
> -	    TP_printk("o=%08x i=%lx",
> +	    TP_printk("o=%08x B=%lx",
>  		      __entry->obj, __entry->inode)
>  	    );
>  
> @@ -594,7 +614,7 @@ TRACE_EVENT(cachefiles_vfs_error,
>  		    __entry->where	= where;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x %s e=%d",
> +	    TP_printk("o=%08x B=%x %s e=%d",
>  		      __entry->obj,
>  		      __entry->backer,
>  		      __print_symbolic(__entry->where, cachefiles_error_traces),
> @@ -621,7 +641,7 @@ TRACE_EVENT(cachefiles_io_error,
>  		    __entry->where	= where;
>  			   ),
>  
> -	    TP_printk("o=%08x b=%08x %s e=%d",
> +	    TP_printk("o=%08x B=%x %s e=%d",
>  		      __entry->obj,
>  		      __entry->backer,
>  		      __print_symbolic(__entry->where, cachefiles_error_traces),
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 05/11] cachefiles: Trace active-mark failure
  2022-01-18 13:54 ` [PATCH 05/11] cachefiles: Trace active-mark failure David Howells
@ 2022-01-21 17:53   ` Jeff Layton
  0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2022-01-21 17:53 UTC (permalink / raw)
  To: David Howells, linux-cachefs
  Cc: Trond Myklebust, Anna Schumaker, Steve French,
	Dominique Martinet, Matthew Wilcox, Alexander Viro,
	Omar Sandoval, JeffleXu, Linus Torvalds, linux-afs, linux-nfs,
	linux-cifs, ceph-devel, v9fs-developer, linux-fsdevel,
	linux-kernel

On Tue, 2022-01-18 at 13:54 +0000, David Howells wrote:
> Add a tracepoint to log failure to apply an active mark to a file in
> addition to tracing successfully setting and unsetting the mark.
> 
> Also include the backing file inode number in the message logged to dmesg.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-cachefs@redhat.com
> ---
> 
>  fs/cachefiles/namei.c             |    4 +++-
>  include/trace/events/cachefiles.h |   21 +++++++++++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 52c9f0864a87..f256c8aff7bb 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -25,7 +25,9 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
>  		trace_cachefiles_mark_active(object, inode);
>  		can_use = true;
>  	} else {
> -		pr_notice("cachefiles: Inode already in use: %pd\n", dentry);
> +		trace_cachefiles_mark_failed(object, inode);
> +		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
> +			  dentry, inode->i_ino);
>  	}
>  
>  	return can_use;
> diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
> index 093c4acb7a3a..c6f5aa74db89 100644
> --- a/include/trace/events/cachefiles.h
> +++ b/include/trace/events/cachefiles.h
> @@ -573,6 +573,27 @@ TRACE_EVENT(cachefiles_mark_active,
>  		      __entry->obj, __entry->inode)
>  	    );
>  
> +TRACE_EVENT(cachefiles_mark_failed,
> +	    TP_PROTO(struct cachefiles_object *obj,
> +		     struct inode *inode),
> +
> +	    TP_ARGS(obj, inode),
> +
> +	    /* Note that obj may be NULL */
> +	    TP_STRUCT__entry(
> +		    __field(unsigned int,		obj		)
> +		    __field(ino_t,			inode		)
> +			     ),
> +
> +	    TP_fast_assign(
> +		    __entry->obj	= obj ? obj->debug_id : 0;
> +		    __entry->inode	= inode->i_ino;
> +			   ),
> +
> +	    TP_printk("o=%08x B=%lx",
> +		      __entry->obj, __entry->inode)
> +	    );
> +
>  TRACE_EVENT(cachefiles_mark_inactive,
>  	    TP_PROTO(struct cachefiles_object *obj,
>  		     struct inode *inode),
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-01-21 17:53 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18 13:52 [PATCH 00/11] fscache, cachefiles: Rewrite fixes/updates David Howells
2022-01-18 13:53 ` [PATCH 01/11] fscache: Fix the volume collision wait condition David Howells
2022-01-18 13:53 ` [PATCH 02/11] cachefiles: Calculate the blockshift in terms of bytes, not pages David Howells
2022-01-21 17:47   ` Jeff Layton
2022-01-18 13:53 ` [PATCH 03/11] cachefiles: set default tag name if it's unspecified David Howells
2022-01-21 17:51   ` Jeff Layton
2022-01-18 13:53 ` [PATCH 04/11] cachefiles: Make some tracepoint adjustments David Howells
2022-01-21 17:52   ` Jeff Layton
2022-01-18 13:54 ` [PATCH 05/11] cachefiles: Trace active-mark failure David Howells
2022-01-21 17:53   ` Jeff Layton
2022-01-18 13:54 ` [PATCH 06/11] cachefiles: Explain checks in a comment David Howells
2022-01-18 13:54 ` [PATCH 07/11] cachefiles: Check that the backing filesystem supports tmpfiles David Howells
2022-01-18 13:54 ` [PATCH 08/11] fscache: Add a comment explaining how page-release optimisation works David Howells
2022-01-18 13:54 ` [PATCH 09/11] vfs, fscache: Add an IS_KERNEL_FILE() macro for the S_KERNEL_FILE flag David Howells
2022-01-18 16:23   ` Christoph Hellwig
2022-01-18 17:40   ` David Howells
2022-01-19  5:20     ` Christoph Hellwig
2022-01-19  9:18     ` David Howells
2022-01-19 11:15       ` Christian Brauner
2022-01-20  9:08       ` Christoph Hellwig
2022-01-20  9:37       ` David Howells
2022-01-18 13:55 ` [PATCH 10/11] netfs: Make ops->init_rreq() optional David Howells
2022-01-18 13:55 ` [PATCH 11/11] cifs: Support fscache indexing rewrite David Howells
2022-01-19  5:31   ` Steve French
2022-01-19  8:32   ` David Howells
2022-01-19  9:48     ` Shyam Prasad N
2022-01-19 10:38     ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).