Skip to contentSkip to conversation

fuse: O_DIRECT support for files

PATCH Superseded Anand Avati 2 participants 2 messages Jan 19, 2012 – Jan 23, 2012

Subsystems (2)

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 fuse: O_DIRECT support for files

Implement ->direct_IO() method in aops. The ->direct_IO() method combines the existing fuse_direct_read/fuse_direct_write with a generic fuse_loop_rwv (inspired by do_loop_readv/writev) to neatly re-use the existing direct_io framework.

Reaching ->direct_IO() in the read path via generic_file_aio_read ensures proper synchronization with page cache with its existing framework.

Reaching ->direct_IO() in the write path via fuse_file_aio_write is made to come via generic_file_direct_write() which makes it play nice with the page cache w.r.t other mmap pages etc.

On files marked 'direct_io' by the filesystem server, IO always follows the fuse_direct_read/write path. There is no effect of fcntl(O_DIRECT) and it always succeeds.

On files not marked with 'direct_io' by the filesystem server, the IO path depends on O_DIRECT flag by the application. This can be passed at the time of open() as well as via fcntl().

Note that asynchronous O_DIRECT iocb jobs are completed synchronously always (this has been the case with FUSE even before this patch)

---
 fs/fuse/dir.c  |    3 --
 fs/fuse/file.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 8 deletions(-)
fs/fuse/dir.c +0 -3
index 2066328..7e5dbd0 100644
@@ -387,9 +387,6 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	if (fc->no_create)
 		return -ENOSYS;
 
-	if (flags & O_DIRECT)
-		return -EINVAL;
-
 	forget = fuse_alloc_forget();
 	if (!forget)
 		return -ENOMEM;
fs/fuse/file.c +68 -5

112 lines not on this page · raw message

Signature · 1 line
1.7.6.4
maintainer
#1 lore raw
Quoted 16 lines · Anand Avati

Implement ->direct_IO() method in aops. The ->direct_IO() method combines the existing fuse_direct_read/fuse_direct_write with a generic fuse_loop_rwv (inspired by do_loop_readv/writev) to neatly re-use the existing direct_io framework.

Reaching ->direct_IO() in the read path via generic_file_aio_read ensures proper synchronization with page cache with its existing framework.

Reaching ->direct_IO() in the write path via fuse_file_aio_write is made to come via generic_file_direct_write() which makes it play nice with the page cache w.r.t other mmap pages etc.

On files marked 'direct_io' by the filesystem server, IO always follows the fuse_direct_read/write path. There is no effect of fcntl(O_DIRECT) and it always succeeds.

Quoted 6 lines

On files not marked with 'direct_io' by the filesystem server, the IO path depends on O_DIRECT flag by the application. This can be passed at the time of open() as well as via fcntl().

Note that asynchronous O_DIRECT iocb jobs are completed synchronously always (this has been the case with FUSE even before this patch)

I'm generally fine with this approach, but see comments inline.

Quoted 60 lines
---
 fs/fuse/dir.c  |    3 --
 fs/fuse/file.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 8 deletions(-)
fs/fuse/dir.c +0 -3
index 2066328..7e5dbd0 100644
@@ -387,9 +387,6 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	if (fc->no_create)
 		return -ENOSYS;
 
-	if (flags & O_DIRECT)
-		return -EINVAL;
-
 	forget = fuse_alloc_forget();
 	if (!forget)
 		return -ENOMEM;
fs/fuse/file.c +5 -5
index 4a199fd..3c6b8bc 100644
@@ -194,10 +194,6 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err;
 
-	/* VFS checks this, but only _after_ ->open() */
-	if (file->f_flags & O_DIRECT)
-		return -EINVAL;
-
 	err = generic_file_open(inode, file);
 	if (err)
 		return err;
@@ -932,6 +928,7 @@ static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 	struct file *file = iocb->ki_filp;
 	struct address_space *mapping = file->f_mapping;
 	size_t count = 0;
+	size_t ocount = 0;
 	ssize_t written = 0;
 	struct inode *inode = mapping->host;
 	ssize_t err;
@@ -939,10 +936,13 @@ static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 
 	WARN_ON(iocb->ki_pos != pos);
 
-	err = generic_segment_checks(iov, &nr_segs, &count, VERIFY_READ);
+	ocount = 0;
+	err = generic_segment_checks(iov, &nr_segs, &ocount, VERIFY_READ);
 	if (err)
 		return err;
 
+	count = ocount;
+
 	mutex_lock(&inode->i_mutex);
 	vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
 

fs/fuse/file.c static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov, · line 968

Quoted 7 lines
fs/fuse/file.c +4 -0
@@ -962,6 +962,13 @@ static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
 
 	file_update_time(file);
 
+	if (file->f_flags & O_DIRECT) {
+		written = generic_file_direct_write(iocb, iov, &nr_segs,
+						    pos, &iocb->ki_pos,
+						    count, ocount);

It looks to me like this will deadlock on i_mutex (fuse_direct_write() will try to lock it again, while it is already locked). Was this code path tested at all?

Also generic_file_direct_write() may return a short count in which case the write should be finished using buffered write.

Thanks,
Miklos

Quoted 76 lines
fs/fuse/file.c +59 -0
+		goto out;
+	}
+
 	iov_iter_init(&i, iov, nr_segs, count, 0);
 	written = fuse_perform_write(file, mapping, &i, pos);
 	if (written >= 0)
@@ -2077,6 +2084,61 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
 	return 0;
 }
 
+
+static ssize_t fuse_loop_rwv(struct file *filp, const struct iovec *iov,
+			     unsigned long nr_segs, loff_t *ppos, io_fn_t fn)
+{
+	const struct iovec *vector = iov;
+	ssize_t ret = 0;
+
+	while (nr_segs > 0) {
+		void __user *base;
+		size_t len;
+		ssize_t nr;
+
+		base = vector->iov_base;
+		len = vector->iov_len;
+		vector++;
+		nr_segs--;
+
+		nr = fn(filp, base, len, ppos);
+
+		if (nr < 0) {
+			if (!ret)
+				ret = nr;
+			break;
+		}
+		ret += nr;
+		if (nr != len)
+			break;
+	}
+
+	return ret;
+}
+
+
+static ssize_t
+fuse_direct_IO(int rw, struct kiocb *iocb, const struct iovec *iov,
+			loff_t offset, unsigned long nr_segs)
+{
+	ssize_t ret = 0;
+	struct file *file = NULL;
+	loff_t pos = 0;
+	io_fn_t fn = NULL;
+
+	file = iocb->ki_filp;
+	pos = offset;
+
+	if (rw == WRITE)
+		fn = (io_fn_t)fuse_direct_write;
+	else
+		fn = fuse_direct_read;
+
+	ret = fuse_loop_rwv(file, iov, nr_segs, &pos, fn);
+
+	return ret;
+}
+
 static const struct file_operations fuse_file_operations = {
 	.llseek		= fuse_file_llseek,
 	.read		= do_sync_read,
@@ -2120,6 +2182,7 @@ static const struct address_space_operations fuse_file_aops  = {
 	.readpages	= fuse_readpages,
 	.set_page_dirty	= __set_page_dirty_nobuffers,
 	.bmap		= fuse_bmap,
+	.direct_IO	= fuse_direct_IO,
 };
 
 void fuse_init_file_inode(struct inode *inode)