From 9cd684551124e71630ab96d238747051463f5b56 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 1 Feb 2006 03:04:40 -0800 Subject: [PATCH] fuse: fix async read for legacy filesystems While asynchronous reads mean a performance improvement in most cases, if the filesystem assumed that reads are synchronous, then async reads may degrade performance (filesystem may receive reads out of order, which can confuse it's own readahead logic). With sshfs a 1.5 to 4 times slowdown can be measured. There's also a need for userspace filesystems to know whether asynchronous reads are supported by the kernel or not. To achive these, negotiate in the INIT request whether async reads will be used and the maximum readahead value. Update interface version to 7.6 If userspace uses a version earlier than 7.6, then disable async reads, and set maximum readahead value to the maximum read size, as done in previous versions. Signed-off-by: Miklos Szeredi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fuse/file.c | 9 +++++++-- fs/fuse/fuse_i.h | 3 +++ fs/fuse/inode.c | 14 ++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) (limited to 'fs/fuse') diff --git a/fs/fuse/file.c b/fs/fuse/file.c index a7ef5e716f..296351615b 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -335,9 +335,14 @@ static void fuse_send_readpages(struct fuse_req *req, struct file *file, loff_t pos = page_offset(req->pages[0]); size_t count = req->num_pages << PAGE_CACHE_SHIFT; req->out.page_zeroing = 1; - req->end = fuse_readpages_end; fuse_read_fill(req, file, inode, pos, count, FUSE_READ); - request_send_background(fc, req); + if (fc->async_read) { + req->end = fuse_readpages_end; + request_send_background(fc, req); + } else { + request_send(fc, req); + fuse_readpages_end(fc, req); + } } struct fuse_readpages_data { diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 46cf933aa3..4a83adfec9 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -272,6 +272,9 @@ struct fuse_conn { reply, before any other request, and never cleared */ unsigned conn_error : 1; + /** Do readpages asynchronously? Only set in INIT */ + unsigned async_read : 1; + /* * The following bitfields are only for optimization purposes * and hence races in setting them will not cause malfunction diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index c755a0440a..879e6fba94 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -473,6 +473,16 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req) if (req->out.h.error || arg->major != FUSE_KERNEL_VERSION) fc->conn_error = 1; else { + unsigned long ra_pages; + + if (arg->minor >= 6) { + ra_pages = arg->max_readahead / PAGE_CACHE_SIZE; + if (arg->flags & FUSE_ASYNC_READ) + fc->async_read = 1; + } else + ra_pages = fc->max_read / PAGE_CACHE_SIZE; + + fc->bdi.ra_pages = min(fc->bdi.ra_pages, ra_pages); fc->minor = arg->minor; fc->max_write = arg->minor < 5 ? 4096 : arg->max_write; } @@ -496,6 +506,8 @@ static void fuse_send_init(struct fuse_conn *fc) arg->major = FUSE_KERNEL_VERSION; arg->minor = FUSE_KERNEL_MINOR_VERSION; + arg->max_readahead = fc->bdi.ra_pages * PAGE_CACHE_SIZE; + arg->flags |= FUSE_ASYNC_READ; req->in.h.opcode = FUSE_INIT; req->in.numargs = 1; req->in.args[0].size = sizeof(*arg); @@ -552,8 +564,6 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent) fc->user_id = d.user_id; fc->group_id = d.group_id; fc->max_read = d.max_read; - if (fc->max_read / PAGE_CACHE_SIZE < fc->bdi.ra_pages) - fc->bdi.ra_pages = fc->max_read / PAGE_CACHE_SIZE; /* Used by get_root_inode() */ sb->s_fs_info = fc; -- cgit v1.2.2 From 7128ec2a747d7a5f3c764c37bef17081ccc2374c Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Sat, 4 Feb 2006 23:27:40 -0800 Subject: [PATCH] fuse: fix request_end() vs fuse_reset_request() race The last fix for this function in fact opened up a much more often triggering race. It was uncommented tricky code, that was buggy. Add comment, make it less tricky and fix bug. Signed-off-by: Miklos Szeredi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fuse/dev.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) (limited to 'fs/fuse') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 4526da8907..f556a0d5c0 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -120,9 +120,9 @@ struct fuse_req *fuse_get_request(struct fuse_conn *fc) return do_get_request(fc); } +/* Must be called with fuse_lock held */ static void fuse_putback_request(struct fuse_conn *fc, struct fuse_req *req) { - spin_lock(&fuse_lock); if (req->preallocated) { atomic_dec(&fc->num_waiting); list_add(&req->list, &fc->unused_list); @@ -134,10 +134,18 @@ static void fuse_putback_request(struct fuse_conn *fc, struct fuse_req *req) fc->outstanding_debt--; else up(&fc->outstanding_sem); - spin_unlock(&fuse_lock); } void fuse_put_request(struct fuse_conn *fc, struct fuse_req *req) +{ + if (atomic_dec_and_test(&req->count)) { + spin_lock(&fuse_lock); + fuse_putback_request(fc, req); + spin_unlock(&fuse_lock); + } +} + +static void fuse_put_request_locked(struct fuse_conn *fc, struct fuse_req *req) { if (atomic_dec_and_test(&req->count)) fuse_putback_request(fc, req); @@ -163,26 +171,36 @@ void fuse_release_background(struct fuse_req *req) * still waiting), the 'end' callback is called if given, else the * reference to the request is released * + * Releasing extra reference for foreground requests must be done + * within the same locked region as setting state to finished. This + * is because fuse_reset_request() may be called after request is + * finished and it must be the sole possessor. If request is + * interrupted and put in the background, it will return with an error + * and hence never be reset and reused. + * * Called with fuse_lock, unlocks it */ static void request_end(struct fuse_conn *fc, struct fuse_req *req) { - void (*end) (struct fuse_conn *, struct fuse_req *) = req->end; - req->end = NULL; list_del(&req->list); req->state = FUSE_REQ_FINISHED; - spin_unlock(&fuse_lock); - if (req->background) { + if (!req->background) { + wake_up(&req->waitq); + fuse_put_request_locked(fc, req); + spin_unlock(&fuse_lock); + } else { + void (*end) (struct fuse_conn *, struct fuse_req *) = req->end; + req->end = NULL; + spin_unlock(&fuse_lock); down_read(&fc->sbput_sem); if (fc->mounted) fuse_release_background(req); up_read(&fc->sbput_sem); + if (end) + end(fc, req); + else + fuse_put_request(fc, req); } - wake_up(&req->waitq); - if (end) - end(fc, req); - else - fuse_put_request(fc, req); } /* -- cgit v1.2.2 From 77e7f250f88cd62844e24c42aff4d0e95969c746 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Fri, 17 Feb 2006 13:52:52 -0800 Subject: [PATCH] fuse: fix bug in aborted fuse_release_end() There's a rather theoretical case of the BUG triggering in fuse_reset_request(): - iget() fails because of OOM after a successful CREATE_OPEN request - during IO on the resulting RELEASE request the connection is aborted Fix and add warning to fuse_reset_request(). Signed-off-by: Miklos Szeredi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fuse/dev.c | 6 ++++++ fs/fuse/file.c | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'fs/fuse') diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index f556a0d5c0..0c9a2ee54c 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -66,6 +66,12 @@ static void restore_sigs(sigset_t *oldset) sigprocmask(SIG_SETMASK, oldset, NULL); } +/* + * Reset request, so that it can be reused + * + * The caller must be _very_ careful to make sure, that it is holding + * the only reference to req + */ void fuse_reset_request(struct fuse_req *req) { int preallocated = req->preallocated; diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 296351615b..6f05379b0a 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -116,9 +116,14 @@ int fuse_open_common(struct inode *inode, struct file *file, int isdir) /* Special case for failed iget in CREATE */ static void fuse_release_end(struct fuse_conn *fc, struct fuse_req *req) { - u64 nodeid = req->in.h.nodeid; - fuse_reset_request(req); - fuse_send_forget(fc, req, nodeid, 1); + /* If called from end_io_requests(), req has more than one + reference and fuse_reset_request() cannot work */ + if (fc->connected) { + u64 nodeid = req->in.h.nodeid; + fuse_reset_request(req); + fuse_send_forget(fc, req, nodeid, 1); + } else + fuse_put_request(fc, req); } void fuse_send_release(struct fuse_conn *fc, struct fuse_file *ff, -- cgit v1.2.2 From 50322fe7d46b544d5649edb58bdbe5c95dd44b98 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Tue, 28 Feb 2006 16:59:03 -0800 Subject: [PATCH] fuse: fix bug in negative lookup If negative entries (nodeid == 0) were sent in reply to LOOKUP requests, two bugs could be triggered: - looking up a negative entry would return -EIO, - revaildate on an entry which turned negative would send a FORGET request with zero nodeid, which would cause an abort() in the library. The above would only happen if the 'negative_timeout=N' option was used, otherwise lookups reply -ENOENT, which worked correctly. Signed-off-by: Miklos Szeredi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/fuse/dir.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'fs/fuse') diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 21fd59c7bc..c72a8a9793 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -111,6 +111,8 @@ static int fuse_dentry_revalidate(struct dentry *entry, struct nameidata *nd) /* Doesn't hurt to "reset" the validity timeout */ fuse_invalidate_entry_cache(entry); + + /* For negative dentries, always do a fresh lookup */ if (!inode) return 0; @@ -122,6 +124,9 @@ static int fuse_dentry_revalidate(struct dentry *entry, struct nameidata *nd) fuse_lookup_init(req, entry->d_parent->d_inode, entry, &outarg); request_send(fc, req); err = req->out.h.error; + /* Zero nodeid is same as -ENOENT */ + if (!err && !outarg.nodeid) + err = -ENOENT; if (!err) { struct fuse_inode *fi = get_fuse_inode(inode); if (outarg.nodeid != get_node_id(inode)) { @@ -190,8 +195,9 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry, fuse_lookup_init(req, dir, entry, &outarg); request_send(fc, req); err = req->out.h.error; - if (!err && ((outarg.nodeid && invalid_nodeid(outarg.nodeid)) || - !valid_mode(outarg.attr.mode))) + /* Zero nodeid is same as -ENOENT, but with valid timeout */ + if (!err && outarg.nodeid && + (invalid_nodeid(outarg.nodeid) || !valid_mode(outarg.attr.mode))) err = -EIO; if (!err && outarg.nodeid) { inode = fuse_iget(dir->i_sb, outarg.nodeid, outarg.generation, -- cgit v1.2.2