[PATCH] fs/fuse: Call generic_write_sync after fuse write

[PATCH] fs/fuse: Call generic_write_sync after fuse write

[nate]

From: Vitaly Zolotusky <vitaly@unitc.com>

If the IOCB_DSYNC flag is set a sync is not being performed by
fuse_file_write_iter. Honor IOCB_DSYNC by invoking generic_write_sync
when a write has been successful. generic_write_sync appropriatly syncs
the data which was written when IOCB_DSYNC flag is set.

Signed-off-by: Vitaly Zolotusky <vitaly@unitc.com>
Signed-off-by: Nate Clark <nate@neworld.us>
---
 fs/fuse/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 3ee4fdc..5e0a39e 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1227,6 +1227,8 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 out:
 	current->backing_dev_info = NULL;
 	inode_unlock(inode);
+	if (written > 0)
+		written = generic_write_sync(iocb, written);
 
 	return written ? written : err;
 }
-- 
2.9.4

Re: [fuse-devel] [PATCH] fs/fuse: Call generic_write_sync after fuse write

[Miklos Szeredi]

On Mon, Aug 07, 2017 at 11:46:45AM -0400, nate@neworld.us wrote:
> From: Vitaly Zolotusky <vitaly@unitc.com>
> 
> If the IOCB_DSYNC flag is set a sync is not being performed by
> fuse_file_write_iter. Honor IOCB_DSYNC by invoking generic_write_sync
> when a write has been successful. generic_write_sync appropriatly syncs
> the data which was written when IOCB_DSYNC flag is set.

I fear that sending a separate FSYNC request after each write might be a
performance regression for some use cases.  Also we already have a way to
communicate dsync/sync to the filesystem on writes: the flags field of
fuse_write_in.  Following patch updates the flags sent along with the WRITE
request to correctly reflect the sync state in the kiocb.

Please test.

Thanks,
Miklos
---

From: Miklos Szeredi <mszeredi@redhat.com>
Subject: fuse: honor iocb sync flags on write

If the IOCB_DSYNC flag is set a sync is not being performed by
fuse_file_write_iter.

Honor IOCB_DSYNC/IOCB_SYNC by setting O_DYSNC/O_SYNC respectively in the
flags filed of the write request.

We don't need to sync data or metadata, since fuse_perform_write() does
write-through and the filesystem is responsible for updating file times.

Original patch by Vitaly Zolotusky.

Reported-by: Nate Clark <nate@neworld.us>
Cc: Vitaly Zolotusky <vitaly@unitc.com>.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/cuse.c   |    4 ++--
 fs/fuse/file.c   |   41 ++++++++++++++++++++++++-----------------
 fs/fuse/fuse_i.h |    5 ++---
 3 files changed, 28 insertions(+), 22 deletions(-)

