[PATCH v3] fscache, cachefiles: Fix resize handling

[PATCH v3] fscache, cachefiles: Fix resize handling

[David Howells]

Fix the resize handling in fscache and cachefiles by passing down a flag to
indicate if the file is being shrunk as the cache's idea of how big the
file should be may be stale and thus not useful in comparison to the
object's size.

The issue is that netfs_perform_write() updates the network filesystem's
file's i_size as it adds data to the pagecache, but the cache isn't updated
until writeback flushes that data.  A truncation may then scrap that data
and so the cache never sees it and doesn't update before it is told to
resize.  In the resize, cachefiles was comparing the new size against the
current cache size and then thinking the file was being expanded.

Note that this is a bit more complicated for ceph than other filesystems as
ceph splits the truncation of the local pagecache and fscache to after the
RPC reply handler has updated the inode record - the reply handler
communicates the need for the truncation finalisation through ceph inode
flags.

To this end, ceph is changed such that the outstanding truncation count in the
inode turns into a bool and just set instead of being incremented (the code
only cares if it's zero or non-zero) and then a second bool is added to record
if the file was being shrunk.

Reported-by: Marc Dionne <marc.dionne@auristor.com>
Fixes: 1a9fe4b6e787 ("cachefiles: Don't rely on backing fs storage map for most use cases")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Ilya Dryomov <idryomov@gmail.com>
cc: Alex Markuze <amarkuze@redhat.com>
cc: Viacheslav Dubeyko <slava@dubeyko.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: v9fs@lists.linux.dev
cc: linux-afs@lists.infradead.org
cc: ceph-devel@vger.kernel.org
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---    
 Changes
 =======
 ver #3)
 - Fix issues reported by sashiko[2]:
   - In afs_setattr_edit_file(), get the "did it shrink" test the right way
     round.
   - In ceph_alloc_inode(), initialised the i_truncate_shrank field.
   - In __ceph_do_pending_vmtruncate(), read the i_truncate_shrank field
     inside the lock.
   - In cifs_precopy_set_eof(), assume no shrinkage of the cache file as
     this func is used to expand the server file to cover a hole in the
     pagecache.
   - In cifs_file_copychunk_range(), assume no shrinkage of the file.
   - In smb3_zero_range(), assume no shrinkage of the file.
   - In smb3_collapse_range(), assume the file always shrinks.
   - In smb3_insert_range(), assume no shrinkage of the file.
 
 ver #2)
 - Fix issues reported by sashiko[1]:
   - In cifs_file_copychunk_range(), use the old size of the target inode,
     not the source inode.
   - Switch to passing down a flag to indicate if the file shrank rather
     than the old file size.
   - In ceph, cache the truncate-shrank flag in the ceph inode.
 
 [1] https://sashiko.dev/#/patchset/938341.1791446974%40warthog.procyon.org.uk
 [2] https://sashiko.dev/#/patchset/1029097.1791466484%40warthog.procyon.org.uk

 Documentation/filesystems/caching/backend-api.rst |    2 +-
 Documentation/filesystems/caching/netfs-api.rst   |    2 +-
 fs/9p/vfs_inode.c                                 |    6 +++++-
 fs/9p/vfs_inode_dotl.c                            |    5 ++++-
 fs/afs/inode.c                                    |    4 ++--
 fs/cachefiles/interface.c                         |    8 ++++----
 fs/ceph/cache.h                                   |    6 +++---
 fs/ceph/inode.c                                   |   16 +++++++++++-----
 fs/ceph/super.h                                   |    3 ++-
 fs/netfs/fscache_io.c                             |    4 ++--
 fs/smb/client/cifsfs.c                            |    6 +++---
 fs/smb/client/inode.c                             |    2 +-
 fs/smb/client/smb2ops.c                           |    9 +++++----
 include/linux/fscache-cache.h                     |    2 +-
 include/linux/fscache.h                           |   11 ++++++-----
 15 files changed, 51 insertions(+), 35 deletions(-)

diff --git a/Documentation/filesystems/caching/backend-api.rst b/Documentation/filesystems/caching/backend-api.rst
index 3a199fc50828..df837531b3fd 100644
--- a/Documentation/filesystems/caching/backend-api.rst
+++ b/Documentation/filesystems/caching/backend-api.rst
@@ -339,7 +339,7 @@ pointers to methods to allow fscache to request management of the cache:
    * Change the size of a data storage object [mandatory]::
 
 	void (*resize_cookie)(struct netfs_cache_resources *cres,
-			      loff_t new_size);
+			      loff_t new_size, bool shrank);
 
      This method is called to inform the cache backend of a change in size of
      the netfs file due to local truncation.  The cache backend should make all
