From patchwork Thu Nov 9 07:20:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 10050347 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id B42F0601EA for ; Thu, 9 Nov 2017 07:51:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A30F92A7F5 for ; Thu, 9 Nov 2017 07:51:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 924F42AB6C; Thu, 9 Nov 2017 07:51:47 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CAC842A7F5 for ; Thu, 9 Nov 2017 07:51:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752903AbdKIHvO (ORCPT ); Thu, 9 Nov 2017 02:51:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:49089 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752893AbdKIHvM (ORCPT ); Thu, 9 Nov 2017 02:51:12 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id CA5ADAC16; Thu, 9 Nov 2017 07:51:10 +0000 (UTC) From: NeilBrown To: Ian Kent , Latchesar Ionkov , Jeff Layton , Eric Van Hensbergen , Al Viro , Ron Minnich , Trond Myklebust Date: Thu, 09 Nov 2017 18:20:24 +1100 Subject: [PATCH 3/3] VFS / autofs4: remove kern_path_mountpoint() Cc: linux-nfs@vger.kernel.org, autofs@vger.kernel.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, v9fs-developer@lists.sourceforge.net, Linus Torvalds Message-ID: <151021202428.22743.11071460838655377498.stgit@noble> In-Reply-To: <151021179901.22743.15252956909042161062.stgit@noble> References: <151021179901.22743.15252956909042161062.stgit@noble> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP kern_path_mountpoint() is only called from autofs4 to perform lookups which need to identify autofs4 mount points. Many of the differences between kern_path() and kern_path_mountpoint() are related to the fact that we will never use O_CREAT with the latter, and don't need to "open" the target. The main differences that could be relevant to autofs4 are: - kern_path_mountpoint() does not call complete_walk() in mountpoint_last(), contrasting with do_last() which does call it. This means ->d_weak_revalidate() is not called from autofs4. - follow_managed() is not call from mountpoint_last(). - LOOKUP_NO_REVAL is used for lookup_slow on the last component, if it isn't in cache. As ->d_weak_revalidate() is now a no-op when LOOKUP_OPEN isn't present, the first difference is no longer important. The use of LOOKUP_NO_REVAL shouldn't cause autofs4 any problems as no autofs4 dentry has ->d_revalidate(). follow_managed() might: a/ call ->d_manage() b/ might cross a mountpoint c/ might call follow_automount() 'b' cannot be relevant as path_mountpoint calls follow_mount() after mountpoint_last() is called. 'a' might only be interesting when ->d_manage is autofs4_d_manage(), but autofs4 only calls kern_path_mountpoint from ioctls issued by the automount daemon, and autofs4_d_manage() will exit quickly in that case. So there is no risk of autofs4_d_manage() waiting for the automount daemon (which it would be blocking) and causing a deadlock. 'c' could have been a problem before commit 42f461482178 ("autofs: fix AT_NO_AUTOMOUNT not being honored"). Prior to that commit a lookup for a negative autofs4 dentry could trigger an automount, even though 'flags' is 0. Since that commit and error is returned instead. So follow_managed() is no longer a problem. So there is no reason that autofs4 needs to use kern_path_mountpoint() any more. It cannot deadlock. So the whole 'path mountpoint' infrastructure can be discarded. Signed-off-by: NeilBrown --- fs/autofs4/dev-ioctl.c | 5 +- fs/namei.c | 129 ------------------------------------------------ include/linux/namei.h | 1 3 files changed, 2 insertions(+), 133 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c index b7c816f39404..716c44593117 100644 --- a/fs/autofs4/dev-ioctl.c +++ b/fs/autofs4/dev-ioctl.c @@ -209,7 +209,7 @@ static int find_autofs_mount(const char *pathname, struct path path; int err; - err = kern_path_mountpoint(AT_FDCWD, pathname, &path, 0); + err = kern_path(pathname,0 , &path); if (err) return err; err = -ENOENT; @@ -547,8 +547,7 @@ static int autofs_dev_ioctl_ismountpoint(struct file *fp, if (!fp || param->ioctlfd == -1) { if (autofs_type_any(type)) - err = kern_path_mountpoint(AT_FDCWD, - name, &path, LOOKUP_FOLLOW); + err = kern_path(name, LOOKUP_FOLLOW, &path); else err = find_autofs_mount(name, &path, test_by_type, &type); diff --git a/fs/namei.c b/fs/namei.c index 6639203d7eba..e90680a3f6f1 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2601,135 +2601,6 @@ int user_path_at_empty(int dfd, const char __user *name, unsigned flags, } EXPORT_SYMBOL(user_path_at_empty); -/** - * mountpoint_last - look up last component for umount - * @nd: pathwalk nameidata - currently pointing at parent directory of "last" - * - * This is a special lookup_last function just for umount. In this case, we - * need to resolve the path without doing any revalidation. - * - * The nameidata should be the result of doing a LOOKUP_PARENT pathwalk. Since - * mountpoints are always pinned in the dcache, their ancestors are too. Thus, - * in almost all cases, this lookup will be served out of the dcache. The only - * cases where it won't are if nd->last refers to a symlink or the path is - * bogus and it doesn't exist. - * - * Returns: - * -error: if there was an error during lookup. This includes -ENOENT if the - * lookup found a negative dentry. - * - * 0: if we successfully resolved nd->last and found it to not to be a - * symlink that needs to be followed. - * - * 1: if we successfully resolved nd->last and found it to be a symlink - * that needs to be followed. - */ -static int -mountpoint_last(struct nameidata *nd) -{ - int error = 0; - struct dentry *dir = nd->path.dentry; - struct path path; - - /* If we're in rcuwalk, drop out of it to handle last component */ - if (nd->flags & LOOKUP_RCU) { - if (unlazy_walk(nd)) - return -ECHILD; - } - - nd->flags &= ~LOOKUP_PARENT; - - if (unlikely(nd->last_type != LAST_NORM)) { - error = handle_dots(nd, nd->last_type); - if (error) - return error; - path.dentry = dget(nd->path.dentry); - } else { - path.dentry = d_lookup(dir, &nd->last); - if (!path.dentry) { - /* - * No cached dentry. Mounted dentries are pinned in the - * cache, so that means that this dentry is probably - * a symlink or the path doesn't actually point - * to a mounted dentry. - */ - path.dentry = lookup_slow(&nd->last, dir, - nd->flags | LOOKUP_NO_REVAL); - if (IS_ERR(path.dentry)) - return PTR_ERR(path.dentry); - } - } - if (d_is_negative(path.dentry)) { - dput(path.dentry); - return -ENOENT; - } - path.mnt = nd->path.mnt; - return step_into(nd, &path, 0, d_backing_inode(path.dentry), 0); -} - -/** - * path_mountpoint - look up a path to be umounted - * @nd: lookup context - * @flags: lookup flags - * @path: pointer to container for result - * - * Look up the given name, but don't attempt to revalidate the last component. - * Returns 0 and "path" will be valid on success; Returns error otherwise. - */ -static int -path_mountpoint(struct nameidata *nd, unsigned flags, struct path *path) -{ - const char *s = path_init(nd, flags); - int err; - if (IS_ERR(s)) - return PTR_ERR(s); - while (!(err = link_path_walk(s, nd)) && - (err = mountpoint_last(nd)) > 0) { - s = trailing_symlink(nd); - if (IS_ERR(s)) { - err = PTR_ERR(s); - break; - } - } - if (!err) { - *path = nd->path; - nd->path.mnt = NULL; - nd->path.dentry = NULL; - follow_mount(path); - } - terminate_walk(nd); - return err; -} - -static int -filename_mountpoint(int dfd, struct filename *name, struct path *path, - unsigned int flags) -{ - struct nameidata nd; - int error; - if (IS_ERR(name)) - return PTR_ERR(name); - set_nameidata(&nd, dfd, name); - error = path_mountpoint(&nd, flags | LOOKUP_RCU, path); - if (unlikely(error == -ECHILD)) - error = path_mountpoint(&nd, flags, path); - if (unlikely(error == -ESTALE)) - error = path_mountpoint(&nd, flags | LOOKUP_REVAL, path); - if (likely(!error)) - audit_inode(name, path->dentry, 0); - restore_nameidata(); - putname(name); - return error; -} - -int -kern_path_mountpoint(int dfd, const char *name, struct path *path, - unsigned int flags) -{ - return filename_mountpoint(dfd, getname_kernel(name), path, flags); -} -EXPORT_SYMBOL(kern_path_mountpoint); - int __check_sticky(struct inode *dir, struct inode *inode) { kuid_t fsuid = current_fsuid(); diff --git a/include/linux/namei.h b/include/linux/namei.h index a982bb7cd480..e576bf40d761 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -79,7 +79,6 @@ extern struct dentry *kern_path_create(int, const char *, struct path *, unsigne extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int); extern void done_path_create(struct path *, struct dentry *); extern struct dentry *kern_path_locked(const char *, struct path *); -extern int kern_path_mountpoint(int, const char *, struct path *, unsigned int); extern struct dentry *lookup_one_len(const char *, struct dentry *, int); extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);