Re: [PATCH 11/13] orangefs: lock inode during fsync

[martin@omnibond.com]

On Thu, May 25, 2017 at 11:58:57AM -0400, Jeff Layton wrote:
> On Mon, 2017-05-22 at 05:58 -0400, Martin Brandenburg wrote:
> > Signed-off-by: Martin Brandenburg <martin@omnibond.com>
> > ---
> >  fs/orangefs/file.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
> > index cd126dd..f8536a7 100644
> > --- a/fs/orangefs/file.c
> > +++ b/fs/orangefs/file.c
> > @@ -652,7 +652,9 @@ static int orangefs_fsync(struct file *file,
> >  	struct orangefs_kernel_op_s *new_op = NULL;
> >  
> >  	/* required call */
> > +	inode_lock(file_inode(file));
> >  	filemap_write_and_wait_range(file->f_mapping, start, end);
> > +	inode_unlock(file_inode(file));
> >  
> >  	new_op = op_alloc(ORANGEFS_VFS_OP_FSYNC);
> >  	if (!new_op)
> 
> Why? You're just writing back the cached file data here. There's no
> reason to lock the inode for that, AFAICS.
> 
> -- 
> Jeff Layton <jlayton@redhat.com>

Because FUSE does.  Now I see FUSE needs it for fuse_sync_writes.

	/*
	 * Wait for all pending writepages on the inode to finish.
	 *
	 * This is currently done by blocking further writes with FUSE_NOWRITE
	 * and waiting for all sent writes to complete.
	 *
	 * This must be called under i_mutex, otherwise the FUSE_NOWRITE usage
	 * could conflict with truncation.
	 */
	static void fuse_sync_writes(struct inode *inode)

I think OrangeFS is okay because writepage does not return until the
write has completed.  Does that sound right?

I'm not sure about the truncate conflict.  Truncates are sent to the
server immediately.  Can a pending write show past the end of the file
show up later?  I don't handle that at all.

Martin