From: Han-Wen N. <ha...@gm...> - 2014-01-07 22:46:47
|
This looks kind of interesting, but how would you populate the data in the kernel caches if the filesystem does not support OPEN? Do you use CREATE to insert data from the VFS side? On Wed, Nov 6, 2013 at 2:37 AM, Andrew Gallagher <and...@fb...> wrote: > Hey, > > We use FUSE internally to implement a read-only FS in which the data never changes. As such, we’re able to greatly improve performance by relying on in-kernel caches for lookups, attrs, and reads. We currently at a point where a significant amount of overhead is due to open/release calls hitting into userspace. Due to the read-only nature of our FS, we don’t actually need for these operations to hit userspace. As such I put together a few patches which allow the kernel module to forgo open/release userspace operations when the user-implemented open callback returns ENOSYS. This obviously breaks things that require that the fuse library keep track of open counts (e.g. rename, unlink), but in certain read-only setups this shouldn’t be an issue. > > I was hoping to get some feedback about whether this approach seems at all reasonable and what changes I’d need to make to push these upstream. In particular, I’m not sure of an appropriate way that the library can determine wether the fuse module supports the new FOPEN_NO_OPEN flag. Something like capabilities look interesting, but it didn’t seem to fit with the other things listed there. > > Thanks! > Andrew > > — > commit 5081c4385828be7a74fc622a04e8b829d6cbb21b > Author: Andrew Gallagher <aga...@fb...> > Date: Tue Nov 5 16:05:52 2013 > > fuse: support clients that don't implement 'open' > > open/release operations require userspace transitions to keep track > of the open count and to perform any FS-specific setup. However, > for some purely read-only FSs which don't need to perform any setup > at open/release time, we can avoid the performance overhead of > calling into userspace for open/release calls. > > This patch adds the necessary support to the fuse kernel modules > to prevent open/release operations from hitting in userspace. When > the client returns with the FOPEN_NO_OPEN flag set, we avoid sending > the subsequent release to userspace, and also remember this so that > future opens also don't trigger a userspace operation. > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index b7989f2..d5ba5b8 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -496,7 +496,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, > fuse_sync_release(ff, flags); > } else { > file->private_data = fuse_file_get(ff); > - fuse_finish_open(inode, file); > + fuse_finish_open(inode, file, false); > } > return err; > > diff --git a/fs/fuse/file.c b/fs/fuse/file.c > index 4598345..85c1789 100644 > --- a/fs/fuse/file.c > +++ b/fs/fuse/file.c > @@ -127,7 +127,12 @@ static void fuse_file_put(struct fuse_file *ff, bool sync) > if (atomic_dec_and_test(&ff->count)) { > struct fuse_req *req = ff->reserved_req; > > - if (sync) { > + /* Drop the release request when client does not implement 'open' */ > + if (ff->open_flags & FOPEN_NO_OPEN) { > + req->background = 0; > + path_put(&req->misc.release.path); > + fuse_put_request(ff->fc, req); > + } else if (sync) { > req->background = 0; > fuse_request_send(ff->fc, req); > path_put(&req->misc.release.path); > @@ -153,10 +158,15 @@ int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file, > if (!ff) > return -ENOMEM; > > - err = fuse_send_open(fc, nodeid, file, opcode, &outarg); > - if (err) { > - fuse_file_free(ff); > - return err; > + if (fc->no_open && !isdir) { > + outarg.fh = 0; > + outarg.open_flags = fc->no_open_flags; > + } else { > + err = fuse_send_open(fc, nodeid, file, opcode, &outarg); > + if (err) { > + fuse_file_free(ff); > + return err; > + } > } > > if (isdir) > @@ -171,7 +181,7 @@ int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file, > } > EXPORT_SYMBOL_GPL(fuse_do_open); > > -void fuse_finish_open(struct inode *inode, struct file *file) > +void fuse_finish_open(struct inode *inode, struct file *file, bool isdir) > { > struct fuse_file *ff = file->private_data; > struct fuse_conn *fc = get_fuse_conn(inode); > @@ -182,6 +192,11 @@ void fuse_finish_open(struct inode *inode, struct file *file) > invalidate_inode_pages2(inode->i_mapping); > if (ff->open_flags & FOPEN_NONSEEKABLE) > nonseekable_open(inode, file); > + /* Remember if the client doesn't implement open */ > + if (ff->open_flags & FOPEN_NO_OPEN && !isdir) { > + fc->no_open_flags = ff->open_flags; > + fc->no_open = 1; > + } > if (fc->atomic_o_trunc && (file->f_flags & O_TRUNC)) { > struct fuse_inode *fi = get_fuse_inode(inode); > > @@ -206,7 +221,7 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir) > if (err) > return err; > > - fuse_finish_open(inode, file); > + fuse_finish_open(inode, file, isdir); > > return 0; > } > @@ -287,7 +302,9 @@ void fuse_sync_release(struct fuse_file *ff, int flags) > fuse_prepare_release(ff, flags, FUSE_RELEASE); > ff->reserved_req->force = 1; > ff->reserved_req->background = 0; > - fuse_request_send(ff->fc, ff->reserved_req); > + /* Drop the release request when client does not implement 'open' */ > + if (!(ff->open_flags & FOPEN_NO_OPEN)) > + fuse_request_send(ff->fc, ff->reserved_req); > fuse_put_request(ff->fc, ff->reserved_req); > kfree(ff); > } > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index 5b9e6f3..4914023 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -485,6 +485,9 @@ struct fuse_conn { > * and hence races in setting them will not cause malfunction > */ > > + /** Is open/release not implemented by fs? */ > + unsigned no_open:1; > + > /** Is fsync not implemented by fs? */ > unsigned no_fsync:1; > > @@ -589,6 +592,9 @@ struct fuse_conn { > > /** Read/write semaphore to hold when accessing sb. */ > struct rw_semaphore killsb; > + > + /** Open flags to use when no_open is set. */ > + u32 no_open_flags; > }; > > static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb) > @@ -656,7 +662,7 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir); > struct fuse_file *fuse_file_alloc(struct fuse_conn *fc); > struct fuse_file *fuse_file_get(struct fuse_file *ff); > void fuse_file_free(struct fuse_file *ff); > -void fuse_finish_open(struct inode *inode, struct file *file); > +void fuse_finish_open(struct inode *inode, struct file *file, bool isdir); > > void fuse_sync_release(struct fuse_file *ff, int flags); > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h > index 60bb2f9..94b7dcb 100644 > --- a/include/uapi/linux/fuse.h > +++ b/include/uapi/linux/fuse.h > @@ -199,6 +199,7 @@ struct fuse_file_lock { > #define FOPEN_DIRECT_IO (1 << 0) > #define FOPEN_KEEP_CACHE (1 << 1) > #define FOPEN_NONSEEKABLE (1 << 2) > +#define FOPEN_NO_OPEN (1 << 3) > > /** > * INIT request/reply flags > > commit f94e96fc6e08f7856d78e16e7e682b38eb2c3250 > Author: Andrew Gallagher <aga...@fb...> > Date: Tue Nov 5 16:12:17 2013 > > fuse: support clients not iplementing 'open' > > open/release operations require userspace transitions to keep track > of the open count and to perform any FS-specific setup. However, > for some purely read-only FSs which don't need to perform any setup > at open/release time, we can avoid the performance overhead of > calling into userspace for open/release calls. > > This patch adds the necessary support to the fuse library component > to prevent open/release operations from hitting in userspace. When > the user-implemented open call just returns -ENOSYS a new open flag, > FOPEN_NO_OPEN, is set and passed back to the kernel to indiciate > that this client should not sent future open/release operations. > > Since this means we would no longer keep track of the open count in > userspace, this obviously won't work for filesystems that do mods > like renames, unlinks, etc. However, for read-only filesystems > this provides an optimization opportunity to avoid some costly > userspace switches. > > diff --git a/include/fuse_common.h b/include/fuse_common.h > index 765e0a3..efc4d3a 100644 > --- a/include/fuse_common.h > +++ b/include/fuse_common.h > @@ -52,6 +52,11 @@ struct fuse_file_info { > need not be invalidated. Introduced in version 2.4 */ > unsigned int keep_cache : 1; > > + /** Can be filled in by open, to indicate, that userspace does not > + implement open/release and we can therefore avoid userspace > + transitions. */ > + unsigned int no_open : 1; > + > /** Indicates a flush operation. Set in flush operation, also > maybe set in highlevel lock operation and lowlevel release > operation. Introduced in version 2.6 */ > diff --git a/include/fuse_kernel.h b/include/fuse_kernel.h > index 706d035..36044e2 100644 > --- a/include/fuse_kernel.h > +++ b/include/fuse_kernel.h > @@ -192,10 +192,12 @@ struct fuse_file_lock { > * FOPEN_DIRECT_IO: bypass page cache for this open file > * FOPEN_KEEP_CACHE: don't invalidate the data cache on open > * FOPEN_NONSEEKABLE: the file is not seekable > + * FOPEN_NO_OPEN: open is not implemented in userspace > */ > #define FOPEN_DIRECT_IO (1 << 0) > #define FOPEN_KEEP_CACHE (1 << 1) > #define FOPEN_NONSEEKABLE (1 << 2) > +#define FOPEN_NO_OPEN (1 << 3) > > /** > * INIT request/reply flags > diff --git a/lib/fuse.c b/lib/fuse.c > index 7508c54..1e647d3 100644 > --- a/lib/fuse.c > +++ b/lib/fuse.c > @@ -3042,13 +3042,15 @@ static void fuse_lib_open(fuse_req_t req, fuse_ino_t ino, > if (!err) { > fuse_prepare_interrupt(f, req, &d); > err = fuse_fs_open(f->fs, path, fi); > - if (!err) { > + if (!err || err == -ENOSYS) { > if (f->conf.direct_io) > fi->direct_io = 1; > if (f->conf.kernel_cache) > fi->keep_cache = 1; > + if (err == -ENOSYS) > + fi->no_open = 1; > > - if (f->conf.auto_cache) > + if (f->conf.auto_cache && !err) > open_auto_cache(f, ino, path, fi); > } > fuse_finish_interrupt(f, req, &d); > @@ -3062,6 +3064,8 @@ static void fuse_lib_open(fuse_req_t req, fuse_ino_t ino, > must be cancelled */ > fuse_do_release(f, ino, path, fi); > } > + } else if (err == -ENOSYS) { > + fuse_reply_open(req, fi); > } else > reply_err(req, err); > > diff --git a/lib/fuse_lowlevel.c b/lib/fuse_lowlevel.c > index 2b8df06..3a20eea 100755 > --- a/lib/fuse_lowlevel.c > +++ b/lib/fuse_lowlevel.c > @@ -443,6 +443,8 @@ static void fill_open(struct fuse_open_out *arg, > arg->open_flags |= FOPEN_KEEP_CACHE; > if (f->nonseekable) > arg->open_flags |= FOPEN_NONSEEKABLE; > + if (f->no_open) > + arg->open_flags |= FOPEN_NO_OPEN; > } > > int fuse_reply_entry(fuse_req_t req, const struct fuse_entry_param *e) > > > ------------------------------------------------------------------------------ > November Webinars for C, C++, Fortran Developers > Accelerate application performance with scalable programming models. Explore > techniques for threading, error checking, porting, and tuning. Get the most > from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk > _______________________________________________ > fuse-devel mailing list > fus...@li... > https://lists.sourceforge.net/lists/listinfo/fuse-devel -- Han-Wen Nienhuys - ha...@xs... - http://www.xs4all.nl/~hanwen |