[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 <[hidden email]>
Signed-off-by: Ashish Samant <[hidden email]>
---
 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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

[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 <[hidden email]>
> Signed-off-by: Ashish Samant <[hidden email]>
> ---
>   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,           \
>   }
>  
>   /**

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

[Miklos Szeredi]

On Mon, Jul 10, 2017 at 11:36 PM, Ashish Samant
<[hidden email]> 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

[Ashish Samant]

On 07/12/2017 05:21 AM, Miklos Szeredi wrote:

> On Mon, Jul 10, 2017 at 11:36 PM, Ashish Samant
> <[hidden email]> 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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel