Re: [PATCH 3/6] fuse: Allow parallel direct writes for O_DIRECT

[Hao Xu <hao.xu@linux.dev>]

On 8/30/23 00:11, Bernd Schubert wrote:
> Take a shared lock in fuse_cache_write_iter. This was already
> done for FOPEN_DIRECT_IO in
> 
> commit 153524053bbb ("fuse: allow non-extending parallel direct
> writes on the same file")
> 
> but so far missing for plain O_DIRECT. Server side needs
> to set FOPEN_PARALLEL_DIRECT_WRITES in order to signal that
> it supports parallel dio writes.
> 
> Cc: Hao Xu <howeyxu@tencent.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Dharmendra Singh <dsingh@ddn.com>
> Cc: linux-fsdevel@vger.kernel.org
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> ---
>   fs/fuse/file.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 6b8b9512c336..a6b99bc80fe7 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1314,6 +1314,10 @@ static bool fuse_dio_wr_exclusive_lock(struct kiocb *iocb, struct iov_iter *from
>   	struct file *file = iocb->ki_filp;
>   	struct fuse_file *ff = file->private_data;
>   
> +	/* this function is about direct IO only */
> +	if (!(iocb->ki_flags & IOCB_DIRECT))
> +		return false;

This means for buffered write in fuse_cache_write_iter(), we grab shared 
lock, looks not right.

> +
>   	/* server side has to advise that it supports parallel dio writes */
>   	if (!(ff->open_flags & FOPEN_PARALLEL_DIRECT_WRITES))
>   		return false;
> @@ -1337,6 +1341,7 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	struct inode *inode = mapping->host;
>   	ssize_t err;
>   	struct fuse_conn *fc = get_fuse_conn(inode);
> +	bool excl_lock = fuse_dio_wr_exclusive_lock(iocb, from);
>   
>   	if (fc->writeback_cache && !(iocb->ki_flags & IOCB_DIRECT)) {
>   		/* Update size (EOF optimization) and mode (SUID clearing) */
> @@ -1355,7 +1360,10 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   	}
>   
>   writethrough:
> -	inode_lock(inode);
> +	if (excl_lock)
> +		inode_lock(inode);
> +	else
> +		inode_lock_shared(inode);
>   
>   	err = generic_write_checks(iocb, from);
>   	if (err <= 0)
> @@ -1370,6 +1378,9 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   		goto out;
>   
>   	if (iocb->ki_flags & IOCB_DIRECT) {
> +		/* file extending writes will trigger i_size_write - exclusive
> +		 * lock is needed
> +		 */
>   		written = generic_file_direct_write(iocb, from);
>   		if (written < 0 || !iov_iter_count(from))
>   			goto out;
> @@ -1379,7 +1390,10 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   		written = fuse_perform_write(iocb, from);
>   	}
>   out:
> -	inode_unlock(inode);
> +	if (excl_lock)
> +		inode_unlock(inode);
> +	else
> +		inode_unlock_shared(inode);
>   	if (written > 0)
>   		written = generic_write_sync(iocb, written);
>