From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-f67.google.com ([209.85.166.67]:42526 "EHLO mail-io1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727124AbeI0URU (ORCPT ); Thu, 27 Sep 2018 16:17:20 -0400 Received: by mail-io1-f67.google.com with SMTP id n18-v6so2001290ioa.9 for ; Thu, 27 Sep 2018 06:58:56 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180927135211.31641-1-dschatzberg@fb.com> References: <20180927135211.31641-1-dschatzberg@fb.com> From: Miklos Szeredi Date: Thu, 27 Sep 2018 15:58:55 +0200 Message-ID: Subject: Re: [PATCH] fuse: enable caching of symlinks To: Dan Schatzberg Cc: Tejun Heo , kernel-team , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Sep 27, 2018 at 3:52 PM, Dan Schatzberg wrote: > FUSE file reads are cached in the page cache, but symlink reads are > not. This patch enables FUSE READLINK operations to be cached which > can improve performance of some FUSE workloads. > > In particular, I'm working on a FUSE filesystem for access to source > code and discovered that about a 10% improvement to build times is > achieved with this patch (there are a lot of symlinks in the source > tree). > > Please provide feedback, I'm looking to flesh this out more. Need to think about how/when to invalidate the cached symlink contents. I think treating it like metadata (i.e. look at attr_timeout for validity) would be the simplest. Thanks, Miklos > > Signed-off-by: Dan Schatzberg > --- > fs/fuse/dir.c | 70 +++++++++++++++++++++++++++++++-------------------- > 1 file changed, 43 insertions(+), 27 deletions(-) > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index 0979609d6eba..3c0a81ef5bca 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -1398,38 +1398,46 @@ static int fuse_readdir(struct file *file, struct dir_context *ctx) > return err; > } > > -static const char *fuse_get_link(struct dentry *dentry, > - struct inode *inode, > - struct delayed_call *done) > +static int fuse_symlink_readpage(struct file *file, struct page *page) > { > + struct inode *inode = page->mapping->host; > struct fuse_conn *fc = get_fuse_conn(inode); > - FUSE_ARGS(args); > - char *link; > - ssize_t ret; > - > - if (!dentry) > - return ERR_PTR(-ECHILD); > + struct fuse_req *req; > + int err; > + size_t num_read; > > - link = kmalloc(PAGE_SIZE, GFP_KERNEL); > - if (!link) > - return ERR_PTR(-ENOMEM); > + err = -EIO; > + if (is_bad_inode(inode)) > + goto out; > > - args.in.h.opcode = FUSE_READLINK; > - args.in.h.nodeid = get_node_id(inode); > - args.out.argvar = 1; > - args.out.numargs = 1; > - args.out.args[0].size = PAGE_SIZE - 1; > - args.out.args[0].value = link; > - ret = fuse_simple_request(fc, &args); > - if (ret < 0) { > - kfree(link); > - link = ERR_PTR(ret); > - } else { > - link[ret] = '\0'; > - set_delayed_call(done, kfree_link, link); > + req = fuse_get_req(fc, 1); > + if (IS_ERR(req)) { > + err = PTR_ERR(req); > + goto out; > } > + > + req->out.page_zeroing = 1; > + req->out.argpages = 1; > + req->num_pages = 1; > + req->pages[0] = page; > + req->page_descs[0].length = PAGE_SIZE - 1; > + req->in.h.opcode = FUSE_READLINK; > + req->in.h.nodeid = get_node_id(inode); > + req->out.argvar = 1; > + req->out.numargs = 1; > + req->out.args[0].size = PAGE_SIZE - 1; > + fuse_request_send(fc, req); > + num_read = req->out.args[0].size; > + err = req->out.h.error; > + > + if (!err) > + SetPageUptodate(page); > + > + fuse_put_request(fc, req); > fuse_invalidate_atime(inode); > - return link; > +out: > + unlock_page(page); > + return err; > } > > static int fuse_dir_open(struct inode *inode, struct file *file) > @@ -1855,7 +1863,7 @@ static const struct inode_operations fuse_common_inode_operations = { > > static const struct inode_operations fuse_symlink_inode_operations = { > .setattr = fuse_setattr, > - .get_link = fuse_get_link, > + .get_link = page_get_link, > .getattr = fuse_getattr, > .listxattr = fuse_listxattr, > }; > @@ -1871,7 +1879,15 @@ void fuse_init_dir(struct inode *inode) > inode->i_fop = &fuse_dir_operations; > } > > +static const struct address_space_operations fuse_symlink_aops = { > + .readpage = fuse_symlink_readpage, > +}; > + > void fuse_init_symlink(struct inode *inode) > { > inode->i_op = &fuse_symlink_inode_operations; > + inode->i_data.a_ops = &fuse_symlink_aops; > + mapping_set_gfp_mask(inode->i_mapping, > + mapping_gfp_constraint(inode->i_mapping, > + ~(__GFP_FS | __GFP_HIGHMEM))); > } > -- > 2.17.1 >