From patchwork Fri Jul 16 09:28:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12381809 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8ACFAC07E95 for ; Fri, 16 Jul 2021 09:37:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6BE00613D8 for ; Fri, 16 Jul 2021 09:37:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233772AbhGPJk1 (ORCPT ); Fri, 16 Jul 2021 05:40:27 -0400 Received: from icp-osb-irony-out9.external.iinet.net.au ([203.59.1.226]:22594 "EHLO icp-osb-irony-out9.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229833AbhGPJk0 (ORCPT ); Fri, 16 Jul 2021 05:40:26 -0400 X-Greylist: delayed 556 seconds by postgrey-1.27 at vger.kernel.org; Fri, 16 Jul 2021 05:40:26 EDT X-SMTP-MATCH: 0 IronPort-HdrOrdr: A9a23:TyZQ4a2U2q0zHeqTam8V0QqjBLIkLtp133Aq2lEZdPRUGvb3qynIpoV+6faUskd1ZJhOo7290cW7LU80lqQFg7X5X43SOzUO0VHAROoJ0WKL+UyHJ8SUzI5gPMlbHJSWcOeAbmSS9vya3DWF X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AdBwA6UPFg/y2ELHlQCh4BAQsSDEAJgUULAoN2bIQCRpATAQEBAQEBBowphW9fiikUgWgLAQEBAQEBAQEBSgQBAYRUAoJ8ASU0CQ4CBBUBAQEFAQEBAQEGAwGBDoV1QwEMAYV1BiMEUhAYDQIRBw4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgUgDgy2DfRpsgliCZASDGIEPNHyBD5UMAUaKGVudDIMunmQXlU8DkHWWCKcOghRNLgqDJFAZjjkWjjo3LzgCBgoBAQMJgnKHIieCHgEB X-IPAS-Result: A2AdBwA6UPFg/y2ELHlQCh4BAQsSDEAJgUULAoN2bIQCRpATAQEBAQEBBowphW9fiikUgWgLAQEBAQEBAQEBSgQBAYRUAoJ8ASU0CQ4CBBUBAQEFAQEBAQEGAwGBDoV1QwEMAYV1BiMEUhAYDQIRBw4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgUgDgy2DfRpsgliCZASDGIEPNHyBD5UMAUaKGVudDIMunmQXlU8DkHWWCKcOghRNLgqDJFAZjjkWjjo3LzgCBgoBAQMJgnKHIieCHgEB X-IronPort-AV: E=Sophos;i="5.84,244,1620662400"; d="scan'208";a="329873560" Received: from unknown (HELO web.messagingengine.com) ([121.44.132.45]) by icp-osb-irony-out9.iinet.net.au with ESMTP; 16 Jul 2021 17:28:19 +0800 Subject: [PATCH v8 1/5] kernfs: add a revision to identify directory node changes From: Ian Kent To: Greg Kroah-Hartman , Tejun Heo Cc: Eric Sandeen , Fox Chen , Brice Goglin , Al Viro , Rick Lindsley , David Howells , Miklos Szeredi , Marcelo Tosatti , "Eric W. Biederman" , Carlos Maiolino , linux-fsdevel , Kernel Mailing List Date: Fri, 16 Jul 2021 17:28:18 +0800 Message-ID: <162642769895.63632.8356662784964509867.stgit@web.messagingengine.com> In-Reply-To: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> References: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Add a revision counter to kernfs directory nodes so it can be used to detect if a directory node has changed during negative dentry revalidation. There's an assumption that sizeof(unsigned long) <= sizeof(pointer) on all architectures and as far as I know that assumption holds. So adding a revision counter to the struct kernfs_elem_dir variant of the kernfs_node type union won't increase the size of the kernfs_node struct. This is because struct kernfs_elem_dir is at least sizeof(pointer) smaller than the largest union variant. It's tempting to make the revision counter a u64 but that would increase the size of kernfs_node on archs where sizeof(pointer) is smaller than the revision counter. Signed-off-by: Ian Kent Reviewed-by: Miklos Szeredi --- fs/kernfs/dir.c | 2 ++ fs/kernfs/kernfs-internal.h | 19 +++++++++++++++++++ include/linux/kernfs.h | 5 +++++ 3 files changed, 26 insertions(+) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 33166ec90a11..b3d1bc0f317d 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -372,6 +372,7 @@ static int kernfs_link_sibling(struct kernfs_node *kn) /* successfully added, account subdir number */ if (kernfs_type(kn) == KERNFS_DIR) kn->parent->dir.subdirs++; + kernfs_inc_rev(kn->parent); return 0; } @@ -394,6 +395,7 @@ static bool kernfs_unlink_sibling(struct kernfs_node *kn) if (kernfs_type(kn) == KERNFS_DIR) kn->parent->dir.subdirs--; + kernfs_inc_rev(kn->parent); rb_erase(&kn->rb, &kn->parent->dir.children); RB_CLEAR_NODE(&kn->rb); diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h index ccc3b44f6306..c2ae58f3b202 100644 --- a/fs/kernfs/kernfs-internal.h +++ b/fs/kernfs/kernfs-internal.h @@ -81,6 +81,25 @@ static inline struct kernfs_node *kernfs_dentry_node(struct dentry *dentry) return d_inode(dentry)->i_private; } +static inline void kernfs_set_rev(struct kernfs_node *parent, + struct dentry *dentry) +{ + dentry->d_time = parent->dir.rev; +} + +static inline void kernfs_inc_rev(struct kernfs_node *parent) +{ + parent->dir.rev++; +} + +static inline bool kernfs_dir_changed(struct kernfs_node *parent, + struct dentry *dentry) +{ + if (parent->dir.rev != dentry->d_time) + return true; + return false; +} + extern const struct super_operations kernfs_sops; extern struct kmem_cache *kernfs_node_cache, *kernfs_iattrs_cache; diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index 9e8ca8743c26..d68b4ad09573 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -98,6 +98,11 @@ struct kernfs_elem_dir { * better directly in kernfs_node but is here to save space. */ struct kernfs_root *root; + /* + * Monotonic revision counter, used to identify if a directory + * node has changed during negative dentry revalidation. + */ + unsigned long rev; }; struct kernfs_elem_symlink { From patchwork Fri Jul 16 09:28:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12381813 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE7E0C636CA for ; Fri, 16 Jul 2021 09:37:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 952F6613D8 for ; Fri, 16 Jul 2021 09:37:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234249AbhGPJkf (ORCPT ); Fri, 16 Jul 2021 05:40:35 -0400 Received: from icp-osb-irony-out9.external.iinet.net.au ([203.59.1.226]:22594 "EHLO icp-osb-irony-out9.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233986AbhGPJkd (ORCPT ); Fri, 16 Jul 2021 05:40:33 -0400 X-Greylist: delayed 556 seconds by postgrey-1.27 at vger.kernel.org; Fri, 16 Jul 2021 05:40:26 EDT X-SMTP-MATCH: 0 IronPort-HdrOrdr: A9a23:HCUZiq2XJJWX1INLK/FgJQqjBIgkLtp133Aq2lEZdPU1SKClfqWV98jzuiWatN98Yh8dcLK7WJVoMEm8yXcd2+B4V9qftW/dyQmVxepZnOjfKlPbakrD398Y+aB8c7VvTP3cZGIK6/oSOTPIdurIFuP3lJyVuQ== X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AbBwA6UPFg/y2ELHlQCh4BAQsSDEAJgUULAoN2bIQCRpATAQEBAQEBBoFAimmFb1+KKYF8CwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBDAGFdQYjBFIQGA0CGA4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgRUzA4EFgiiEF4NEgmQEglJFAYEOFIFkSJIRgnsBRohtgSxbnQyDLp5kF5VPA5B1hwePAacOghRNLgqDJFAZjjgBFo46Ny84AgYKAQEDCYJyhyItghgBAQ X-IPAS-Result: A2AbBwA6UPFg/y2ELHlQCh4BAQsSDEAJgUULAoN2bIQCRpATAQEBAQEBBoFAimmFb1+KKYF8CwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBDAGFdQYjBFIQGA0CGA4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgRUzA4EFgiiEF4NEgmQEglJFAYEOFIFkSJIRgnsBRohtgSxbnQyDLp5kF5VPA5B1hwePAacOghRNLgqDJFAZjjgBFo46Ny84AgYKAQEDCYJyhyItghgBAQ X-IronPort-AV: E=Sophos;i="5.84,244,1620662400"; d="scan'208";a="329873575" Received: from unknown (HELO web.messagingengine.com) ([121.44.132.45]) by icp-osb-irony-out9.iinet.net.au with ESMTP; 16 Jul 2021 17:28:24 +0800 Subject: [PATCH v8 2/5] kernfs: use VFS negative dentry caching From: Ian Kent To: Greg Kroah-Hartman , Tejun Heo Cc: Eric Sandeen , Fox Chen , Brice Goglin , Al Viro , Rick Lindsley , David Howells , Miklos Szeredi , Marcelo Tosatti , "Eric W. Biederman" , Carlos Maiolino , linux-fsdevel , Kernel Mailing List Date: Fri, 16 Jul 2021 17:28:24 +0800 Message-ID: <162642770420.63632.15791924970508867106.stgit@web.messagingengine.com> In-Reply-To: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> References: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org If there are many lookups for non-existent paths these negative lookups can lead to a lot of overhead during path walks. The VFS allows dentries to be created as negative and hashed, and caches them so they can be used to reduce the fairly high overhead alloc/free cycle that occurs during these lookups. Use the kernfs node parent revision to identify if a change has been made to the containing directory so that the negative dentry can be discarded and the lookup redone. Signed-off-by: Ian Kent Reviewed-by: Miklos Szeredi --- fs/kernfs/dir.c | 55 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index b3d1bc0f317d..0b21a8f961ac 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -1039,9 +1039,31 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) if (flags & LOOKUP_RCU) return -ECHILD; - /* Always perform fresh lookup for negatives */ - if (d_really_is_negative(dentry)) - goto out_bad_unlocked; + /* Negative hashed dentry? */ + if (d_really_is_negative(dentry)) { + struct kernfs_node *parent; + + /* If the kernfs parent node has changed discard and + * proceed to ->lookup. + */ + mutex_lock(&kernfs_mutex); + spin_lock(&dentry->d_lock); + parent = kernfs_dentry_node(dentry->d_parent); + if (parent) { + if (kernfs_dir_changed(parent, dentry)) { + spin_unlock(&dentry->d_lock); + mutex_unlock(&kernfs_mutex); + return 0; + } + } + spin_unlock(&dentry->d_lock); + mutex_unlock(&kernfs_mutex); + + /* The kernfs parent node hasn't changed, leave the + * dentry negative and return success. + */ + return 1; + } kn = kernfs_dentry_node(dentry); mutex_lock(&kernfs_mutex); @@ -1067,7 +1089,6 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) return 1; out_bad: mutex_unlock(&kernfs_mutex); -out_bad_unlocked: return 0; } @@ -1082,33 +1103,27 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir, struct dentry *ret; struct kernfs_node *parent = dir->i_private; struct kernfs_node *kn; - struct inode *inode; + struct inode *inode = NULL; const void *ns = NULL; mutex_lock(&kernfs_mutex); - if (kernfs_ns_enabled(parent)) ns = kernfs_info(dir->i_sb)->ns; kn = kernfs_find_ns(parent, dentry->d_name.name, ns); - - /* no such entry */ - if (!kn || !kernfs_active(kn)) { - ret = NULL; - goto out_unlock; - } - /* attach dentry and inode */ - inode = kernfs_get_inode(dir->i_sb, kn); - if (!inode) { - ret = ERR_PTR(-ENOMEM); - goto out_unlock; + if (kn && kernfs_active(kn)) { + inode = kernfs_get_inode(dir->i_sb, kn); + if (!inode) + inode = ERR_PTR(-ENOMEM); } - - /* instantiate and hash dentry */ + /* Needed only for negative dentry validation */ + if (!inode) + kernfs_set_rev(parent, dentry); + /* instantiate and hash (possibly negative) dentry */ ret = d_splice_alias(inode, dentry); - out_unlock: mutex_unlock(&kernfs_mutex); + return ret; } From patchwork Fri Jul 16 09:28:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12381817 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 88BDAC07E95 for ; Fri, 16 Jul 2021 09:37:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6B680613ED for ; Fri, 16 Jul 2021 09:37:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234429AbhGPJko (ORCPT ); Fri, 16 Jul 2021 05:40:44 -0400 Received: from icp-osb-irony-out9.external.iinet.net.au ([203.59.1.226]:22628 "EHLO icp-osb-irony-out9.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234171AbhGPJkm (ORCPT ); Fri, 16 Jul 2021 05:40:42 -0400 X-SMTP-MATCH: 0 IronPort-HdrOrdr: A9a23:3Psp5Kh+8mu4r5gWjz8DT+hA1XBQXvEji2hC6mlwRA09TyX4raCTdZEgviMc5wx6ZJhNo7290cq7IE80l6Qb3WB5B97LYOCMggWVxe9ZgbcLQ1Xbak7DHm0079YYT0BUY+eAb2STh63BkXGF+1FJ+qjhzEgr7d2uqUuEzGlRGsRdB4IQMHf+LnFL X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2A3DAA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCg3ZshAJGkBMBAQEBAQEGgUCBE4lWhQVqX4opFIFoCwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBAQQHAYV1BiMEUhAYDQIYDgICRxAGARKFUyWnZ38zGgJlgQGJQoEQKgGHCIJog3onHH2BEIEVMwOBBYEzdYQqgzGCZASDFwEWZQkKAX94CSgXY5EugnsBRohtgSxbnQyDLp5kF5VPA5B1lgiffgcfhmqCFE0uCjuCaVAZjjgBFo46Ny84AgYKAQEDCYJyhHqCJiyCGwEB X-IPAS-Result: A2A3DAA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCg3ZshAJGkBMBAQEBAQEGgUCBE4lWhQVqX4opFIFoCwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBAQQHAYV1BiMEUhAYDQIYDgICRxAGARKFUyWnZ38zGgJlgQGJQoEQKgGHCIJog3onHH2BEIEVMwOBBYEzdYQqgzGCZASDFwEWZQkKAX94CSgXY5EugnsBRohtgSxbnQyDLp5kF5VPA5B1lgiffgcfhmqCFE0uCjuCaVAZjjgBFo46Ny84AgYKAQEDCYJyhHqCJiyCGwEB X-IronPort-AV: E=Sophos;i="5.84,244,1620662400"; d="scan'208";a="329873587" Received: from unknown (HELO web.messagingengine.com) ([121.44.132.45]) by icp-osb-irony-out9.iinet.net.au with ESMTP; 16 Jul 2021 17:28:29 +0800 Subject: [PATCH v8 3/5] kernfs: switch kernfs to use an rwsem From: Ian Kent To: Greg Kroah-Hartman , Tejun Heo Cc: Eric Sandeen , Fox Chen , Brice Goglin , Al Viro , Rick Lindsley , David Howells , Miklos Szeredi , Marcelo Tosatti , "Eric W. Biederman" , Carlos Maiolino , linux-fsdevel , Kernel Mailing List Date: Fri, 16 Jul 2021 17:28:29 +0800 Message-ID: <162642770946.63632.2218304587223241374.stgit@web.messagingengine.com> In-Reply-To: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> References: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org The kernfs global lock restricts the ability to perform kernfs node lookup operations in parallel during path walks. Change the kernfs mutex to an rwsem so that, when opportunity arises, node searches can be done in parallel with path walk lookups. Signed-off-by: Ian Kent Reviewed-by: Miklos Szeredi --- fs/kernfs/dir.c | 100 ++++++++++++++++++++++--------------------- fs/kernfs/file.c | 4 +- fs/kernfs/inode.c | 16 +++---- fs/kernfs/kernfs-internal.h | 5 +- fs/kernfs/mount.c | 12 +++-- fs/kernfs/symlink.c | 4 +- include/linux/kernfs.h | 2 - 7 files changed, 72 insertions(+), 71 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 0b21a8f961ac..4994723d6cf7 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -17,7 +17,7 @@ #include "kernfs-internal.h" -DEFINE_MUTEX(kernfs_mutex); +DECLARE_RWSEM(kernfs_rwsem); static DEFINE_SPINLOCK(kernfs_rename_lock); /* kn->parent and ->name */ static char kernfs_pr_cont_buf[PATH_MAX]; /* protected by rename_lock */ static DEFINE_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */ @@ -26,7 +26,7 @@ static DEFINE_SPINLOCK(kernfs_idr_lock); /* root->ino_idr */ static bool kernfs_active(struct kernfs_node *kn) { - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held(&kernfs_rwsem); return atomic_read(&kn->active) >= 0; } @@ -340,7 +340,7 @@ static int kernfs_sd_compare(const struct kernfs_node *left, * @kn->parent->dir.children. * * Locking: - * mutex_lock(kernfs_mutex) + * kernfs_rwsem held exclusive * * RETURNS: * 0 on susccess -EEXIST on failure. @@ -386,7 +386,7 @@ static int kernfs_link_sibling(struct kernfs_node *kn) * removed, %false if @kn wasn't on the rbtree. * * Locking: - * mutex_lock(kernfs_mutex) + * kernfs_rwsem held exclusive */ static bool kernfs_unlink_sibling(struct kernfs_node *kn) { @@ -457,14 +457,14 @@ void kernfs_put_active(struct kernfs_node *kn) * return after draining is complete. */ static void kernfs_drain(struct kernfs_node *kn) - __releases(&kernfs_mutex) __acquires(&kernfs_mutex) + __releases(&kernfs_rwsem) __acquires(&kernfs_rwsem) { struct kernfs_root *root = kernfs_root(kn); - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held_write(&kernfs_rwsem); WARN_ON_ONCE(kernfs_active(kn)); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); if (kernfs_lockdep(kn)) { rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_); @@ -483,7 +483,7 @@ static void kernfs_drain(struct kernfs_node *kn) kernfs_drain_open_files(kn); - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); } /** @@ -722,7 +722,7 @@ int kernfs_add_one(struct kernfs_node *kn) bool has_ns; int ret; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); ret = -EINVAL; has_ns = kernfs_ns_enabled(parent); @@ -753,7 +753,7 @@ int kernfs_add_one(struct kernfs_node *kn) ps_iattr->ia_mtime = ps_iattr->ia_ctime; } - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); /* * Activate the new node unless CREATE_DEACTIVATED is requested. @@ -767,7 +767,7 @@ int kernfs_add_one(struct kernfs_node *kn) return 0; out_unlock: - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return ret; } @@ -788,7 +788,7 @@ static struct kernfs_node *kernfs_find_ns(struct kernfs_node *parent, bool has_ns = kernfs_ns_enabled(parent); unsigned int hash; - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held(&kernfs_rwsem); if (has_ns != (bool)ns) { WARN(1, KERN_WARNING "kernfs: ns %s in '%s' for '%s'\n", @@ -820,7 +820,7 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent, size_t len; char *p, *name; - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held_read(&kernfs_rwsem); /* grab kernfs_rename_lock to piggy back on kernfs_pr_cont_buf */ spin_lock_irq(&kernfs_rename_lock); @@ -860,10 +860,10 @@ struct kernfs_node *kernfs_find_and_get_ns(struct kernfs_node *parent, { struct kernfs_node *kn; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); kn = kernfs_find_ns(parent, name, ns); kernfs_get(kn); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return kn; } @@ -884,10 +884,10 @@ struct kernfs_node *kernfs_walk_and_get_ns(struct kernfs_node *parent, { struct kernfs_node *kn; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); kn = kernfs_walk_ns(parent, path, ns); kernfs_get(kn); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return kn; } @@ -1046,18 +1046,18 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) /* If the kernfs parent node has changed discard and * proceed to ->lookup. */ - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); spin_lock(&dentry->d_lock); parent = kernfs_dentry_node(dentry->d_parent); if (parent) { if (kernfs_dir_changed(parent, dentry)) { spin_unlock(&dentry->d_lock); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return 0; } } spin_unlock(&dentry->d_lock); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); /* The kernfs parent node hasn't changed, leave the * dentry negative and return success. @@ -1066,7 +1066,7 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) } kn = kernfs_dentry_node(dentry); - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); /* The kernfs node has been deactivated */ if (!kernfs_active(kn)) @@ -1085,10 +1085,10 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) kernfs_info(dentry->d_sb)->ns != kn->ns) goto out_bad; - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return 1; out_bad: - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return 0; } @@ -1106,7 +1106,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir, struct inode *inode = NULL; const void *ns = NULL; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); if (kernfs_ns_enabled(parent)) ns = kernfs_info(dir->i_sb)->ns; @@ -1122,7 +1122,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir, kernfs_set_rev(parent, dentry); /* instantiate and hash (possibly negative) dentry */ ret = d_splice_alias(inode, dentry); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return ret; } @@ -1244,7 +1244,7 @@ static struct kernfs_node *kernfs_next_descendant_post(struct kernfs_node *pos, { struct rb_node *rbn; - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held_write(&kernfs_rwsem); /* if first iteration, visit leftmost descendant which may be root */ if (!pos) @@ -1280,7 +1280,7 @@ void kernfs_activate(struct kernfs_node *kn) { struct kernfs_node *pos; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); pos = NULL; while ((pos = kernfs_next_descendant_post(pos, kn))) { @@ -1294,14 +1294,14 @@ void kernfs_activate(struct kernfs_node *kn) pos->flags |= KERNFS_ACTIVATED; } - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); } static void __kernfs_remove(struct kernfs_node *kn) { struct kernfs_node *pos; - lockdep_assert_held(&kernfs_mutex); + lockdep_assert_held_write(&kernfs_rwsem); /* * Short-circuit if non-root @kn has already finished removal. @@ -1324,7 +1324,7 @@ static void __kernfs_remove(struct kernfs_node *kn) pos = kernfs_leftmost_descendant(kn); /* - * kernfs_drain() drops kernfs_mutex temporarily and @pos's + * kernfs_drain() drops kernfs_rwsem temporarily and @pos's * base ref could have been put by someone else by the time * the function returns. Make sure it doesn't go away * underneath us. @@ -1371,9 +1371,9 @@ static void __kernfs_remove(struct kernfs_node *kn) */ void kernfs_remove(struct kernfs_node *kn) { - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); __kernfs_remove(kn); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); } /** @@ -1460,17 +1460,17 @@ bool kernfs_remove_self(struct kernfs_node *kn) { bool ret; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); kernfs_break_active_protection(kn); /* * SUICIDAL is used to arbitrate among competing invocations. Only * the first one will actually perform removal. When the removal * is complete, SUICIDED is set and the active ref is restored - * while holding kernfs_mutex. The ones which lost arbitration - * waits for SUICDED && drained which can happen only after the - * enclosing kernfs operation which executed the winning instance - * of kernfs_remove_self() finished. + * while kernfs_rwsem for held exclusive. The ones which lost + * arbitration waits for SUICIDED && drained which can happen only + * after the enclosing kernfs operation which executed the winning + * instance of kernfs_remove_self() finished. */ if (!(kn->flags & KERNFS_SUICIDAL)) { kn->flags |= KERNFS_SUICIDAL; @@ -1488,9 +1488,9 @@ bool kernfs_remove_self(struct kernfs_node *kn) atomic_read(&kn->active) == KN_DEACTIVATED_BIAS) break; - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); schedule(); - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); } finish_wait(waitq, &wait); WARN_ON_ONCE(!RB_EMPTY_NODE(&kn->rb)); @@ -1498,12 +1498,12 @@ bool kernfs_remove_self(struct kernfs_node *kn) } /* - * This must be done while holding kernfs_mutex; otherwise, waiting - * for SUICIDED && deactivated could finish prematurely. + * This must be done while kernfs_rwsem held exclusive; otherwise, + * waiting for SUICIDED && deactivated could finish prematurely. */ kernfs_unbreak_active_protection(kn); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return ret; } @@ -1527,13 +1527,13 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name, return -ENOENT; } - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); kn = kernfs_find_ns(parent, name, ns); if (kn) __kernfs_remove(kn); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); if (kn) return 0; @@ -1559,7 +1559,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, if (!kn->parent) return -EINVAL; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); error = -ENOENT; if (!kernfs_active(kn) || !kernfs_active(new_parent) || @@ -1613,7 +1613,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent, error = 0; out: - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return error; } @@ -1688,7 +1688,7 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx) if (!dir_emit_dots(file, ctx)) return 0; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); if (kernfs_ns_enabled(parent)) ns = kernfs_info(dentry->d_sb)->ns; @@ -1705,12 +1705,12 @@ static int kernfs_fop_readdir(struct file *file, struct dir_context *ctx) file->private_data = pos; kernfs_get(pos); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); if (!dir_emit(ctx, name, len, ino, type)) return 0; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); } - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); file->private_data = NULL; ctx->pos = INT_MAX; return 0; diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index c75719312147..60e2a86c535e 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -860,7 +860,7 @@ static void kernfs_notify_workfn(struct work_struct *work) spin_unlock_irq(&kernfs_notify_lock); /* kick fsnotify */ - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); list_for_each_entry(info, &kernfs_root(kn)->supers, node) { struct kernfs_node *parent; @@ -898,7 +898,7 @@ static void kernfs_notify_workfn(struct work_struct *work) iput(inode); } - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); kernfs_put(kn); goto repeat; } diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index 26f2aa3586f9..dad749f39518 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -100,9 +100,9 @@ int kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr) { int ret; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); ret = __kernfs_setattr(kn, iattr); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return ret; } @@ -116,7 +116,7 @@ int kernfs_iop_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, if (!kn) return -EINVAL; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); error = setattr_prepare(&init_user_ns, dentry, iattr); if (error) goto out; @@ -129,7 +129,7 @@ int kernfs_iop_setattr(struct user_namespace *mnt_userns, struct dentry *dentry, setattr_copy(&init_user_ns, inode, iattr); out: - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return error; } @@ -185,9 +185,9 @@ int kernfs_iop_getattr(struct user_namespace *mnt_userns, struct inode *inode = d_inode(path->dentry); struct kernfs_node *kn = inode->i_private; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); kernfs_refresh_inode(kn, inode); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); generic_fillattr(&init_user_ns, inode, stat); return 0; @@ -278,9 +278,9 @@ int kernfs_iop_permission(struct user_namespace *mnt_userns, kn = inode->i_private; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); kernfs_refresh_inode(kn, inode); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); return generic_permission(&init_user_ns, inode, mask); } diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h index c2ae58f3b202..f9cc912c31e1 100644 --- a/fs/kernfs/kernfs-internal.h +++ b/fs/kernfs/kernfs-internal.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -69,7 +70,7 @@ struct kernfs_super_info { */ const void *ns; - /* anchored at kernfs_root->supers, protected by kernfs_mutex */ + /* anchored at kernfs_root->supers, protected by kernfs_rwsem */ struct list_head node; }; #define kernfs_info(SB) ((struct kernfs_super_info *)(SB->s_fs_info)) @@ -121,7 +122,7 @@ int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr); /* * dir.c */ -extern struct mutex kernfs_mutex; +extern struct rw_semaphore kernfs_rwsem; extern const struct dentry_operations kernfs_dops; extern const struct file_operations kernfs_dir_fops; extern const struct inode_operations kernfs_dir_iops; diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c index 9dc7e7a64e10..baa4155ba2ed 100644 --- a/fs/kernfs/mount.c +++ b/fs/kernfs/mount.c @@ -255,9 +255,9 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k sb->s_shrink.seeks = 0; /* get root inode, initialize and unlock it */ - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); inode = kernfs_get_inode(sb, info->root->kn); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); if (!inode) { pr_debug("kernfs: could not get root inode\n"); return -ENOMEM; @@ -344,9 +344,9 @@ int kernfs_get_tree(struct fs_context *fc) } sb->s_flags |= SB_ACTIVE; - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); list_add(&info->node, &info->root->supers); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); } fc->root = dget(sb->s_root); @@ -372,9 +372,9 @@ void kernfs_kill_sb(struct super_block *sb) { struct kernfs_super_info *info = kernfs_info(sb); - mutex_lock(&kernfs_mutex); + down_write(&kernfs_rwsem); list_del(&info->node); - mutex_unlock(&kernfs_mutex); + up_write(&kernfs_rwsem); /* * Remove the superblock from fs_supers/s_instances diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c index 5432883d819f..c8f8e41b8411 100644 --- a/fs/kernfs/symlink.c +++ b/fs/kernfs/symlink.c @@ -116,9 +116,9 @@ static int kernfs_getlink(struct inode *inode, char *path) struct kernfs_node *target = kn->symlink.target_kn; int error; - mutex_lock(&kernfs_mutex); + down_read(&kernfs_rwsem); error = kernfs_get_target_path(parent, target, path); - mutex_unlock(&kernfs_mutex); + up_read(&kernfs_rwsem); return error; } diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h index d68b4ad09573..1093abf7c28c 100644 --- a/include/linux/kernfs.h +++ b/include/linux/kernfs.h @@ -193,7 +193,7 @@ struct kernfs_root { u32 id_highbits; struct kernfs_syscall_ops *syscall_ops; - /* list of kernfs_super_info of this root, protected by kernfs_mutex */ + /* list of kernfs_super_info of this root, protected by kernfs_rwsem */ struct list_head supers; wait_queue_head_t deactivate_waitq; From patchwork Fri Jul 16 09:28:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12381815 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B92A9C07E95 for ; Fri, 16 Jul 2021 09:37:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A36066128D for ; Fri, 16 Jul 2021 09:37:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234222AbhGPJkl (ORCPT ); Fri, 16 Jul 2021 05:40:41 -0400 Received: from icp-osb-irony-out9.external.iinet.net.au ([203.59.1.226]:22594 "EHLO icp-osb-irony-out9.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234143AbhGPJkj (ORCPT ); Fri, 16 Jul 2021 05:40:39 -0400 X-Greylist: delayed 556 seconds by postgrey-1.27 at vger.kernel.org; Fri, 16 Jul 2021 05:40:26 EDT X-SMTP-MATCH: 0 IronPort-HdrOrdr: A9a23:8kqwx6Mh05j+ysBcTuqjsMiBIKoaSvp037BL7SBMoHluGfBw+PrFoB1273LJYVUqOU3I++ruBEDoexq1yXcf2+cs1NmZMDUPOAOTXeJfBQiL+UyYJ8XUndQtt5uJecNFeaXN5d8Tt7ec3OE7e+xQpuVucciT9ILjJ/IEd3APV51d X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AbBwA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCgXSCAmyEAkaQEwEBAQEBAQaBQIpphW9fiimBfAsBAQEBAQEBAQFKBAEBhFQCgnwBJTQJDgIEFQEBAQUBAQEBAQYDAYEOhXVDAQwBhXUGIwRSEBgNAhgOAgJHEAYBEoVTJadnfzMaAmWKQ4EQKgGHCIJog3onHH2BEIFIA4EFgiiHW4JkBIIlcgEtYnsEJwoqJUArO5QmAUaIbYEsW50Mgy6eZBeDTJIDAxaQXy2McIhrghykcoIUTS4KgyRQGZ0JNy84AgYKAQEDCYJyiWcBAQ X-IPAS-Result: A2AbBwA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCgXSCAmyEAkaQEwEBAQEBAQaBQIpphW9fiimBfAsBAQEBAQEBAQFKBAEBhFQCgnwBJTQJDgIEFQEBAQUBAQEBAQYDAYEOhXVDAQwBhXUGIwRSEBgNAhgOAgJHEAYBEoVTJadnfzMaAmWKQ4EQKgGHCIJog3onHH2BEIFIA4EFgiiHW4JkBIIlcgEtYnsEJwoqJUArO5QmAUaIbYEsW50Mgy6eZBeDTJIDAxaQXy2McIhrghykcoIUTS4KgyRQGZ0JNy84AgYKAQEDCYJyiWcBAQ X-IronPort-AV: E=Sophos;i="5.84,244,1620662400"; d="scan'208";a="329873602" Received: from unknown (HELO web.messagingengine.com) ([121.44.132.45]) by icp-osb-irony-out9.iinet.net.au with ESMTP; 16 Jul 2021 17:28:34 +0800 Subject: [PATCH v8 4/5] kernfs: use i_lock to protect concurrent inode updates From: Ian Kent To: Greg Kroah-Hartman , Tejun Heo Cc: Eric Sandeen , Fox Chen , Brice Goglin , Al Viro , Rick Lindsley , David Howells , Miklos Szeredi , Marcelo Tosatti , "Eric W. Biederman" , Carlos Maiolino , linux-fsdevel , Kernel Mailing List Date: Fri, 16 Jul 2021 17:28:34 +0800 Message-ID: <162642771474.63632.16295959115893904470.stgit@web.messagingengine.com> In-Reply-To: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> References: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org The inode operations .permission() and .getattr() use the kernfs node write lock but all that's needed is the read lock to protect against partial updates of these kernfs node fields which are all done under the write lock. And .permission() is called frequently during path walks and can cause quite a bit of contention between kernfs node operations and path walks when the number of concurrent walks is high. To change kernfs_iop_getattr() and kernfs_iop_permission() to take the rw sem read lock instead of the write lock an additional lock is needed to protect against multiple processes concurrently updating the inode attributes and link count in kernfs_refresh_inode(). The inode i_lock seems like the sensible thing to use to protect these inode attribute updates so use it in kernfs_refresh_inode(). The last hunk in the patch, applied to kernfs_fill_super(), is possibly not needed but taking the lock was present originally. I prefer to continue to take it to protect against a partial update of the source kernfs fields during the call to kernfs_refresh_inode() made by kernfs_get_inode(). Signed-off-by: Ian Kent Reviewed-by: Miklos Szeredi --- fs/kernfs/inode.c | 18 ++++++++++++------ fs/kernfs/mount.c | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index dad749f39518..c0eae1725435 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -185,11 +185,13 @@ int kernfs_iop_getattr(struct user_namespace *mnt_userns, struct inode *inode = d_inode(path->dentry); struct kernfs_node *kn = inode->i_private; - down_write(&kernfs_rwsem); + down_read(&kernfs_rwsem); + spin_lock(&inode->i_lock); kernfs_refresh_inode(kn, inode); - up_write(&kernfs_rwsem); - generic_fillattr(&init_user_ns, inode, stat); + spin_unlock(&inode->i_lock); + up_read(&kernfs_rwsem); + return 0; } @@ -272,17 +274,21 @@ int kernfs_iop_permission(struct user_namespace *mnt_userns, struct inode *inode, int mask) { struct kernfs_node *kn; + int ret; if (mask & MAY_NOT_BLOCK) return -ECHILD; kn = inode->i_private; - down_write(&kernfs_rwsem); + down_read(&kernfs_rwsem); + spin_lock(&inode->i_lock); kernfs_refresh_inode(kn, inode); - up_write(&kernfs_rwsem); + ret = generic_permission(&init_user_ns, inode, mask); + spin_unlock(&inode->i_lock); + up_read(&kernfs_rwsem); - return generic_permission(&init_user_ns, inode, mask); + return ret; } int kernfs_xattr_get(struct kernfs_node *kn, const char *name, diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c index baa4155ba2ed..f2f909d09f52 100644 --- a/fs/kernfs/mount.c +++ b/fs/kernfs/mount.c @@ -255,9 +255,9 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k sb->s_shrink.seeks = 0; /* get root inode, initialize and unlock it */ - down_write(&kernfs_rwsem); + down_read(&kernfs_rwsem); inode = kernfs_get_inode(sb, info->root->kn); - up_write(&kernfs_rwsem); + up_read(&kernfs_rwsem); if (!inode) { pr_debug("kernfs: could not get root inode\n"); return -ENOMEM; From patchwork Fri Jul 16 09:28:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12381819 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F931C07E95 for ; Fri, 16 Jul 2021 09:38:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6910F613D8 for ; Fri, 16 Jul 2021 09:38:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234820AbhGPJk4 (ORCPT ); Fri, 16 Jul 2021 05:40:56 -0400 Received: from icp-osb-irony-out9.external.iinet.net.au ([203.59.1.226]:22642 "EHLO icp-osb-irony-out9.external.iinet.net.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234645AbhGPJkw (ORCPT ); Fri, 16 Jul 2021 05:40:52 -0400 X-SMTP-MATCH: 0 IronPort-HdrOrdr: A9a23:t7TAma1IrZHKw7M8QUCgYgqjBJkkLtp133Aq2lEZdPU1SKylfqWV98jzuiWftN98YhwdcLO7WZVoI0myyXcd2+B4AV7IZmfbUQWTQL2LbePZsl7d866XzJ8l6U9KGJIOauEZBzNB/L7HCI7RKadG/DEEmJrY49s3Th9WPGRXgyQJ1XYcNjqm X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AZBwA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCg3ZshAJGkBMBAQEBAQEGjCmFb1+KKYF8CwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBDAGFdQYjBFIQGA0CGA4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgRUzA4EFgiiHW4JkBIMYgQ4mWoFAlQwBRooZW50Mgy6eZBeVTwOQdS2VW6cOghRNLgqDJFAZjjgXjjo3LzgCBgoBAQMJgnKHIi2CGAEB X-IPAS-Result: A2AZBwA6UPFg/y2ELHlaHgEBCxIMQAmBRQsCg3ZshAJGkBMBAQEBAQEGjCmFb1+KKYF8CwEBAQEBAQEBAUoEAQGEVAKCfAElNAkOAgQVAQEBBQEBAQEBBgMBgQ6FdUMBDAGFdQYjBFIQGA0CGA4CAkcQBgEShVMlp2d/MxoCZYpDgRAqAYcIgmiDeiccfYEQgRUzA4EFgiiHW4JkBIMYgQ4mWoFAlQwBRooZW50Mgy6eZBeVTwOQdS2VW6cOghRNLgqDJFAZjjgXjjo3LzgCBgoBAQMJgnKHIi2CGAEB X-IronPort-AV: E=Sophos;i="5.84,244,1620662400"; d="scan'208";a="329873615" Received: from unknown (HELO web.messagingengine.com) ([121.44.132.45]) by icp-osb-irony-out9.iinet.net.au with ESMTP; 16 Jul 2021 17:28:40 +0800 Subject: [PATCH v8 5/5] kernfs: dont call d_splice_alias() under kernfs node lock From: Ian Kent To: Greg Kroah-Hartman , Tejun Heo Cc: Eric Sandeen , Fox Chen , Brice Goglin , Al Viro , Rick Lindsley , David Howells , Miklos Szeredi , Marcelo Tosatti , "Eric W. Biederman" , Carlos Maiolino , linux-fsdevel , Kernel Mailing List Date: Fri, 16 Jul 2021 17:28:40 +0800 Message-ID: <162642772000.63632.10672683419693513226.stgit@web.messagingengine.com> In-Reply-To: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> References: <162642752894.63632.5596341704463755308.stgit@web.messagingengine.com> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org The call to d_splice_alias() in kernfs_iop_lookup() doesn't depend on any kernfs node so there's no reason to hold the kernfs node lock when calling it. Signed-off-by: Ian Kent Reviewed-by: Miklos Szeredi --- fs/kernfs/dir.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 4994723d6cf7..ba581429bf7b 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -1100,7 +1100,6 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags) { - struct dentry *ret; struct kernfs_node *parent = dir->i_private; struct kernfs_node *kn; struct inode *inode = NULL; @@ -1120,11 +1119,10 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir, /* Needed only for negative dentry validation */ if (!inode) kernfs_set_rev(parent, dentry); - /* instantiate and hash (possibly negative) dentry */ - ret = d_splice_alias(inode, dentry); up_read(&kernfs_rwsem); - return ret; + /* instantiate and hash (possibly negative) dentry */ + return d_splice_alias(inode, dentry); } static int kernfs_iop_mkdir(struct user_namespace *mnt_userns,