Hello! I do not think you got it right.
i_mutex in fsync is not about some atomicity, it is about stopping data feed while fsync is executed to prevent livelock. I cannot tell anything about mtime update, it is just some voodoo magic for me. What's about fsync semantics, I see two different ways: A. 1. Remove useless write_inode_now. Its work is done by filemap_write_and_wait_range(), there is no need to repeat it under mutex. 2. move mutex_lock _after_ fuse_sync_writes(), which is essentially fuse continuation forfilemap_write_and_wait_range(). 3. i_mutex is preserved only around fsync call. B. 1. Remove write_inode_now as well. 2. Remove i_mutex _completely_. (No idea about mtime voodo though) 2. Replace fuse_sync_writes() with fuse_set_nowrite() and add release after call to FSYNC. Both prevent livelock. B is obviosly optimal. But A preserves historic fuse protocol semantics. F.e. I have no idea would user space survive truncate racing with fsync. pstorage should survice, though this path was never tested. On Wed, Nov 30, 2016 at 4:02 AM, Maxim Patlasov <mpatla...@virtuozzo.com> wrote: > fuse_fsync_common() does need i_mutex for fuse_sync_writes() and > fuse_flush_mtime(). But when those operations are done, it's actually > doesn't matter whether to hold the lock over fuse_request_send(FUSE_FSYNC) > or not: we ensured that all relevant data were already seen by > userspace fuse daemon, and so it will sync them (by handling FUSE_FSYNC) > anyway; if the user screws up by leaking new data updates in-between, it > is up to the user and doesn't violate fsync(2) semantics. > > https://jira.sw.ru/browse/PSBM-55919 > > Signed-off-by: Maxim Patlasov <mpatla...@virtuozzo.com> > --- > fs/fuse/file.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 464b2f5..559dfd9 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -697,6 +697,8 @@ int fuse_fsync_common(struct file *file, loff_t start, > loff_t end, > goto out; > } > > + mutex_unlock(&inode->i_mutex); > + > memset(&inarg, 0, sizeof(inarg)); > inarg.fh = ff->fh; > inarg.fsync_flags = datasync ? 1 : 0; > @@ -715,6 +717,7 @@ int fuse_fsync_common(struct file *file, loff_t start, > loff_t end, > fc->no_fsync = 1; > err = 0; > } > + return err; > out: > mutex_unlock(&inode->i_mutex); > return err; > _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel