[PATCH 0/3] afs: Fix directory size handling

[PATCH 0/3] afs: Fix directory size handling

[David Howells]

Hi Linus,

Could you apply these three fixes to AFS directory handling?

 (1) Make sure that afs_read_dir() sees any increase in file size if the
     file unexpectedly changed on the server (e.g. due to another client
     making a change).

 (2) Make afs_getattr() always return the server's dir file size, not the
     locally edited one, so that pagecache eviction doesn't cause the dir
     file size to change unexpectedly.

 (3) Prevent afs_read_dir() from getting into an endless loop if the server
     indicates that the directory file size is larger than expected.

The patches can be found here:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs-fixes

Thanks,
David

---
%(shortlog)s
%(diffstat)s

David Howells (1):
  afs: Fix getattr to report server i_size on dirs, not local size

Marc Dionne (2):
  afs: Fix updating of i_size with dv jump from server
  afs: Avoid endless loop if file is larger than expected

 fs/afs/dir.c   |  4 ++++
 fs/afs/inode.c | 10 +++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

[PATCH 1/3] afs: Fix updating of i_size with dv jump from server

[David Howells]

From: Marc Dionne <marc.dionne@auristor.com>

If the data version returned from the server is larger than expected,
the local data is invalidated, but we may still want to note the remote
file size.

Since we're setting change_size, we have to also set data_changed
for the i_size to get updated.

Fixes: 3f4aa9818163 ("afs: Fix EOF corruption")
Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index b1bdffd5e888..82edd3351734 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -230,6 +230,7 @@ static void afs_apply_status(struct afs_operation *op,
 			set_bit(AFS_VNODE_ZAP_DATA, &vnode->flags);
 		}
 		change_size = true;
+		data_changed = true;
 	} else if (vnode->status.type == AFS_FTYPE_DIR) {
 		/* Expected directory change is handled elsewhere so
 		 * that we can locally edit the directory and save on a

[PATCH 2/3] afs: Fix getattr to report server i_size on dirs, not local size

[David Howells]

Fix afs_getattr() to report the server's idea of the file size of a
directory rather than the local size.  The local size may differ as we edit
the local copy to avoid having to redownload it and we may end up with a
differently structured blob of a different size.

However, if the directory is discarded from the pagecache we then download
it again and the user may see the directory file size apparently change.

Fixes: 63a4681ff39c ("afs: Locally edit directory data for mkdir/create/unlink/...")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/inode.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 82edd3351734..866bab860a88 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -450,7 +450,7 @@ static void afs_get_inode_cache(struct afs_vnode *vnode)
 				    0 : FSCACHE_ADV_SINGLE_CHUNK,
 				    &key, sizeof(key),
 				    &aux, sizeof(aux),
-				    vnode->status.size));
+				    i_size_read(&vnode->netfs.inode)));
 #endif
 }
 
@@ -777,6 +777,13 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path *path,
 		if (test_bit(AFS_VNODE_SILLY_DELETED, &vnode->flags) &&
 		    stat->nlink > 0)
 			stat->nlink -= 1;
+
+		/* Lie about the size of directories.  We maintain a locally
+		 * edited copy and may make different allocation decisions on
+		 * it, but we need to give userspace the server's size.
+		 */
+		if (S_ISDIR(inode->i_mode))
+			stat->size = vnode->netfs.remote_i_size;
 	} while (need_seqretry(&vnode->cb_lock, seq));
 
 	done_seqretry(&vnode->cb_lock, seq);

[PATCH 3/3] afs: Avoid endless loop if file is larger than expected

[David Howells]

From: Marc Dionne <marc.dionne@auristor.com>

afs_read_dir fetches an amount of data that's based on what the inode
size is thought to be.  If the file on the server is larger than what
was fetched, the code rechecks i_size and retries.  If the local i_size
was not properly updated, this can lead to an endless loop of fetching
i_size from the server and noticing each time that the size is larger on
the server.

If it is known that the remote size is larger than i_size, bump up the
fetch size to that size.

Fixes: f3ddee8dc4e2 ("afs: Fix directory handling")
Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-afs@lists.infradead.org
---
 fs/afs/dir.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index f92b9e62d567..4dd97afa536c 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -275,6 +275,7 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key)
 	loff_t i_size;
 	int nr_pages, i;
 	int ret;
+	loff_t remote_size = 0;
 
 	_enter("");
 
@@ -289,6 +290,8 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key)
 
 expand:
 	i_size = i_size_read(&dvnode->netfs.inode);
+	if (i_size < remote_size)
+	    i_size = remote_size;
 	if (i_size < 2048) {
 		ret = afs_bad(dvnode, afs_file_error_dir_small);
 		goto error;
@@ -364,6 +367,7 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key)
 			 * buffer.
 			 */
 			up_write(&dvnode->validate_lock);
+			remote_size = req->file_size;
 			goto expand;
 		}
 

Re: [PATCH 0/3] afs: Fix directory size handling

[Linus Torvalds]

On Tue, May 2, 2023 at 9:35 AM David Howells <dhowells@redhat.com> wrote:
>
> The patches can be found here:
>
>         https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs-fixes

Ok, I started pulling this, and then I realized that this is the first
time in this release window that I've gotten a non-signed pull
request.

(Not that it was a proper pull request format anyway - the above is
just a gitweb pointer, not a real git branch pointer, but it was close
enough)

So can you please just make this a proper signed tag, and maybe we'll
have the first release ever when every single pull is done with
signage?

I know you can do this properly, since  I have several pull requests
from you that were signed, including for AFS.

                 Linus