diff --git a/Documentation/filesystems/caching/netfs-api.rst b/Documentation/filesystems/caching/netfs-api.rst
index 665b27f1556e..3154a1686480 100644
--- a/Documentation/filesystems/caching/netfs-api.rst
+++ b/Documentation/filesystems/caching/netfs-api.rst
@@ -226,7 +226,7 @@ If a network filesystem file is resized locally by truncation, the following
 should be called to notify the cache::
 
 	void fscache_resize_cookie(struct fscache_cookie *cookie,
-				   loff_t new_size);
+				   loff_t new_size, bool shrank);
 
 The caller must have first marked the cookie in-use.  The cookie and the new
 size are passed in and the cache is synchronously resized.  This is expected to
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 3829554ca369..2d33c53da8d9 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -1022,6 +1022,9 @@ static int v9fs_vfs_setattr(struct mnt_idmap *idmap,
 	struct v9fs_session_info *v9ses;
 	struct p9_fid *fid = NULL;
 	struct p9_wstat wstat;
+#ifdef CONFIG_9P_FSCACHE
+	uoff_t old_size = i_size_read(inode);
+#endif
 
 	p9_debug(P9_DEBUG_VFS, "\n");
 	retval = setattr_prepare(&nop_mnt_idmap, dentry, iattr);
@@ -1086,7 +1089,8 @@ static int v9fs_vfs_setattr(struct mnt_idmap *idmap,
 		if (v9ses->cache & CACHE_FSCACHE) {
 			struct v9fs_inode *v9inode = V9FS_I(inode);
 
-			fscache_resize_cookie(v9fs_inode_cookie(v9inode), iattr->ia_size);
+			fscache_resize_cookie(v9fs_inode_cookie(v9inode),
+					      iattr->ia_size, iattr->ia_size < old_size);
 		}
 #endif
 	}
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 116b29e95f21..dde0dbea2685 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -519,6 +519,9 @@ int v9fs_vfs_setattr_dotl(struct mnt_idmap *idmap,
 		.uid = INVALID_UID,
 		.gid = INVALID_GID,
 	};
+#ifdef CONFIG_9P_FSCACHE
+	uoff_t old_size = i_size_read(inode);
+#endif
 
 	p9_debug(P9_DEBUG_VFS, "\n");
 
@@ -580,7 +583,7 @@ int v9fs_vfs_setattr_dotl(struct mnt_idmap *idmap,
 #ifdef CONFIG_9P_FSCACHE
 		if (v9ses->cache & CACHE_FSCACHE)
 			fscache_resize_cookie(v9fs_inode_cookie(V9FS_I(inode)),
-				iattr->ia_size);
+					      iattr->ia_size, iattr->ia_size < old_size);
 #endif
 	}
 
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 634fbf8eb212..75cb5cd8ba14 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -744,7 +744,7 @@ static void afs_setattr_edit_file(struct afs_operation *op)
 		if (size != old) {
 			truncate_pagecache(inode, size);
 			netfs_resize_file(&vnode->netfs, size, true);
-			fscache_resize_cookie(afs_vnode_cache(vnode), size);
+			fscache_resize_cookie(afs_vnode_cache(vnode), size, size < old);
 		}
 	}
 }
@@ -817,7 +817,7 @@ int afs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			truncate_setsize(inode, attr->ia_size);
 			netfs_resize_file(&vnode->netfs, size, false);
 			fscache_resize_cookie(afs_vnode_cache(vnode),
-					      attr->ia_size);
+					      attr->ia_size, attr->ia_size < i_size);
 			ret = 0;
 			goto out_unlock;
 		}
diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c
index 789ff6abe926..4a31904aaf0e 100644
--- a/fs/cachefiles/interface.c
+++ b/fs/cachefiles/interface.c
@@ -204,7 +204,7 @@ static bool cachefiles_shorten_object(struct cachefiles_object *object,
  * Resize the backing object.
  */
 static void cachefiles_resize_cookie(struct netfs_cache_resources *cres,
-				     uoff_t new_size)
+				     uoff_t new_size, bool shrank)
 {
 	struct cachefiles_object *object = cachefiles_cres_object(cres);
 	struct cachefiles_cache *cache = object->volume->cache;
@@ -215,10 +215,10 @@ static void cachefiles_resize_cookie(struct netfs_cache_resources *cres,
 
 	_enter("%llu->%llu", object->object_size, new_size);
 
-	/* If the file is being shrunk, we need to downsize the backing file
-	 * and clear the end of the final block.
+	/* If the file shrank, we need to downsize the backing file and clear
+	 * the end of the final block.
 	 */
-	if (new_size < object->object_size) {
+	if (shrank) {
 		if (new_size >= i_size)
 			goto out;
 		cachefiles_begin_secure(cache, &saved_cred);
diff --git a/fs/ceph/cache.h b/fs/ceph/cache.h
index 20efac020394..892b18c31741 100644
--- a/fs/ceph/cache.h
+++ b/fs/ceph/cache.h
@@ -31,14 +31,14 @@ static inline struct fscache_cookie *ceph_fscache_cookie(struct ceph_inode_info
 	return netfs_i_cookie(&ci->netfs);
 }
 
-static inline void ceph_fscache_resize(struct inode *inode, loff_t to)
+static inline void ceph_fscache_resize(struct inode *inode, loff_t to, bool shrank)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	struct fscache_cookie *cookie = ceph_fscache_cookie(ci);
 
 	if (cookie) {
 		ceph_fscache_use_cookie(inode, true);
-		fscache_resize_cookie(cookie, to);
+		fscache_resize_cookie(cookie, to, shrank);
 		ceph_fscache_unuse_cookie(inode, true);
 	}
 }
@@ -96,7 +96,7 @@ static inline struct fscache_cookie *ceph_fscache_cookie(struct ceph_inode_info
 	return NULL;
 }
 
-static inline void ceph_fscache_resize(struct inode *inode, loff_t to)
+static inline void ceph_fscache_resize(struct inode *inode, loff_t to, bool shrank)
 {
 }
 
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index d52e2b389e0b..8ed63659ae58 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -680,7 +680,8 @@ struct inode *ceph_alloc_inode(struct super_block *sb)
 	mutex_init(&ci->i_truncate_mutex);
 	ci->i_truncate_seq = 0;
 	ci->i_truncate_size = 0;
-	ci->i_truncate_pending = 0;
+	ci->i_truncate_pending = false;
+	ci->i_truncate_shrank = false;
 	ci->i_truncate_pagecache_size = 0;
 
 	ci->i_max_size = 0;
@@ -847,7 +848,9 @@ int ceph_fill_file_size(struct inode *inode, int issued,
 				       CEPH_CAP_FILE_BUFFER)) ||
 			    mapping_mapped(inode->i_mapping) ||
 			    __ceph_is_file_opened(ci)) {
-				ci->i_truncate_pending++;
+				ci->i_truncate_pending = true;
+				if (size < isize)
+					ci->i_truncate_shrank = true;
 				queue_trunc = 1;
 			}
 		}
@@ -2304,12 +2307,13 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
 	struct ceph_client *cl = ceph_inode_to_client(inode);
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	u64 to;
+	bool shrank;
 	int wrbuffer_refs, finish = 0;
 
 	mutex_lock(&ci->i_truncate_mutex);
 retry:
 	spin_lock(&ci->i_ceph_lock);
