Sorry, missed that pair fuse_set_nowrite/fuse_release_writes can be done only under i_mutex.
IMHO it is only due to bad implementation. If fuse_set_nowrite would be done with separate count instead of adding negative bias, it would be possible. On Wed, Nov 30, 2016 at 3:47 PM, Alexey Kuznetsov <kuz...@virtuozzo.com> wrote: > 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
