On Mon, Oct 14, 2024 at 8:30 AM yangyun <yangyun50@xxxxxxxxxx> wrote: > > On Fri, Oct 11, 2024 at 03:53:26PM +0200, Amir Goldstein wrote: > > 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. > > > > Extend the fuse inode size to the size of the backing inode after every > > passthrough write if the backing inode size is larger. > > > > This does not yet 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@xxxxxxxxxx> > > Closes: https://github.com/libfuse/libfuse/issues/1048 > > Fixes: 57e1176e6086 ("fuse: implement read/write passthrough") > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > --- > > > > Doh! force of habbit - fixed subject s/ovl/fuse > > > > Thanks, > > Amir. > > > > fs/fuse/passthrough.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c > > index ba3207f6c4ce..d3047a4bc40e 100644 > > --- a/fs/fuse/passthrough.c > > +++ b/fs/fuse/passthrough.c > > @@ -20,9 +20,18 @@ static void fuse_file_accessed(struct file *file) > > > > static void fuse_file_modified(struct file *file) > > { > > + struct fuse_file *ff = file->private_data; > > + struct file *backing_file = fuse_file_passthrough(ff); > > struct inode *inode = file_inode(file); > > - > > - fuse_invalidate_attr_mask(inode, FUSE_STATX_MODSIZE); > > + loff_t size = i_size_read(file_inode(backing_file)); > > + > > + /* > > + * Most of the time we will be holding inode_lock(), but even if we are > > + * called from async io completion without inode_lock(), the last write > > + * will update fuse inode size to the size of the backing inode, even if > > + * the last write was not the extending write. > > + */ > > + fuse_write_update_attr(inode, size, size); > > } > > > > ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter) > > -- > > 2.34.1 > > Sorry for a late reply. Because I have spent some time on figuring out whether we > need some FUSE_I_SIZE_UNSTABLE bit operations before and after the write which are > provided in your v14 patch of fuse passthrough, just like in fuse_perform_write. > > The conclusion is not, IMO. The FUSE_I_SIZE_UNSTABLE bit is provided in > commit 06a7c3c2781(fuse: hotfix truncate_pagecache() issue) for the > races between i_size updates and truncate_pagecache() in > fuse_change_attributes. Because the pagecache operations of fuse inode > is not allowed in fuse passthrough mode, this FUSE_I_SIZE_UNSTABLE bit is useless. > And we just need the minimum fix for extending writes by now. > That was my conclusion as well. In general, too large i_size is less of a problem IMO. Shortening i_size on short read/truncate is an optimization. > I have also tested this patch with xfstests (using ./check -fuse -b) and libfuse > test. In xfstest, this patch does not import new failed tests compared with pre-this > patch. And in libfuse test, this patch can solve the problem test_copy_file_range(). Thanks for testing! Amir.
