[PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio

[PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio

[Ashish Samant]

'Commit 8fba54aebbdf ("fuse: direct-io: don't dirty ITER_BVEC pages")'
fixes the ITER_BVEC page deadlock for direct io in fuse but misses the
async_dio case.

Fix this by making should_dirty a fuse_io_priv parameter.

Reported-by: Tiger Yang <tiger.yang@oracle.com>
Signed-off-by: Ashish Samant <ashish.samant@oracle.com>
---
 fs/fuse/file.c   | 8 +++++---
 fs/fuse/fuse_i.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc..e865ac7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -608,8 +608,9 @@ static void fuse_aio_complete_req(struct fuse_conn *fc, struct fuse_req *req)
 {
 	struct fuse_io_priv *io = req->io;
 	ssize_t pos = -1;
+	bool should_dirty = io->should_dirty && !io->write;
 
-	fuse_release_user_pages(req, !io->write);
+	fuse_release_user_pages(req, should_dirty);
 
 	if (io->write) {
 		if (req->misc.write.in.size != req->misc.write.out.size)
@@ -1316,7 +1317,6 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
 		       loff_t *ppos, int flags)
 {
 	int write = flags & FUSE_DIO_WRITE;
-	bool should_dirty = !write && iter_is_iovec(iter);
 	int cuse = flags & FUSE_DIO_CUSE;
 	struct file *file = io->file;
 	struct inode *inode = file->f_mapping->host;
@@ -1346,6 +1346,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
 			inode_unlock(inode);
 	}
 
+	io->should_dirty = !write && iter_is_iovec(iter);
 	while (count) {
 		size_t nres;
 		fl_owner_t owner = current->files;
@@ -1360,7 +1361,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
 			nres = fuse_send_read(req, io, pos, nbytes, owner);
 
 		if (!io->async)
-			fuse_release_user_pages(req, should_dirty);
+			fuse_release_user_pages(req, io->should_dirty);
 		if (req->out.h.error) {
 			err = req->out.h.error;
 			break;
@@ -2872,6 +2873,7 @@ static inline loff_t fuse_round_up(loff_t off)
 	io->size = 0;
 	io->offset = offset;
 	io->write = (iov_iter_rw(iter) == WRITE);
+	io->should_dirty = true;
 	io->err = 0;
 	io->file = file;
 	/*
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 1bd7ffd..2ef205e 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -249,6 +249,7 @@ struct fuse_io_priv {
 	size_t size;
 	__u64 offset;
 	bool write;
+	bool should_dirty;
 	int err;
 	struct kiocb *iocb;
 	struct file *file;
@@ -261,6 +262,7 @@ struct fuse_io_priv {
 	.refcnt = KREF_INIT(1),		\
 	.async = 0,			\
 	.file = f,			\
+	.should_dirty = true,           \
 }
 
 /**
-- 
1.9.1

Re: [PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio

[Ashish Samant]

Hi,

Any update here? This attempts to fix a real deadlock which happens as 
soon as a loop device is created in a fuse mount point with the 
async_dio mount option enabled.

Thanks,
Ashish

On 06/22/2017 05:33 PM, Ashish Samant wrote:
> 'Commit 8fba54aebbdf ("fuse: direct-io: don't dirty ITER_BVEC pages")'
> fixes the ITER_BVEC page deadlock for direct io in fuse but misses the
> async_dio case.
>
> Fix this by making should_dirty a fuse_io_priv parameter.
>
> Reported-by: Tiger Yang <tiger.yang@oracle.com>
> Signed-off-by: Ashish Samant <ashish.samant@oracle.com>
> ---
>   fs/fuse/file.c   | 8 +++++---
>   fs/fuse/fuse_i.h | 2 ++
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 3ee4fdc..e865ac7 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -608,8 +608,9 @@ static void fuse_aio_complete_req(struct fuse_conn *fc, struct fuse_req *req)
>   {
>   	struct fuse_io_priv *io = req->io;
>   	ssize_t pos = -1;
> +	bool should_dirty = io->should_dirty && !io->write;
>   
> -	fuse_release_user_pages(req, !io->write);
> +	fuse_release_user_pages(req, should_dirty);
>   
>   	if (io->write) {
>   		if (req->misc.write.in.size != req->misc.write.out.size)
> @@ -1316,7 +1317,6 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
>   		       loff_t *ppos, int flags)
>   {
>   	int write = flags & FUSE_DIO_WRITE;
> -	bool should_dirty = !write && iter_is_iovec(iter);
>   	int cuse = flags & FUSE_DIO_CUSE;
>   	struct file *file = io->file;
>   	struct inode *inode = file->f_mapping->host;
> @@ -1346,6 +1346,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
>   			inode_unlock(inode);
>   	}
>   
> +	io->should_dirty = !write && iter_is_iovec(iter);
>   	while (count) {
>   		size_t nres;
>   		fl_owner_t owner = current->files;
> @@ -1360,7 +1361,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
>   			nres = fuse_send_read(req, io, pos, nbytes, owner);
>   
>   		if (!io->async)
> -			fuse_release_user_pages(req, should_dirty);
> +			fuse_release_user_pages(req, io->should_dirty);
>   		if (req->out.h.error) {
>   			err = req->out.h.error;
>   			break;
> @@ -2872,6 +2873,7 @@ static inline loff_t fuse_round_up(loff_t off)
>   	io->size = 0;
>   	io->offset = offset;
>   	io->write = (iov_iter_rw(iter) == WRITE);
> +	io->should_dirty = true;
>   	io->err = 0;
>   	io->file = file;
>   	/*
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 1bd7ffd..2ef205e 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -249,6 +249,7 @@ struct fuse_io_priv {
>   	size_t size;
>   	__u64 offset;
>   	bool write;
> +	bool should_dirty;
>   	int err;
>   	struct kiocb *iocb;
>   	struct file *file;
> @@ -261,6 +262,7 @@ struct fuse_io_priv {
>   	.refcnt = KREF_INIT(1),		\
>   	.async = 0,			\
>   	.file = f,			\
> +	.should_dirty = true,           \
>   }
>   
>   /**

Re: [PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio

[Miklos Szeredi]

On Mon, Jul 10, 2017 at 11:36 PM, Ashish Samant
<ashish.samant@oracle.com> wrote:
> Hi,
>
> Any update here? This attempts to fix a real deadlock which happens as soon
> as a loop device is created in a fuse mount point with the async_dio mount
> option enabled.

The patch header should contain the explanation of this deadlock.

Can you please resend the patch with a more detailed description?

Thanks,
Miklos

Re: [PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio

[Ashish Samant]

On 07/12/2017 05:21 AM, Miklos Szeredi wrote:
> On Mon, Jul 10, 2017 at 11:36 PM, Ashish Samant
> <ashish.samant@oracle.com> wrote:
>> Hi,
>>
>> Any update here? This attempts to fix a real deadlock which happens as soon
>> as a loop device is created in a fuse mount point with the async_dio mount
>> option enabled.
> The patch header should contain the explanation of this deadlock.
>
> Can you please resend the patch with a more detailed description?

Sent V2 with detailed commit message.

Thanks,
Ashish
>
> Thanks,
> Miklos