Discussion:
[PATCH] fuse: Dont set_page_dirty_lock() for ITER_BVEC pages for async_dio
(too old to reply)
Ashish Samant
2017-06-23 00:33:08 UTC
Permalink
'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 <***@oracle.com>
Signed-off-by: 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
Ashish Samant
2017-07-10 21:36:16 UTC
Permalink
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
Post by 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.
---
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, \
}
/**
Miklos Szeredi
2017-07-12 12:21:37 UTC
Permalink
On Mon, Jul 10, 2017 at 11:36 PM, 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.
The patch header should contain the explanation of this deadlock.

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

Thanks,
Miklos
Ashish Samant
2017-07-13 02:29:01 UTC
Permalink
Post by Miklos Szeredi
On Mon, Jul 10, 2017 at 11:36 PM, 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.
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
Post by Miklos Szeredi
Thanks,
Miklos
Loading...