-	if (ci->i_truncate_pending == 0) {
+	if (!ci->i_truncate_pending) {
 		doutc(cl, "%p %llx.%llx none pending\n", inode,
 		      ceph_vinop(inode));
 		spin_unlock(&ci->i_ceph_lock);
@@ -2333,18 +2337,20 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
 	/* there should be no reader or writer */
 	WARN_ON_ONCE(ci->i_rd_ref || ci->i_wr_ref);
 
+	shrank = ci->i_truncate_shrank;
 	to = ci->i_truncate_pagecache_size;
 	wrbuffer_refs = ci->i_wrbuffer_ref;
 	doutc(cl, "%p %llx.%llx (%d) to %lld\n", inode, ceph_vinop(inode),
 	      ci->i_truncate_pending, to);
 	spin_unlock(&ci->i_ceph_lock);
 
-	ceph_fscache_resize(inode, to);
+	ceph_fscache_resize(inode, to, shrank);
 	truncate_pagecache(inode, to);
 
 	spin_lock(&ci->i_ceph_lock);
 	if (to == ci->i_truncate_pagecache_size) {
-		ci->i_truncate_pending = 0;
+		ci->i_truncate_shrank = false;
+		ci->i_truncate_pending = false;
 		finish = 1;
 	}
 	spin_unlock(&ci->i_ceph_lock);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 72d4e30304dc..e6eaf1647fe5 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -481,7 +481,8 @@ struct ceph_inode_info {
 	struct mutex i_truncate_mutex;
 	u32 i_truncate_seq;        /* last truncate to smaller size */
 	u64 i_truncate_size;       /*  and the size we last truncated down to */
-	int i_truncate_pending;    /*  still need to call vmtruncate */
+	bool i_truncate_pending;   /*  still need to call vmtruncate */
+	bool i_truncate_shrank;		/* Truncate shrank the file */
 	/*
 	 * For none fscrypt case it equals to i_truncate_size or it will
 	 * equals to fscrypt_file_size
diff --git a/fs/netfs/fscache_io.c b/fs/netfs/fscache_io.c
index 056a2bae5d99..ea0b11807034 100644
--- a/fs/netfs/fscache_io.c
+++ b/fs/netfs/fscache_io.c
@@ -267,7 +267,7 @@ EXPORT_SYMBOL(__fscache_write_to_cache);
 /*
  * Change the size of a backing object.
  */
-void __fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size)
+void __fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size, bool shrank)
 {
 	struct netfs_cache_resources cres;
 
@@ -281,7 +281,7 @@ void __fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size)
 		 * netfs's inode lock so that we're serialised with respect to
 		 * writes.
 		 */
-		cookie->volume->cache->ops->resize_cookie(&cres, new_size);
+		cookie->volume->cache->ops->resize_cookie(&cres, new_size, shrank);
 		fscache_end_operation(&cres);
 	} else {
 		fscache_stat(&fscache_n_resizes_null);
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 98b610b2a114..a60956a40aa9 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1332,7 +1332,7 @@ static int cifs_precopy_set_eof(struct inode *src_inode, struct cifsInodeInfo *s
 		goto set_failed;
 
 	netfs_resize_file(&src_cifsi->netfs, src_end, true);
-	fscache_resize_cookie(cifs_inode_cookie(src_inode), src_end);
+	fscache_resize_cookie(cifs_inode_cookie(src_inode), src_end, false);
 	return 0;
 
 set_failed:
@@ -1626,9 +1626,9 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,
 		if (rc > 0 && destoff + rc > i_size_read(target_inode)) {
 			truncate_setsize(target_inode, destoff + rc);
 			netfs_resize_file(&target_cifsi->netfs,
-					  i_size_read(target_inode), true);
+					  destoff + rc, true);
 			fscache_resize_cookie(cifs_inode_cookie(target_inode),
-					      i_size_read(target_inode));
+					      destoff + rc, false);
 		}
 		if (rc > 0) {
 			spin_lock(&target_inode->i_lock);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index f29555f3c7d0..fb1afe72e6d2 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3097,7 +3097,7 @@ void cifs_resize_file_locked(struct inode *inode, loff_t old_size,
 		return;
 
 	fscache_use_cookie(cookie, true);
-	fscache_resize_cookie(cookie, offset);
+	fscache_resize_cookie(cookie, offset, offset < old_size);
 	cifs_fscache_unuse_inode_cookie(inode, true);
 }
 
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 1e9018880639..f53ba045ba82 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3611,7 +3611,8 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,
 			if (offset < cifsi->netfs._zero_point)
 				netfs_write_zero_point(inode, offset);
 			spin_unlock(&inode->i_lock);
-			fscache_resize_cookie(cifs_inode_cookie(inode), new_size);
+			fscache_resize_cookie(cifs_inode_cookie(inode),
+					      new_size, false);
 		}
 	}
 
@@ -4146,7 +4147,7 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
 	netfs_resize_file(&cifsi->netfs, new_eof, true);
 	netfs_write_zero_point(inode, new_eof);
 	spin_unlock(&inode->i_lock);
-	fscache_resize_cookie(cifs_inode_cookie(inode), new_eof);
+	fscache_resize_cookie(cifs_inode_cookie(inode), new_eof, true);
 out_2:
 	filemap_invalidate_unlock(inode->i_mapping);
 out:
@@ -4206,9 +4207,9 @@ static long smb3_insert_range(struct file *file, struct cifs_tcon *tcon,
 
 	truncate_setsize(inode, new_eof);
 	spin_lock(&inode->i_lock);
-	netfs_resize_file(&cifsi->netfs, i_size_read(inode), true);
+	netfs_resize_file(&cifsi->netfs, new_eof, true);
 	spin_unlock(&inode->i_lock);
-	fscache_resize_cookie(cifs_inode_cookie(inode), i_size_read(inode));
+	fscache_resize_cookie(cifs_inode_cookie(inode), new_eof, false);
 
 	/*
 	 * Move [off, old_eof) right by len. The helper copies backwards if the
diff --git a/include/linux/fscache-cache.h b/include/linux/fscache-cache.h
index ee524c863fa9..2b83ffa4510e 100644
--- a/include/linux/fscache-cache.h
+++ b/include/linux/fscache-cache.h
@@ -67,7 +67,7 @@ struct fscache_cache_ops {
 
 	/* Change the size of a data object */
 	void (*resize_cookie)(struct netfs_cache_resources *cres,
-			      uoff_t new_size);
+			      uoff_t new_size, bool shrank);
 
 	/* Invalidate an object */
 	bool (*invalidate_cookie)(struct fscache_cookie *cookie);
diff --git a/include/linux/fscache.h b/include/linux/fscache.h
index f2d958bd1f48..ecd14d645690 100644
--- a/include/linux/fscache.h
+++ b/include/linux/fscache.h
@@ -184,7 +184,7 @@ extern struct fscache_cookie *__fscache_acquire_cookie(
 extern void __fscache_use_cookie(struct fscache_cookie *, bool);
 extern void __fscache_unuse_cookie(struct fscache_cookie *, const void *, const uoff_t *);
 extern void __fscache_relinquish_cookie(struct fscache_cookie *, bool);
-extern void __fscache_resize_cookie(struct fscache_cookie *, uoff_t);
+void __fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size, bool shrank);
 extern void __fscache_invalidate(struct fscache_cookie *, const void *, uoff_t, unsigned int);
 extern int __fscache_begin_read_operation(struct netfs_cache_resources *, struct fscache_cookie *);
 extern int __fscache_begin_write_operation(struct netfs_cache_resources *, struct fscache_cookie *);
@@ -394,8 +394,9 @@ void fscache_update_cookie(struct fscache_cookie *cookie, const void *aux_data,
 
 /**
  * fscache_resize_cookie - Request that a cache object be resized
- * @cookie: The cookie representing the cache object
- * @new_size: The new size of the object (may be NULL)
+ * @cookie: The cookie representing the cache object (may be NULL)
+ * @new_size: The new size of the object
+ * @shrank: True if the file shrank; false if it expanded
  *
  * Request that the size of an object be changed.
  *
@@ -403,10 +404,10 @@ void fscache_update_cookie(struct fscache_cookie *cookie, const void *aux_data,
  * description.
  */
 static inline
-void fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size)
+void fscache_resize_cookie(struct fscache_cookie *cookie, uoff_t new_size, bool shrank)
 {
 	if (fscache_cookie_enabled(cookie))
-		__fscache_resize_cookie(cookie, new_size);
+		__fscache_resize_cookie(cookie, new_size, shrank);
 }
 
 /**

Re: [v3] fscache, cachefiles: Fix resize handling

[Alex Markuze]

Hi David,

Reviewed this on your netfs-next-5 tree. The resize helper it changes is not in our testing branch.

For Ceph this is the right split. ceph_fill_file_size() stores the new i_size before __ceph_do_pending_vmtruncate() trims the page cache and the cookie, so the worker can no longer see that the file shrank. i_truncate_shrank records size < isize at the truncate_seq bump. Without that, a truncate that is still above the stale cache size takes the grow path and leaves the backing tail in place.

IMHO the same flag is right for the other filesystems. AFS, 9p, and cifs_resize_file_locked() still have the old size at the call, so shrank is a direct comparison. The grow paths pass false, collapse passes true, and cachefiles shortens only when the caller says the file shrank and the backing file is actually larger.

Reviewed-by: Alex Markuze <amarkuze@redhat.com>

-- 
Alex Markuze