From: Fox Chen <foxhlchen@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Ian Kent <raven@themaw.net>, Tejun Heo <tj@kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Eric Sandeen <sandeen@sandeen.net>,
Brice Goglin <brice.goglin@gmail.com>,
Rick Lindsley <ricklind@linux.vnet.ibm.com>,
David Howells <dhowells@redhat.com>,
Miklos Szeredi <miklos@szeredi.hu>,
Marcelo Tosatti <mtosatti@redhat.com>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 0/5] kernfs: proposed locking and concurrency improvement
Date: Wed, 12 May 2021 15:16:01 +0800 [thread overview]
Message-ID: <CAC2o3D+28g67vbNOaVxuF0OfE0RjFGHVwAcA_3t1AAS_b_EnPg@mail.gmail.com> (raw)
In-Reply-To: <YJtz6mmgPIwEQNgD@kroah.com>
On Wed, May 12, 2021 at 2:21 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, May 12, 2021 at 08:38:35AM +0800, Ian Kent wrote:
> > There have been a few instances of contention on the kernfs_mutex during
> > path walks, a case on very large IBM systems seen by myself, a report by
> > Brice Goglin and followed up by Fox Chen, and I've since seen a couple
> > of other reports by CoreOS users.
> >
> > The common thread is a large number of kernfs path walks leading to
> > slowness of path walks due to kernfs_mutex contention.
> >
> > The problem being that changes to the VFS over some time have increased
> > it's concurrency capabilities to an extent that kernfs's use of a mutex
> > is no longer appropriate. There's also an issue of walks for non-existent
> > paths causing contention if there are quite a few of them which is a less
> > common problem.
> >
> > This patch series is relatively straight forward.
> >
> > All it does is add the ability to take advantage of VFS negative dentry
> > caching to avoid needless dentry alloc/free cycles for lookups of paths
> > that don't exit and change the kernfs_mutex to a read/write semaphore.
> >
> > The patch that tried to stay in VFS rcu-walk mode during path walks has
> > been dropped for two reasons. First, it doesn't actually give very much
> > improvement and, second, if there's a place where mistakes could go
> > unnoticed it would be in that path. This makes the patch series simpler
> > to review and reduces the likelihood of problems going unnoticed and
> > popping up later.
> >
> > The patch to use a revision to identify if a directory has changed has
> > also been dropped. If the directory has changed the dentry revision
> > needs to be updated to avoid subsequent rb tree searches and after
> > changing to use a read/write semaphore the update also requires a lock.
> > But the d_lock is the only lock available at this point which might
> > itself be contended.
> >
> > Changes since v3:
> > - remove unneeded indirection when referencing the super block.
> > - check if inode attribute update is actually needed.
> >
> > Changes since v2:
> > - actually fix the inode attribute update locking.
> > - drop the patch that tried to stay in rcu-walk mode.
> > - drop the use a revision to identify if a directory has changed patch.
> >
> > Changes since v1:
> > - fix locking in .permission() and .getattr() by re-factoring the attribute
> > handling code.
> > ---
> >
> > Ian Kent (5):
> > kernfs: move revalidate to be near lookup
> > kernfs: use VFS negative dentry caching
> > kernfs: switch kernfs to use an rwsem
> > kernfs: use i_lock to protect concurrent inode updates
> > kernfs: add kernfs_need_inode_refresh()
> >
> >
> > fs/kernfs/dir.c | 170 ++++++++++++++++++++----------------
> > fs/kernfs/file.c | 4 +-
> > fs/kernfs/inode.c | 45 ++++++++--
> > fs/kernfs/kernfs-internal.h | 5 +-
> > fs/kernfs/mount.c | 12 +--
> > fs/kernfs/symlink.c | 4 +-
> > include/linux/kernfs.h | 2 +-
> > 7 files changed, 147 insertions(+), 95 deletions(-)
> >
> > --
> > Ian
> >
>
> Any benchmark numbers that you ran that are better/worse with this patch
> series? That woul dbe good to know, otherwise you aren't changing
> functionality here, so why would we take these changes? :)
Let me run it on my benchmark and bring back the result to you.
> thanks,
>
> greg k-h
thanks,
fox
next prev parent reply other threads:[~2021-05-12 7:16 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 0:38 [PATCH v4 0/5] kernfs: proposed locking and concurrency improvement Ian Kent
2021-05-12 0:38 ` [PATCH v4 1/5] kernfs: move revalidate to be near lookup Ian Kent
2021-05-12 0:38 ` [PATCH v4 2/5] kernfs: use VFS negative dentry caching Ian Kent
2021-05-12 0:39 ` [PATCH v4 3/5] kernfs: switch kernfs to use an rwsem Ian Kent
2021-05-12 0:39 ` [PATCH v4 4/5] kernfs: use i_lock to protect concurrent inode updates Ian Kent
2021-05-12 0:39 ` [PATCH v4 5/5] kernfs: add kernfs_need_inode_refresh() Ian Kent
2021-05-12 6:21 ` [PATCH v4 0/5] kernfs: proposed locking and concurrency improvement Greg Kroah-Hartman
2021-05-12 7:16 ` Fox Chen [this message]
2021-05-12 8:47 ` Fox Chen
2021-05-12 8:54 ` Fox Chen
2021-05-13 14:10 ` Ian Kent
2021-05-13 15:37 ` Fox Chen
2021-05-14 1:34 ` Ian Kent
2021-05-14 2:34 ` Fox Chen
2021-05-17 1:32 ` Ian Kent
2021-05-18 8:26 ` Fox Chen
2021-05-27 1:23 ` Ian Kent
2021-05-27 6:50 ` Greg Kroah-Hartman
2021-05-28 5:45 ` Ian Kent
2021-05-13 13:50 ` Ian Kent
2021-05-13 15:19 ` Greg Kroah-Hartman
2021-05-14 1:02 ` Ian Kent
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAC2o3D+28g67vbNOaVxuF0OfE0RjFGHVwAcA_3t1AAS_b_EnPg@mail.gmail.com \
--to=foxhlchen@gmail.com \
--cc=brice.goglin@gmail.com \
--cc=dhowells@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=mtosatti@redhat.com \
--cc=raven@themaw.net \
--cc=ricklind@linux.vnet.ibm.com \
--cc=sandeen@sandeen.net \
--cc=tj@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.