fuse: O_DIRECT support for files
Subsystems (2)
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
@@ -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
Miklos Szeredi
maintainerQuoted 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 existingdirect_ioframework.Reaching ->direct_IO() in the read path via
generic_file_aio_readensures proper synchronization with page cache with its existing framework.Reaching ->direct_IO() in the write path via
fuse_file_aio_writeis made to come viageneric_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 offcntl(O_DIRECT)and it always succeeds.
Quoted 6 lines
On files not marked with '
direct_io' by the filesystem server, the IO path depends onO_DIRECTflag by the application. This can be passed at the time ofopen()as well as viafcntl().Note that asynchronous
O_DIRECTiocb 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
Signed-off-by Anand Avati <[email protected]>--- fs/fuse/dir.c | 3 -- fs/fuse/file.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 8 deletions(-)fs/fuse/dir.c +0
-3@@ -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@@ -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)