--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -645,7 +645,7 @@ static size_t fuse_async_req_send(struct
 static size_t fuse_send_read(struct fuse_req *req, struct fuse_io_priv *io,
 			     loff_t pos, size_t count, fl_owner_t owner)
 {
-	struct file *file = io->file;
+	struct file *file = io->iocb->ki_filp;
 	struct fuse_file *ff = file->private_data;
 	struct fuse_conn *fc = ff->fc;
 
@@ -707,7 +707,8 @@ static void fuse_short_read(struct fuse_
 
 static int fuse_do_readpage(struct file *file, struct page *page)
 {
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(file);
+	struct kiocb iocb;
+	struct fuse_io_priv io;
 	struct inode *inode = page->mapping->host;
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	struct fuse_req *req;
@@ -735,6 +736,8 @@ static int fuse_do_readpage(struct file
 	req->num_pages = 1;
 	req->pages[0] = page;
 	req->page_descs[0].length = count;
+	init_sync_kiocb(&iocb, file);
+	io = (struct fuse_io_priv) FUSE_IO_PRIV_SYNC(&iocb);
 	num_read = fuse_send_read(req, &io, pos, count, NULL);
 	err = req->out.h.error;
 
@@ -957,13 +960,18 @@ static void fuse_write_fill(struct fuse_
 static size_t fuse_send_write(struct fuse_req *req, struct fuse_io_priv *io,
 			      loff_t pos, size_t count, fl_owner_t owner)
 {
-	struct file *file = io->file;
+	struct kiocb *iocb = io->iocb;
+	struct file *file = iocb->ki_filp;
 	struct fuse_file *ff = file->private_data;
 	struct fuse_conn *fc = ff->fc;
 	struct fuse_write_in *inarg = &req->misc.write.in;
 
 	fuse_write_fill(req, ff, pos, count);
 	inarg->flags = file->f_flags;
+	if (iocb->ki_flags & IOCB_DSYNC)
+		inarg->flags |= O_DSYNC;
+	if (iocb->ki_flags & IOCB_SYNC)
+		inarg->flags |= O_SYNC;
 	if (owner != NULL) {
 		inarg->write_flags |= FUSE_WRITE_LOCKOWNER;
 		inarg->lock_owner = fuse_lock_owner_id(fc, owner);
@@ -993,14 +1001,14 @@ bool fuse_write_update_size(struct inode
 	return ret;
 }
 
-static size_t fuse_send_write_pages(struct fuse_req *req, struct file *file,
+static size_t fuse_send_write_pages(struct fuse_req *req, struct kiocb *iocb,
 				    struct inode *inode, loff_t pos,
 				    size_t count)
 {
 	size_t res;
 	unsigned offset;
 	unsigned i;
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(file);
+	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(iocb);
 
 	for (i = 0; i < req->num_pages; i++)
 		fuse_wait_on_page_writeback(inode, req->pages[i]->index);
@@ -1100,7 +1108,7 @@ static inline unsigned fuse_wr_pages(lof
 		     FUSE_MAX_PAGES_PER_REQ);
 }
 
-static ssize_t fuse_perform_write(struct file *file,
+static ssize_t fuse_perform_write(struct kiocb *iocb,
 				  struct address_space *mapping,
 				  struct iov_iter *ii, loff_t pos)
 {
@@ -1133,7 +1141,7 @@ static ssize_t fuse_perform_write(struct
 		} else {
 			size_t num_written;
 
-			num_written = fuse_send_write_pages(req, file, inode,
+			num_written = fuse_send_write_pages(req, iocb, inode,
 							    pos, count);
 			err = req->out.h.error;
 			if (!err) {
@@ -1201,7 +1209,7 @@ static ssize_t fuse_file_write_iter(stru
 
 		pos += written;
 
-		written_buffered = fuse_perform_write(file, mapping, from, pos);
+		written_buffered = fuse_perform_write(iocb, mapping, from, pos);
 		if (written_buffered < 0) {
 			err = written_buffered;
 			goto out;
@@ -1220,13 +1228,15 @@ static ssize_t fuse_file_write_iter(stru
 		written += written_buffered;
 		iocb->ki_pos = pos + written_buffered;
 	} else {
-		written = fuse_perform_write(file, mapping, from, iocb->ki_pos);
+		written = fuse_perform_write(iocb, mapping, from, iocb->ki_pos);
 		if (written >= 0)
 			iocb->ki_pos += written;
 	}
 out:
 	current->backing_dev_info = NULL;
 	inode_unlock(inode);
+	if (written > 0)
+		written = generic_write_sync(iocb, written);
 
 	return written ? written : err;
 }
@@ -1317,7 +1327,7 @@ ssize_t fuse_direct_io(struct fuse_io_pr
 {
 	int write = flags & FUSE_DIO_WRITE;
 	int cuse = flags & FUSE_DIO_CUSE;
-	struct file *file = io->file;
+	struct file *file = io->iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
 	struct fuse_file *ff = file->private_data;
 	struct fuse_conn *fc = ff->fc;
@@ -1399,8 +1409,7 @@ static ssize_t __fuse_direct_read(struct
 				  loff_t *ppos)
 {
 	ssize_t res;
-	struct file *file = io->file;
-	struct inode *inode = file_inode(file);
+	struct inode *inode = file_inode(io->iocb->ki_filp);
 
 	if (is_bad_inode(inode))
 		return -EIO;
@@ -1414,15 +1423,14 @@ static ssize_t __fuse_direct_read(struct
 
 static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(iocb->ki_filp);
+	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(iocb);
 	return __fuse_direct_read(&io, to, &iocb->ki_pos);
 }
 
 static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
 {
-	struct file *file = iocb->ki_filp;
-	struct inode *inode = file_inode(file);
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(file);
+	struct inode *inode = file_inode(iocb->ki_filp);
+	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(iocb);
 	ssize_t res;
 
 	if (is_bad_inode(inode))
@@ -2871,7 +2879,6 @@ fuse_direct_IO(struct kiocb *iocb, struc
 	io->offset = offset;
 	io->write = (iov_iter_rw(iter) == WRITE);
 	io->err = 0;
-	io->file = file;
 	/*
 	 * By default, we want to optimize all I/Os with async request
 	 * submission to the client filesystem if supported.
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -90,7 +90,7 @@ static struct list_head *cuse_conntbl_he
 
 static ssize_t cuse_read_iter(struct kiocb *kiocb, struct iov_iter *to)
 {
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(kiocb->ki_filp);
+	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(kiocb);
 	loff_t pos = 0;
 
 	return fuse_direct_io(&io, to, &pos, FUSE_DIO_CUSE);
@@ -98,7 +98,7 @@ static ssize_t cuse_read_iter(struct kio
 
 static ssize_t cuse_write_iter(struct kiocb *kiocb, struct iov_iter *from)
 {
-	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(kiocb->ki_filp);
+	struct fuse_io_priv io = FUSE_IO_PRIV_SYNC(kiocb);
 	loff_t pos = 0;
 	/*
 	 * No locking or generic_write_checks(), the server is
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -252,16 +252,15 @@ struct fuse_io_priv {
 	bool should_dirty;
 	int err;
 	struct kiocb *iocb;
-	struct file *file;
 	struct completion *done;
 	bool blocking;
 };
 
-#define FUSE_IO_PRIV_SYNC(f) \
+#define FUSE_IO_PRIV_SYNC(i) \
 {					\
 	.refcnt = KREF_INIT(1),		\
 	.async = 0,			\
-	.file = f,			\
+	.iocb = i,			\
 }
 
 /**

Re: [PATCH] fs/fuse: Call generic_write_sync after fuse write

[Nate Clark]

Hello,

Does this patch make sense?

On Mon, Aug 7, 2017 at 11:46 AM,  <nate@neworld.us> wrote:
> From: Vitaly Zolotusky <vitaly@unitc.com>
>
> If the IOCB_DSYNC flag is set a sync is not being performed by
> fuse_file_write_iter. Honor IOCB_DSYNC by invoking generic_write_sync
> when a write has been successful. generic_write_sync appropriatly syncs
> the data which was written when IOCB_DSYNC flag is set.
>
> Signed-off-by: Vitaly Zolotusky <vitaly@unitc.com>
> Signed-off-by: Nate Clark <nate@neworld.us>
> ---
>  fs/fuse/file.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 3ee4fdc..5e0a39e 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1227,6 +1227,8 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  out:
>         current->backing_dev_info = NULL;
>         inode_unlock(inode);
> +       if (written > 0)
> +               written = generic_write_sync(iocb, written);
>
>         return written ? written : err;
>  }
> --
> 2.9.4
>

-nate

Re: [PATCH] fs/fuse: Call generic_write_sync after fuse write

[Nate Clark]

On Mon, Oct 9, 2017 at 6:44 AM, Nate Clark <nate@neworld.us> wrote:
> Hello,
>
> Does this patch make sense?
>

Sorry I just found Miklos' response in the archive. For some reason I
never got the response delivered to my email.

-nate