[PATCH v2 0/2] Fix regression in libfuse test_copy_file

linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range()
@ 2024-10-14 19:27 Amir Goldstein
  2024-10-14 19:27 ` [PATCH v2 1/2] fs: pass offset and result to backing_file end_write() callback Amir Goldstein
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Amir Goldstein @ 2024-10-14 19:27 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Bernd Schubert, yangyun, linux-fsdevel

Miklos,

I figured it was best to split the backing_file interface change from
the fix, but both changes should be targetting stable.

Thanks,
Amir.

Changes since v1:
- Change end_write() callback arguments
- Use pos argument to extend i_size instead of using backing inode size

Amir Goldstein (2):
  fs: pass offset and result to backing_file end_write() callback
  fuse: update inode size after extending passthrough write

 fs/backing-file.c            | 8 ++++----
 fs/fuse/passthrough.c        | 8 ++++----
 fs/overlayfs/file.c          | 9 +++++++--
 include/linux/backing-file.h | 2 +-
 4 files changed, 16 insertions(+), 11 deletions(-)

-- 
2.34.1


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

* [PATCH v2 1/2] fs: pass offset and result to backing_file end_write() callback
  2024-10-14 19:27 [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Amir Goldstein
@ 2024-10-14 19:27 ` Amir Goldstein
  2024-10-14 19:27 ` [PATCH v2 2/2] fuse: update inode size after extending passthrough write Amir Goldstein
  2024-10-15 13:26 ` [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Miklos Szeredi
  2 siblings, 0 replies; 4+ messages in thread
From: Amir Goldstein @ 2024-10-14 19:27 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Bernd Schubert, yangyun, linux-fsdevel

This is needed for extending fuse inode size after fuse passthrough write.

Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
Link: https://lore.kernel.org/linux-fsdevel/CAJfpegs=cvZ_NYy6Q_D42XhYS=Sjj5poM1b5TzXzOVvX=R36aA@mail.gmail.com/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/backing-file.c            | 8 ++++----
 fs/fuse/passthrough.c        | 6 +++---
 fs/overlayfs/file.c          | 9 +++++++--
 include/linux/backing-file.h | 2 +-
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/backing-file.c b/fs/backing-file.c
index 8860dac58c37..09a9be945d45 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -80,7 +80,7 @@ struct backing_aio {
 	refcount_t ref;
 	struct kiocb *orig_iocb;
 	/* used for aio completion */
-	void (*end_write)(struct file *);
+	void (*end_write)(struct file *, loff_t, ssize_t);
 	struct work_struct work;
 	long res;
 };
@@ -109,7 +109,7 @@ static void backing_aio_cleanup(struct backing_aio *aio, long res)
 	struct kiocb *orig_iocb = aio->orig_iocb;
 
 	if (aio->end_write)
-		aio->end_write(orig_iocb->ki_filp);
+		aio->end_write(orig_iocb->ki_filp, iocb->ki_pos, res);
 
 	orig_iocb->ki_pos = iocb->ki_pos;
 	backing_aio_put(aio);
@@ -239,7 +239,7 @@ ssize_t backing_file_write_iter(struct file *file, struct iov_iter *iter,
 
 		ret = vfs_iter_write(file, iter, &iocb->ki_pos, rwf);
 		if (ctx->end_write)
-			ctx->end_write(ctx->user_file);
+			ctx->end_write(ctx->user_file, iocb->ki_pos, ret);
 	} else {
 		struct backing_aio *aio;
 
@@ -317,7 +317,7 @@ ssize_t backing_file_splice_write(struct pipe_inode_info *pipe,
 	revert_creds(old_cred);
 
 	if (ctx->end_write)
-		ctx->end_write(ctx->user_file);
+		ctx->end_write(ctx->user_file, ppos ? *ppos : 0, ret);
 
 	return ret;
 }
diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index ba3207f6c4ce..c80b9712eff7 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -18,7 +18,7 @@ static void fuse_file_accessed(struct file *file)
 	fuse_invalidate_atime(inode);
 }
 
-static void fuse_file_modified(struct file *file)
+static void fuse_passthrough_end_write(struct file *file, loff_t, ssize_t)
 {
 	struct inode *inode = file_inode(file);
 
@@ -63,7 +63,7 @@ ssize_t fuse_passthrough_write_iter(struct kiocb *iocb,
 	struct backing_file_ctx ctx = {
 		.cred = ff->cred,
 		.user_file = file,
-		.end_write = fuse_file_modified,
+		.end_write = fuse_passthrough_end_write,
 	};
 
 	pr_debug("%s: backing_file=0x%p, pos=%lld, len=%zu\n", __func__,
@@ -110,7 +110,7 @@ ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe,
 	struct backing_file_ctx ctx = {
 		.cred = ff->cred,
 		.user_file = out,
-		.end_write = fuse_file_modified,
+		.end_write = fuse_passthrough_end_write,
 	};
 
 	pr_debug("%s: backing_file=0x%p, pos=%lld, len=%zu, flags=0x%x\n", __func__,
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 4504493b20be..24a36d61bb0c 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -231,6 +231,11 @@ static void ovl_file_modified(struct file *file)
 	ovl_copyattr(file_inode(file));
 }
 
+static void ovl_file_end_write(struct file *file, loff_t, ssize_t)
+{
+	ovl_file_modified(file);
+}
+
 static void ovl_file_accessed(struct file *file)
 {
 	struct inode *inode, *upperinode;
@@ -294,7 +299,7 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter)
 	struct backing_file_ctx ctx = {
 		.cred = ovl_creds(inode->i_sb),
 		.user_file = file,
-		.end_write = ovl_file_modified,
+		.end_write = ovl_file_end_write,
 	};
 
 	if (!iov_iter_count(iter))
@@ -364,7 +369,7 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out,
 	struct backing_file_ctx ctx = {
 		.cred = ovl_creds(inode->i_sb),
 		.user_file = out,
-		.end_write = ovl_file_modified,
+		.end_write = ovl_file_end_write,
 	};
 
 	inode_lock(inode);
diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h
index 4b61b0e57720..2eed0ffb5e8f 100644
--- a/include/linux/backing-file.h
+++ b/include/linux/backing-file.h
@@ -16,7 +16,7 @@ struct backing_file_ctx {
 	const struct cred *cred;
 	struct file *user_file;
 	void (*accessed)(struct file *);
-	void (*end_write)(struct file *);
+	void (*end_write)(struct file *, loff_t, ssize_t);
 };
 
 struct file *backing_file_open(const struct path *user_path, int flags,
-- 
2.34.1


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

* [PATCH v2 2/2] fuse: update inode size after extending passthrough write
  2024-10-14 19:27 [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Amir Goldstein
  2024-10-14 19:27 ` [PATCH v2 1/2] fs: pass offset and result to backing_file end_write() callback Amir Goldstein
@ 2024-10-14 19:27 ` Amir Goldstein
  2024-10-15 13:26 ` [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Miklos Szeredi
  2 siblings, 0 replies; 4+ messages in thread
From: Amir Goldstein @ 2024-10-14 19:27 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Bernd Schubert, yangyun, linux-fsdevel

yangyun reported that libfuse test test_copy_file_range() copies zero
bytes from a newly written file when fuse passthrough is enabled.

The reason is that extending passthrough write is not updating the fuse
inode size and when vfs_copy_file_range() observes a zero size inode,
it returns without calling the filesystem copy_file_range() method.

Fix this by adjusting the fuse inode size after an extending passthrough
write.

This does not provide cache coherency of fuse inode attributes and
backing inode attributes, but it should prevent situations where fuse
inode size is too small, causing read/copy to be wrongly shortened.

Reported-by: yangyun <yangyun50@huawei.com>
Closes: https://github.com/libfuse/libfuse/issues/1048
Fixes: 57e1176e6086 ("fuse: implement read/write passthrough")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/fuse/passthrough.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index c80b9712eff7..bbac547dfcb3 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -18,11 +18,11 @@ static void fuse_file_accessed(struct file *file)
 	fuse_invalidate_atime(inode);
 }
 
-static void fuse_passthrough_end_write(struct file *file, loff_t, ssize_t)
+static void fuse_passthrough_end_write(struct file *file, loff_t pos, ssize_t ret)
 {
 	struct inode *inode = file_inode(file);
 
-	fuse_invalidate_attr_mask(inode, FUSE_STATX_MODSIZE);
+	fuse_write_update_attr(inode, pos, ret);
 }
 
 ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter)
-- 
2.34.1


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

* Re: [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range()
  2024-10-14 19:27 [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Amir Goldstein
  2024-10-14 19:27 ` [PATCH v2 1/2] fs: pass offset and result to backing_file end_write() callback Amir Goldstein
  2024-10-14 19:27 ` [PATCH v2 2/2] fuse: update inode size after extending passthrough write Amir Goldstein
@ 2024-10-15 13:26 ` Miklos Szeredi
  2 siblings, 0 replies; 4+ messages in thread
From: Miklos Szeredi @ 2024-10-15 13:26 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Bernd Schubert, yangyun, linux-fsdevel

On Mon, 14 Oct 2024 at 21:28, Amir Goldstein <amir73il@gmail.com> wrote:
>
> Miklos,
>
> I figured it was best to split the backing_file interface change from
> the fix, but both changes should be targetting stable.

Looks good.  Applied and pushed.

Thanks,
Miklos

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

end of thread, other threads:[~2024-10-15 13:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-14 19:27 [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Amir Goldstein
2024-10-14 19:27 ` [PATCH v2 1/2] fs: pass offset and result to backing_file end_write() callback Amir Goldstein
2024-10-14 19:27 ` [PATCH v2 2/2] fuse: update inode size after extending passthrough write Amir Goldstein
2024-10-15 13:26 ` [PATCH v2 0/2] Fix regression in libfuse test_copy_file_range() Miklos Szeredi

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).