On Thu, Dec 10, 2015 at 02:40:50AM +0000, Al Viro wrote: > ... except that it would have to be something like work_struct, since > callback_head gets embedded into the argument and here we don't want to > do that. _And_ we've no use for ->entry in work_struct, which makes it > a bad fit as well. IOW, please ignore that idiocy... OK, the following (incremental to that series) seems to be better; in particular, overlayfs ->get_link() doesn't allocate anything whatsoever, ->put_link() as a method is gone and instead of *cookie = <something that will make sense for our put_link> we do set_delayed_call(done, callback, argument) which is a whole lot more straightforward. Memory footprint cost is trivial - one extra pointer in struct nameidata. Basically, we add a new structure (struct delayed_call; poor man's closure) and pass a reference to that to ->get_link(); if anything needs to be done once we are done with the string returned by ->get_link(), just fill that structure (as above) and that's it. It will be evaluated by caller once it's through with the symlink body. I'd put it in linux/fs.h for now, but it certainly belongs in something like linux/types.h; the current location is temporary. And I'm quite sure that we have other places where something of that sort is open-coded - timers, for one thing and probably quite a few other things. I would really love to be able to say set_delayed_call(done, kfree, p); but as it is I had to keep a wrapper - void kfree_link(void *). The problem is, you can't assign void f(const void *) to void (*p)(void *) - mismatch of qualifiers in the arguments makes the latter not assignment-compatible with the former. If there's a clever trick allowing to sidestep that, I'd be very happy; I don't know one. Any ideas not starting with "use C11" (or, worse yet, "use such and such C++ misfeature with arseloads of RTL required in order to implement it") would be welcome... BTW, why are we passing unsigned long to free_page()? We have a bit under 700 callers; excluding the ones that have an explicit cast to unsigned long right in the argument of call leaves ~150, and the rest tend to contain a lot of pointer casts of unsigned long thing they are feeding to free_page() (IOW, they would be just as happy if they kept it as a pointer all along). Sure, that would mean __get_free_page() et.al. returning void *, but I don't see any problems with that either... Is that just for historical reasons, or is there anything more subtle I'm missing here? Anyway, for now the patch below is strictly for review and (if one is brave enough) testing. Comments would be very welcome. diff --git a/drivers/staging/lustre/lustre/llite/symlink.c b/drivers/staging/lustre/lustre/llite/symlink.c index e5c4137..2610348 100644 --- a/drivers/staging/lustre/lustre/llite/symlink.c +++ b/drivers/staging/lustre/lustre/llite/symlink.c @@ -118,8 +118,14 @@ failed: return rc; } +static void ll_put_link(void *p) +{ + ptlrpc_req_finished(p); +} + static const char *ll_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct ptlrpc_request *request = NULL; int rc; @@ -137,22 +143,16 @@ static const char *ll_get_link(struct dentry *dentry, } /* symname may contain a pointer to the request message buffer, - * we delay request releasing until ll_put_link then. + * we delay request releasing then. */ - *cookie = request; + set_delayed_call(done, ll_put_link, request); return symname; } -static void ll_put_link(struct inode *unused, void *cookie) -{ - ptlrpc_req_finished(cookie); -} - const struct inode_operations ll_fast_symlink_inode_operations = { .readlink = generic_readlink, .setattr = ll_setattr, .get_link = ll_get_link, - .put_link = ll_put_link, .getattr = ll_getattr, .permission = ll_inode_permission, .setxattr = ll_setxattr, diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index a237f47..c7cc7c3 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -1226,11 +1226,12 @@ ino_t v9fs_qid2ino(struct p9_qid *qid) * v9fs_vfs_get_link - follow a symlink path * @dentry: dentry for symlink * @inode: inode for symlink - * @cookie: place to pass the data to put_link() + * @done: delayed call for when we are done with the return value */ static const char *v9fs_vfs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct v9fs_session_info *v9ses; struct p9_fid *fid; @@ -1266,7 +1267,8 @@ static const char *v9fs_vfs_get_link(struct dentry *dentry, p9stat_free(st); kfree(st); - return *cookie = res; + set_delayed_call(done, kfree_link, res); + return res; } /** @@ -1460,7 +1462,6 @@ static const struct inode_operations v9fs_file_inode_operations = { static const struct inode_operations v9fs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = v9fs_vfs_get_link, - .put_link = kfree_put_link, .getattr = v9fs_vfs_getattr, .setattr = v9fs_vfs_setattr, }; diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c index 0cc105d..a34702c 100644 --- a/fs/9p/vfs_inode_dotl.c +++ b/fs/9p/vfs_inode_dotl.c @@ -902,12 +902,13 @@ error: * v9fs_vfs_get_link_dotl - follow a symlink path * @dentry: dentry for symlink * @inode: inode for symlink - * @cookie: place to pass the data to put_link() + * @done: destructor for return value */ static const char * v9fs_vfs_get_link_dotl(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct p9_fid *fid; char *target; @@ -924,7 +925,8 @@ v9fs_vfs_get_link_dotl(struct dentry *dentry, retval = p9_client_readlink(fid, &target); if (retval) return ERR_PTR(retval); - return *cookie = target; + set_delayed_call(done, kfree_link, target); + return target; } int v9fs_refresh_inode_dotl(struct p9_fid *fid, struct inode *inode) @@ -991,7 +993,6 @@ const struct inode_operations v9fs_file_inode_operations_dotl = { const struct inode_operations v9fs_symlink_inode_operations_dotl = { .readlink = generic_readlink, .get_link = v9fs_vfs_get_link_dotl, - .put_link = kfree_put_link, .getattr = v9fs_vfs_getattr_dotl, .setattr = v9fs_vfs_setattr_dotl, .setxattr = generic_setxattr, diff --git a/fs/affs/symlink.c b/fs/affs/symlink.c index 39d1194..69b03db 100644 --- a/fs/affs/symlink.c +++ b/fs/affs/symlink.c @@ -72,6 +72,5 @@ const struct address_space_operations affs_symlink_aops = { const struct inode_operations affs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = affs_notify_change, }; diff --git a/fs/autofs4/symlink.c b/fs/autofs4/symlink.c index 39e6f0b..84e037d 100644 --- a/fs/autofs4/symlink.c +++ b/fs/autofs4/symlink.c @@ -13,7 +13,8 @@ #include "autofs_i.h" static const char *autofs4_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct autofs_sb_info *sbi; struct autofs_info *ino; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 5234474..3b8856e 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -10097,7 +10097,6 @@ static const struct inode_operations btrfs_special_inode_operations = { static const struct inode_operations btrfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .getattr = btrfs_getattr, .setattr = btrfs_setattr, .permission = btrfs_permission, diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index edcf0fa..b7fcb31 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -901,7 +901,6 @@ const struct inode_operations cifs_file_inode_ops = { const struct inode_operations cifs_symlink_inode_ops = { .readlink = generic_readlink, .get_link = cifs_get_link, - .put_link = kfree_put_link, .permission = cifs_permission, /* BB add the following two eventually */ /* revalidate: cifs_revalidate, diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index 8e79c25..68c4547 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -120,7 +120,8 @@ extern struct vfsmount *cifs_dfs_d_automount(struct path *path); #endif /* Functions related to symlinks */ -extern const char *cifs_get_link(struct dentry *, struct inode *, void **); +extern const char *cifs_get_link(struct dentry *, struct inode *, + struct delayed_call *); extern int cifs_symlink(struct inode *inode, struct dentry *direntry, const char *symname); extern int cifs_removexattr(struct dentry *, const char *); diff --git a/fs/cifs/link.c b/fs/cifs/link.c index 6f2439b5..062c237 100644 --- a/fs/cifs/link.c +++ b/fs/cifs/link.c @@ -627,7 +627,8 @@ cifs_hl_exit: } const char * -cifs_get_link(struct dentry *direntry, struct inode *inode, void **cookie) +cifs_get_link(struct dentry *direntry, struct inode *inode, + struct delayed_call *done) { int rc = -ENOMEM; unsigned int xid; @@ -680,7 +681,8 @@ cifs_get_link(struct dentry *direntry, struct inode *inode, void **cookie) kfree(target_path); return ERR_PTR(rc); } - return *cookie = target_path; + set_delayed_call(done, kfree_link, target_path); + return target_path; } int diff --git a/fs/coda/cnode.c b/fs/coda/cnode.c index f18139c..1bfb7ba 100644 --- a/fs/coda/cnode.c +++ b/fs/coda/cnode.c @@ -19,7 +19,6 @@ static inline int coda_fideq(struct CodaFid *fid1, struct CodaFid *fid2) static const struct inode_operations coda_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = coda_setattr, }; diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c index b91c01e..83d80a7 100644 --- a/fs/configfs/symlink.c +++ b/fs/configfs/symlink.c @@ -280,9 +280,11 @@ static int configfs_getlink(struct dentry *dentry, char * path) } static const char *configfs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { unsigned long page; + char *body; int error; if (!dentry) @@ -292,9 +294,11 @@ static const char *configfs_get_link(struct dentry *dentry, if (!page) return ERR_PTR(-ENOMEM); - error = configfs_getlink(dentry, (char *)page); + body = (char *)page; + error = configfs_getlink(dentry, body); if (!error) { - return *cookie = (void *)page; + set_delayed_call(done, free_page_link, body); + return body; } free_page(page); @@ -304,7 +308,6 @@ static const char *configfs_get_link(struct dentry *dentry, const struct inode_operations configfs_symlink_inode_operations = { .get_link = configfs_get_link, .readlink = generic_readlink, - .put_link = free_page_put_link, .setattr = configfs_setattr, }; diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 55241bf..040aa87 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -673,7 +673,8 @@ out: } static const char *ecryptfs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { size_t len; char *buf; @@ -687,7 +688,8 @@ static const char *ecryptfs_get_link(struct dentry *dentry, fsstack_copy_attr_atime(d_inode(dentry), d_inode(ecryptfs_dentry_to_lower(dentry))); buf[len] = '\0'; - return *cookie = buf; + set_delayed_call(done, kfree_link, buf); + return buf; } /** @@ -1100,7 +1102,6 @@ out: const struct inode_operations ecryptfs_symlink_iops = { .readlink = generic_readlink, .get_link = ecryptfs_get_link, - .put_link = kfree_put_link, .permission = ecryptfs_permission, .setattr = ecryptfs_setattr, .getattr = ecryptfs_getattr_link, diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c index 4690511..3495d8a 100644 --- a/fs/ext2/symlink.c +++ b/fs/ext2/symlink.c @@ -23,7 +23,6 @@ const struct inode_operations ext2_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = ext2_setattr, #ifdef CONFIG_EXT2_FS_XATTR .setxattr = generic_setxattr, diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c index 3b4bfe2..2281ac2 100644 --- a/fs/ext4/symlink.c +++ b/fs/ext4/symlink.c @@ -24,7 +24,8 @@ #ifdef CONFIG_EXT4_FS_ENCRYPTION static const char *ext4_encrypted_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct page *cpage = NULL; char *caddr, *paddr = NULL; @@ -80,7 +81,8 @@ static const char *ext4_encrypted_get_link(struct dentry *dentry, paddr[res] = '\0'; if (cpage) page_cache_release(cpage); - return *cookie = paddr; + set_delayed_call(done, kfree_link, paddr); + return paddr; errout: if (cpage) page_cache_release(cpage); @@ -91,7 +93,6 @@ errout: const struct inode_operations ext4_encrypted_symlink_inode_operations = { .readlink = generic_readlink, .get_link = ext4_encrypted_get_link, - .put_link = kfree_put_link, .setattr = ext4_setattr, .setxattr = generic_setxattr, .getxattr = generic_getxattr, @@ -103,7 +104,6 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = { const struct inode_operations ext4_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = ext4_setattr, .setxattr = generic_setxattr, .getxattr = generic_getxattr, diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 2a8d84b..1165021 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -316,12 +316,14 @@ fail: } static const char *f2fs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { - const char *link = page_get_link(dentry, inode, cookie); + const char *link = page_get_link(dentry, inode, done); if (!IS_ERR(link) && !*link) { /* this is broken symlink case */ - page_put_link(NULL, *cookie); + do_delayed_call(done); + done->fn = NULL; link = ERR_PTR(-ENOENT); } return link; @@ -926,7 +928,8 @@ static int f2fs_rename2(struct inode *old_dir, struct dentry *old_dentry, #ifdef CONFIG_F2FS_FS_ENCRYPTION static const char *f2fs_encrypted_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct page *cpage = NULL; char *caddr, *paddr = NULL; @@ -988,7 +991,8 @@ static const char *f2fs_encrypted_get_link(struct dentry *dentry, paddr[res] = '\0'; page_cache_release(cpage); - return *cookie = paddr; + set_delayed_call(done, kfree_link, paddr); + return paddr; errout: kfree(cstr.name); f2fs_fname_crypto_free_buffer(&pstr); @@ -999,7 +1003,6 @@ errout: const struct inode_operations f2fs_encrypted_symlink_inode_operations = { .readlink = generic_readlink, .get_link = f2fs_encrypted_get_link, - .put_link = kfree_put_link, .getattr = f2fs_getattr, .setattr = f2fs_setattr, .setxattr = generic_setxattr, @@ -1035,7 +1038,6 @@ const struct inode_operations f2fs_dir_inode_operations = { const struct inode_operations f2fs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = f2fs_get_link, - .put_link = page_put_link, .getattr = f2fs_getattr, .setattr = f2fs_setattr, #ifdef CONFIG_F2FS_FS_XATTR diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 148e8ef..e397c1a 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1366,7 +1366,8 @@ static int fuse_readdir(struct file *file, struct dir_context *ctx) } static const char *fuse_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct fuse_conn *fc = get_fuse_conn(inode); FUSE_ARGS(args); @@ -1392,7 +1393,7 @@ static const char *fuse_get_link(struct dentry *dentry, link = ERR_PTR(ret); } else { link[ret] = '\0'; - *cookie = link; + set_delayed_call(done, free_page_link, link); } fuse_invalidate_atime(inode); return link; @@ -1913,7 +1914,6 @@ 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, - .put_link = free_page_put_link, .readlink = generic_readlink, .getattr = fuse_getattr, .setxattr = fuse_setxattr, diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 10950560..1bae189 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -1715,7 +1715,7 @@ static int gfs2_rename2(struct inode *odir, struct dentry *odentry, * gfs2_get_link - Follow a symbolic link * @dentry: The dentry of the link * @inode: The inode of the link - * @cookie: place to store the information for ->put_link() + * @done: destructor for return value * * This can handle symlinks of any size. * @@ -1723,7 +1723,8 @@ static int gfs2_rename2(struct inode *odir, struct dentry *odentry, */ static const char *gfs2_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct gfs2_inode *ip = GFS2_I(inode); struct gfs2_holder i_gh; @@ -1764,7 +1765,7 @@ static const char *gfs2_get_link(struct dentry *dentry, out: gfs2_glock_dq_uninit(&i_gh); if (!IS_ERR(buf)) - *cookie = buf; + set_delayed_call(done, kfree_link, buf); return buf; } @@ -2138,7 +2139,6 @@ const struct inode_operations gfs2_dir_iops = { const struct inode_operations gfs2_symlink_iops = { .readlink = generic_readlink, .get_link = gfs2_get_link, - .put_link = kfree_put_link, .permission = gfs2_permission, .setattr = gfs2_setattr, .getattr = gfs2_getattr, diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 6ce5309..7db524c 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -893,12 +893,13 @@ static const struct inode_operations hostfs_dir_iops = { }; static const char *hostfs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { char *link; if (!dentry) return ERR_PTR(-ECHILD); - link = __getname(); + link = kmalloc(PATH_MAX, GFP_KERNEL); if (link) { char *path = dentry_name(dentry); int err = -ENOMEM; @@ -909,25 +910,20 @@ static const char *hostfs_get_link(struct dentry *dentry, __putname(path); } if (err < 0) { - __putname(link); + kfree(link); return ERR_PTR(err); } } else { return ERR_PTR(-ENOMEM); } - return *cookie = link; -} - -static void hostfs_put_link(struct inode *unused, void *cookie) -{ - __putname(cookie); + set_delayed_call(done, kfree_link, link); + return link; } static const struct inode_operations hostfs_link_iops = { .readlink = generic_readlink, .get_link = hostfs_get_link, - .put_link = hostfs_put_link, }; static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) diff --git a/fs/jfs/symlink.c b/fs/jfs/symlink.c index 0211328..f8db4fd 100644 --- a/fs/jfs/symlink.c +++ b/fs/jfs/symlink.c @@ -34,7 +34,6 @@ const struct inode_operations jfs_fast_symlink_inode_operations = { const struct inode_operations jfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = jfs_setattr, .setxattr = jfs_setxattr, .getxattr = jfs_getxattr, diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c index ffae857..1267e0c 100644 --- a/fs/kernfs/symlink.c +++ b/fs/kernfs/symlink.c @@ -113,22 +113,26 @@ static int kernfs_getlink(struct dentry *dentry, char *path) } static const char *kernfs_iop_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { - int error = -ENOMEM; unsigned long page; + char *body; + int error; if (!dentry) return ERR_PTR(-ECHILD); page = get_zeroed_page(GFP_KERNEL); if (!page) return ERR_PTR(-ENOMEM); - error = kernfs_getlink(dentry, (char *)page); + body = (char *)page; + error = kernfs_getlink(dentry, body); if (unlikely(error < 0)) { - free_page((unsigned long)page); + free_page(page); return ERR_PTR(error); } - return *cookie = (char *)page; + set_delayed_call(done, free_page_link, body); + return body; } const struct inode_operations kernfs_symlink_iops = { @@ -138,7 +142,6 @@ const struct inode_operations kernfs_symlink_iops = { .listxattr = kernfs_iop_listxattr, .readlink = generic_readlink, .get_link = kernfs_iop_get_link, - .put_link = free_page_put_link, .setattr = kernfs_iop_setattr, .getattr = kernfs_iop_getattr, .permission = kernfs_iop_permission, diff --git a/fs/libfs.c b/fs/libfs.c index 8dc37fc..447bba7 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -1019,17 +1019,18 @@ int noop_fsync(struct file *file, loff_t start, loff_t end, int datasync) } EXPORT_SYMBOL(noop_fsync); -void kfree_put_link(struct inode *unused, void *cookie) +/* Because kfree isn't assignment-compatible with void(void*) ;-/ */ +void kfree_link(void *p) { - kfree(cookie); + kfree(p); } -EXPORT_SYMBOL(kfree_put_link); +EXPORT_SYMBOL(kfree_link); -void free_page_put_link(struct inode *unused, void *cookie) +void free_page_link(void *p) { - free_page((unsigned long) cookie); + free_page((unsigned long)p); } -EXPORT_SYMBOL(free_page_put_link); +EXPORT_SYMBOL(free_page_link); /* * nop .set_page_dirty method so that people can use .page_mkwrite on @@ -1093,7 +1094,7 @@ simple_nosetlease(struct file *filp, long arg, struct file_lock **flp, EXPORT_SYMBOL(simple_nosetlease); const char *simple_get_link(struct dentry *dentry, struct inode *inode, - void **cookie) + struct delayed_call *done) { return inode->i_link; } diff --git a/fs/minix/inode.c b/fs/minix/inode.c index 3cce709..cb1789c 100644 --- a/fs/minix/inode.c +++ b/fs/minix/inode.c @@ -436,7 +436,6 @@ static const struct address_space_operations minix_aops = { static const struct inode_operations minix_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .getattr = minix_getattr, }; diff --git a/fs/namei.c b/fs/namei.c index ea20ff4..8914593 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -505,13 +505,13 @@ struct nameidata { int total_link_count; struct saved { struct path link; - void *cookie; + struct delayed_call done; const char *name; - struct inode *inode; unsigned seq; } *stack, internal[EMBEDDED_LEVELS]; struct filename *name; struct nameidata *saved; + struct inode *link_inode; unsigned root_seq; int dfd; }; @@ -590,11 +590,8 @@ static void drop_links(struct nameidata *nd) int i = nd->depth; while (i--) { struct saved *last = nd->stack + i; - struct inode *inode = last->inode; - if (last->cookie && inode->i_op->put_link) { - inode->i_op->put_link(inode, last->cookie); - last->cookie = NULL; - } + do_delayed_call(&last->done); + last->done.fn = NULL; } } @@ -876,9 +873,7 @@ void nd_jump_link(struct path *path) static inline void put_link(struct nameidata *nd) { struct saved *last = nd->stack + --nd->depth; - struct inode *inode = last->inode; - if (last->cookie && inode->i_op->put_link) - inode->i_op->put_link(inode, last->cookie); + do_delayed_call(&last->done); if (!(nd->flags & LOOKUP_RCU)) path_put(&last->link); } @@ -910,7 +905,7 @@ static inline int may_follow_link(struct nameidata *nd) return 0; /* Allowed if owner and follower match. */ - inode = nd->stack[0].inode; + inode = nd->link_inode; if (uid_eq(current_cred()->fsuid, inode->i_uid)) return 0; @@ -1001,7 +996,7 @@ const char *get_link(struct nameidata *nd) { struct saved *last = nd->stack + nd->depth - 1; struct dentry *dentry = last->link.dentry; - struct inode *inode = last->inode; + struct inode *inode = nd->link_inode; int error; const char *res; @@ -1022,23 +1017,21 @@ const char *get_link(struct nameidata *nd) nd->last_type = LAST_BIND; res = inode->i_link; if (!res) { + const char * (*get)(struct dentry *, struct inode *, + struct delayed_call *); + get = inode->i_op->get_link; if (nd->flags & LOOKUP_RCU) { - res = inode->i_op->get_link(NULL, inode, - &last->cookie); + res = get(NULL, inode, &last->done); if (res == ERR_PTR(-ECHILD)) { if (unlikely(unlazy_walk(nd, NULL, 0))) return ERR_PTR(-ECHILD); - res = inode->i_op->get_link(dentry, inode, - &last->cookie); + res = get(dentry, inode, &last->done); } } else { - res = inode->i_op->get_link(dentry, inode, - &last->cookie); + res = get(dentry, inode, &last->done); } - if (IS_ERR_OR_NULL(res)) { - last->cookie = NULL; + if (IS_ERR_OR_NULL(res)) return res; - } } if (*res == '/') { if (!nd->root.mnt) @@ -1699,8 +1692,8 @@ static int pick_link(struct nameidata *nd, struct path *link, last = nd->stack + nd->depth++; last->link = *link; - last->cookie = NULL; - last->inode = inode; + last->done.fn = NULL; + nd->link_inode = inode; last->seq = seq; return 1; } @@ -4502,26 +4495,25 @@ EXPORT_SYMBOL(readlink_copy); */ int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen) { - void *cookie; + DEFINE_DELAYED_CALL(done); struct inode *inode = d_inode(dentry); const char *link = inode->i_link; int res; if (!link) { - link = inode->i_op->get_link(dentry, inode, &cookie); + link = inode->i_op->get_link(dentry, inode, &done); if (IS_ERR(link)) return PTR_ERR(link); } res = readlink_copy(buffer, buflen, link); - if (inode->i_op->put_link) - inode->i_op->put_link(inode, cookie); + do_delayed_call(&done); return res; } EXPORT_SYMBOL(generic_readlink); /* get the link contents into pagecache */ const char *page_get_link(struct dentry *dentry, struct inode *inode, - void **cookie) + struct delayed_call *callback) { char *kaddr; struct page *page; @@ -4540,7 +4532,7 @@ const char *page_get_link(struct dentry *dentry, struct inode *inode, if (IS_ERR(page)) return (char*)page; } - *cookie = page; + set_delayed_call(callback, page_put_link, page); BUG_ON(mapping_gfp_mask(mapping) & __GFP_HIGHMEM); kaddr = page_address(page); nd_terminate_link(kaddr, inode->i_size, PAGE_SIZE - 1); @@ -4549,21 +4541,19 @@ const char *page_get_link(struct dentry *dentry, struct inode *inode, EXPORT_SYMBOL(page_get_link); -void page_put_link(struct inode *unused, void *cookie) +void page_put_link(void *arg) { - struct page *page = cookie; - page_cache_release(page); + put_page(arg); } EXPORT_SYMBOL(page_put_link); int page_readlink(struct dentry *dentry, char __user *buffer, int buflen) { - void *cookie = NULL; + DEFINE_DELAYED_CALL(done); int res = readlink_copy(buffer, buflen, page_get_link(dentry, d_inode(dentry), - &cookie)); - if (cookie) - page_put_link(NULL, cookie); + &done)); + do_delayed_call(&done); return res; } EXPORT_SYMBOL(page_readlink); @@ -4613,6 +4603,5 @@ EXPORT_SYMBOL(page_symlink); const struct inode_operations page_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, }; EXPORT_SYMBOL(page_symlink_inode_operations); diff --git a/fs/ncpfs/inode.c b/fs/ncpfs/inode.c index 3ab6cdb..ce1eb3f 100644 --- a/fs/ncpfs/inode.c +++ b/fs/ncpfs/inode.c @@ -245,7 +245,6 @@ static void ncp_set_attr(struct inode *inode, struct ncp_entry_info *nwinfo) static const struct inode_operations ncp_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = ncp_notify_change, }; #endif diff --git a/fs/nfs/symlink.c b/fs/nfs/symlink.c index 95c69af..4fe3eea 100644 --- a/fs/nfs/symlink.c +++ b/fs/nfs/symlink.c @@ -43,7 +43,8 @@ error: } static const char *nfs_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct page *page; void *err; @@ -68,7 +69,7 @@ static const char *nfs_get_link(struct dentry *dentry, if (IS_ERR(page)) return ERR_CAST(page); } - *cookie = page; + set_delayed_call(done, page_put_link, page); return page_address(page); } @@ -78,7 +79,6 @@ static const char *nfs_get_link(struct dentry *dentry, const struct inode_operations nfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = nfs_get_link, - .put_link = page_put_link, .getattr = nfs_getattr, .setattr = nfs_setattr, }; diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c index 63dddb7..7ccdb96 100644 --- a/fs/nilfs2/namei.c +++ b/fs/nilfs2/namei.c @@ -570,7 +570,6 @@ const struct inode_operations nilfs_special_inode_operations = { const struct inode_operations nilfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .permission = nilfs_permission, }; diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c index b4e79bc..6c2a3e3 100644 --- a/fs/ocfs2/symlink.c +++ b/fs/ocfs2/symlink.c @@ -89,7 +89,6 @@ const struct address_space_operations ocfs2_fast_symlink_aops = { const struct inode_operations ocfs2_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .getattr = ocfs2_getattr, .setattr = ocfs2_setattr, .setxattr = generic_setxattr, diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c index 38a0b8b..964a60f 100644 --- a/fs/overlayfs/inode.c +++ b/fs/overlayfs/inode.c @@ -131,19 +131,12 @@ out_dput: return err; } - -struct ovl_link_data { - struct dentry *realdentry; - void *cookie; -}; - static const char *ovl_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct dentry *realdentry; struct inode *realinode; - struct ovl_link_data *data = NULL; - const char *ret; if (!dentry) return ERR_PTR(-ECHILD); @@ -154,38 +147,7 @@ static const char *ovl_get_link(struct dentry *dentry, if (WARN_ON(!realinode->i_op->get_link)) return ERR_PTR(-EPERM); - if (realinode->i_op->put_link) { - data = kmalloc(sizeof(struct ovl_link_data), GFP_KERNEL); - if (!data) - return ERR_PTR(-ENOMEM); - data->realdentry = realdentry; - } - - ret = realinode->i_op->get_link(realdentry, realinode, cookie); - if (IS_ERR_OR_NULL(ret)) { - kfree(data); - return ret; - } - - if (data) - data->cookie = *cookie; - - *cookie = data; - - return ret; -} - -static void ovl_put_link(struct inode *unused, void *c) -{ - struct inode *realinode; - struct ovl_link_data *data = c; - - if (!data) - return; - - realinode = data->realdentry->d_inode; - realinode->i_op->put_link(realinode, data->cookie); - kfree(data); + return realinode->i_op->get_link(realdentry, realinode, done); } static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz) @@ -383,7 +345,6 @@ static const struct inode_operations ovl_file_inode_operations = { static const struct inode_operations ovl_symlink_inode_operations = { .setattr = ovl_setattr, .get_link = ovl_get_link, - .put_link = ovl_put_link, .readlink = ovl_readlink, .getattr = ovl_getattr, .setxattr = ovl_setxattr, diff --git a/fs/proc/base.c b/fs/proc/base.c index 1a489e2..71660bb 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1565,7 +1565,8 @@ static int proc_exe_link(struct dentry *dentry, struct path *exe_path) } static const char *proc_pid_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct path path; int error = -EACCES; @@ -1949,12 +1950,13 @@ struct map_files_info { */ static const char * proc_map_files_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { if (!capable(CAP_SYS_ADMIN)) return ERR_PTR(-EPERM); - return proc_pid_get_link(dentry, inode, NULL); + return proc_pid_get_link(dentry, inode, done); } /* diff --git a/fs/proc/inode.c b/fs/proc/inode.c index 10360b2..d0e9b9b 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c @@ -393,25 +393,25 @@ static const struct file_operations proc_reg_file_ops_no_compat = { }; #endif +static void proc_put_link(void *p) +{ + unuse_pde(p); +} + static const char *proc_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct proc_dir_entry *pde = PDE(inode); if (unlikely(!use_pde(pde))) return ERR_PTR(-EINVAL); - *cookie = pde; + set_delayed_call(done, proc_put_link, pde); return pde->data; } -static void proc_put_link(struct inode *unused, void *p) -{ - unuse_pde(p); -} - const struct inode_operations proc_link_inode_operations = { .readlink = generic_readlink, .get_link = proc_get_link, - .put_link = proc_put_link, }; struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de) diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index 63861c1..1dece87 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -31,7 +31,8 @@ static const struct proc_ns_operations *ns_entries[] = { }; static const char *proc_ns_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { const struct proc_ns_operations *ns_ops = PROC_I(inode)->ns_ops; struct task_struct *task; diff --git a/fs/proc/self.c b/fs/proc/self.c index 7a8b19e..67e8db4 100644 --- a/fs/proc/self.c +++ b/fs/proc/self.c @@ -19,7 +19,8 @@ static int proc_self_readlink(struct dentry *dentry, char __user *buffer, } static const char *proc_self_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct pid_namespace *ns = inode->i_sb->s_fs_info; pid_t tgid = task_tgid_nr_ns(current, ns); @@ -32,13 +33,13 @@ static const char *proc_self_get_link(struct dentry *dentry, if (unlikely(!name)) return dentry ? ERR_PTR(-ENOMEM) : ERR_PTR(-ECHILD); sprintf(name, "%d", tgid); - return *cookie = name; + set_delayed_call(done, kfree_link, name); + return name; } static const struct inode_operations proc_self_inode_operations = { .readlink = proc_self_readlink, .get_link = proc_self_get_link, - .put_link = kfree_put_link, }; static unsigned self_inum; diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c index 03eaa84..9eacd59 100644 --- a/fs/proc/thread_self.c +++ b/fs/proc/thread_self.c @@ -20,7 +20,8 @@ static int proc_thread_self_readlink(struct dentry *dentry, char __user *buffer, } static const char *proc_thread_self_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct pid_namespace *ns = inode->i_sb->s_fs_info; pid_t tgid = task_tgid_nr_ns(current, ns); @@ -34,13 +35,13 @@ static const char *proc_thread_self_get_link(struct dentry *dentry, if (unlikely(!name)) return dentry ? ERR_PTR(-ENOMEM) : ERR_PTR(-ECHILD); sprintf(name, "%d/task/%d", tgid, pid); - return *cookie = name; + set_delayed_call(done, kfree_link, name); + return name; } static const struct inode_operations proc_thread_self_inode_operations = { .readlink = proc_thread_self_readlink, .get_link = proc_thread_self_get_link, - .put_link = kfree_put_link, }; static unsigned thread_self_inum; diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c index ecbf11e..2a12d46 100644 --- a/fs/reiserfs/namei.c +++ b/fs/reiserfs/namei.c @@ -1666,7 +1666,6 @@ const struct inode_operations reiserfs_dir_inode_operations = { const struct inode_operations reiserfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .setattr = reiserfs_setattr, .setxattr = reiserfs_setxattr, .getxattr = reiserfs_getxattr, diff --git a/fs/squashfs/symlink.c b/fs/squashfs/symlink.c index 7c635a5..dbcc2f5 100644 --- a/fs/squashfs/symlink.c +++ b/fs/squashfs/symlink.c @@ -120,7 +120,6 @@ const struct address_space_operations squashfs_symlink_aops = { const struct inode_operations squashfs_symlink_inode_ops = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .getxattr = generic_getxattr, .listxattr = squashfs_listxattr }; diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c index 80a40bc..07ac18c 100644 --- a/fs/sysv/inode.c +++ b/fs/sysv/inode.c @@ -147,7 +147,6 @@ static inline void write3byte(struct sysv_sb_info *sbi, static const struct inode_operations sysv_symlink_inode_operations = { .readlink = generic_readlink, .get_link = page_get_link, - .put_link = page_put_link, .getattr = sysv_getattr, }; diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index f638fd5..06eafaf 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -417,7 +417,7 @@ STATIC const char * xfs_vn_get_link( struct dentry *dentry, struct inode *inode, - void **cookie) + struct delayed_call *done) { char *link; int error = -ENOMEM; @@ -433,7 +433,8 @@ xfs_vn_get_link( if (unlikely(error)) goto out_kfree; - return *cookie = link; + set_delayed_call(done, kfree_link, link); + return link; out_kfree: kfree(link); @@ -1177,7 +1178,6 @@ static const struct inode_operations xfs_dir_ci_inode_operations = { static const struct inode_operations xfs_symlink_inode_operations = { .readlink = generic_readlink, .get_link = xfs_vn_get_link, - .put_link = kfree_put_link, .getattr = xfs_vn_getattr, .setattr = xfs_vn_setattr, .setxattr = generic_setxattr, diff --git a/include/linux/fs.h b/include/linux/fs.h index b18c020..351cfb2 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1635,14 +1635,34 @@ struct file_operations { u64); }; +struct delayed_call { + void (*fn)(void *); + void *arg; +}; + +#define DEFINE_DELAYED_CALL(name) struct delayed_call name = {NULL, NULL} + +/* I really wish we had closures with sane typechecking... */ +static inline void set_delayed_call(struct delayed_call *call, + void (*fn)(void *), void *arg) +{ + call->fn = fn; + call->arg = arg; +} + +static inline void do_delayed_call(struct delayed_call *call) +{ + if (call->fn) + call->fn(call->arg); +} + struct inode_operations { struct dentry * (*lookup) (struct inode *,struct dentry *, unsigned int); - const char * (*get_link) (struct dentry *, struct inode *, void **); + const char * (*get_link) (struct dentry *, struct inode *, struct delayed_call *); int (*permission) (struct inode *, int); struct posix_acl * (*get_acl)(struct inode *, int); int (*readlink) (struct dentry *, char __user *,int); - void (*put_link) (struct inode *, void *); int (*create) (struct inode *,struct dentry *, umode_t, bool); int (*link) (struct dentry *,struct inode *,struct dentry *); @@ -2744,14 +2764,15 @@ extern const struct file_operations generic_ro_fops; extern int readlink_copy(char __user *, int, const char *); extern int page_readlink(struct dentry *, char __user *, int); -extern const char *page_get_link(struct dentry *, struct inode *, void **); -extern void page_put_link(struct inode *, void *); +extern const char *page_get_link(struct dentry *, struct inode *, + struct delayed_call *); +extern void page_put_link(void *); extern int __page_symlink(struct inode *inode, const char *symname, int len, int nofs); extern int page_symlink(struct inode *inode, const char *symname, int len); extern const struct inode_operations page_symlink_inode_operations; -extern void kfree_put_link(struct inode *, void *); -extern void free_page_put_link(struct inode *, void *); +extern void kfree_link(void *); +extern void free_page_link(void *); extern int generic_readlink(struct dentry *, char __user *, int); extern void generic_fillattr(struct inode *, struct kstat *); int vfs_getattr_nosec(struct path *path, struct kstat *stat); @@ -2762,7 +2783,8 @@ void __inode_sub_bytes(struct inode *inode, loff_t bytes); void inode_sub_bytes(struct inode *inode, loff_t bytes); loff_t inode_get_bytes(struct inode *inode); void inode_set_bytes(struct inode *inode, loff_t bytes); -const char *simple_get_link(struct dentry *, struct inode *, void **); +const char *simple_get_link(struct dentry *, struct inode *, + struct delayed_call *); extern const struct inode_operations simple_symlink_inode_operations; extern int iterate_dir(struct file *, struct dir_context *); diff --git a/mm/shmem.c b/mm/shmem.c index 5e43f1f..728f67a 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -2496,8 +2496,15 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s return 0; } +static void shmem_put_link(void *arg) +{ + mark_page_accessed(arg); + put_page(arg); +} + static const char *shmem_get_link(struct dentry *dentry, - struct inode *inode, void **cookie) + struct inode *inode, + struct delayed_call *done) { struct page *page = NULL; int error; @@ -2515,17 +2522,10 @@ static const char *shmem_get_link(struct dentry *dentry, return ERR_PTR(error); unlock_page(page); } - *cookie = page; + set_delayed_call(done, shmem_put_link, page); return page_address(page); } -static void shmem_put_link(struct inode *unused, void *cookie) -{ - struct page *page = cookie; - mark_page_accessed(page); - page_cache_release(page); -} - #ifdef CONFIG_TMPFS_XATTR /* * Superblocks without xattr inode operations may get some security.* xattr @@ -2633,7 +2633,6 @@ static const struct inode_operations shmem_short_symlink_operations = { static const struct inode_operations shmem_symlink_inode_operations = { .readlink = generic_readlink, .get_link = shmem_get_link, - .put_link = shmem_put_link, #ifdef CONFIG_TMPFS_XATTR .setxattr = generic_setxattr, .getxattr = generic_getxattr, -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html