linux-kernel - Re: [REPOST PATCH v4 2/5] kernfs: use VFS negative dentry caching

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 Jun 2021 20:06:38 +0200
From:   Miklos Szeredi <miklos@...redi.hu>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Ian Kent <raven@...maw.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>, Eric Sandeen <sandeen@...deen.net>,
        Fox Chen <foxhlchen@...il.com>,
        Brice Goglin <brice.goglin@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Rick Lindsley <ricklind@...ux.vnet.ibm.com>,
        David Howells <dhowells@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [REPOST PATCH v4 2/5] kernfs: use VFS negative dentry caching

On Thu, 3 Jun 2021 at 19:26, Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
> Ian Kent <raven@...maw.net> writes:
>
> > 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.
> >
> > Signed-off-by: Ian Kent <raven@...maw.net>
> > ---
> >  fs/kernfs/dir.c |   55 +++++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 33 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > index 4c69e2af82dac..5151c712f06f5 100644
> > --- a/fs/kernfs/dir.c
> > +++ b/fs/kernfs/dir.c
> > @@ -1037,12 +1037,33 @@ 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;
> > +     mutex_lock(&kernfs_mutex);
> >
> >       kn = kernfs_dentry_node(dentry);
> > -     mutex_lock(&kernfs_mutex);
>
> Why bring kernfs_dentry_node inside the mutex?
>
> The inode lock of the parent should protect negative to positive
> transitions not the kernfs_mutex.  So moving the code inside
> the mutex looks unnecessary and confusing.

Except that d_revalidate() may or may not be called with parent lock held.

Thanks,
Miklos