[PATCH] kernfs: don't create a negative dentry if inactive node exists

[PATCH] kernfs: don't create a negative dentry if inactive node exists

[Ian Kent]

In kernfs_iop_lookup() a negative dentry is created if there's no kernfs
node associated with the dentry or the node is inactive.

But inactive kernfs nodes are meant to be invisible to the VFS and
creating a negative dentry for these can have unexpected side effects
when the node transitions to an active state.

The point of creating negative dentries is to avoid the expensive
alloc/free cycle that occurs if there are frequent lookups for kernfs
attributes that don't exist. So kernfs nodes that are not yet active
should not result in a negative dentry being created so when they
transition to an active state VFS lookups can create an associated
dentry is a natural way.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/kernfs/dir.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index ba581429bf7b..a957c944cf3a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1111,7 +1111,14 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
 
 	kn = kernfs_find_ns(parent, dentry->d_name.name, ns);
 	/* attach dentry and inode */
-	if (kn && kernfs_active(kn)) {
+	if (kn) {
+		/* Inactive nodes are invisible to the VFS so don't
+		 * create a negative.
+		 */
+		if (!kernfs_active(kn)) {
+			up_read(&kernfs_rwsem);
+			return NULL;
+		}
 		inode = kernfs_get_inode(dir->i_sb, kn);
 		if (!inode)
 			inode = ERR_PTR(-ENOMEM);

Re: [PATCH] kernfs: don't create a negative dentry if inactive node exists

[Greg Kroah-Hartman]

On Wed, Sep 29, 2021 at 11:04:34AM +0800, Ian Kent wrote:
> In kernfs_iop_lookup() a negative dentry is created if there's no kernfs
> node associated with the dentry or the node is inactive.
> 
> But inactive kernfs nodes are meant to be invisible to the VFS and
> creating a negative dentry for these can have unexpected side effects
> when the node transitions to an active state.
> 
> The point of creating negative dentries is to avoid the expensive
> alloc/free cycle that occurs if there are frequent lookups for kernfs
> attributes that don't exist. So kernfs nodes that are not yet active
> should not result in a negative dentry being created so when they
> transition to an active state VFS lookups can create an associated
> dentry is a natural way.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  fs/kernfs/dir.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Does this fix a specific commit and need a "Fixes:" tag?

thanks,

greg k-h

Re: [PATCH] kernfs: don't create a negative dentry if inactive node exists

[Ian Kent]

On Wed, 2021-09-29 at 08:05 +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 29, 2021 at 11:04:34AM +0800, Ian Kent wrote:
> > In kernfs_iop_lookup() a negative dentry is created if there's no
> > kernfs
> > node associated with the dentry or the node is inactive.
> > 
> > But inactive kernfs nodes are meant to be invisible to the VFS and
> > creating a negative dentry for these can have unexpected side
> > effects
> > when the node transitions to an active state.
> > 
> > The point of creating negative dentries is to avoid the expensive
> > alloc/free cycle that occurs if there are frequent lookups for
> > kernfs
> > attributes that don't exist. So kernfs nodes that are not yet
> > active
> > should not result in a negative dentry being created so when they
> > transition to an active state VFS lookups can create an associated
> > dentry is a natural way.
> > 
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
> >  fs/kernfs/dir.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Does this fix a specific commit and need a "Fixes:" tag?

Oh, of course yes, apologies, my bad.
I re-post it.


Ian

Re: [PATCH] kernfs: don't create a negative dentry if inactive node exists

[Ian Kent]

On Thu, 2021-09-30 at 06:28 +0800, Ian Kent wrote:
> On Wed, 2021-09-29 at 08:05 +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 29, 2021 at 11:04:34AM +0800, Ian Kent wrote:
> > > In kernfs_iop_lookup() a negative dentry is created if there's no
> > > kernfs
> > > node associated with the dentry or the node is inactive.
> > > 
> > > But inactive kernfs nodes are meant to be invisible to the VFS
> > > and
> > > creating a negative dentry for these can have unexpected side
> > > effects
> > > when the node transitions to an active state.
> > > 
> > > The point of creating negative dentries is to avoid the expensive
> > > alloc/free cycle that occurs if there are frequent lookups for
> > > kernfs
> > > attributes that don't exist. So kernfs nodes that are not yet
> > > active
> > > should not result in a negative dentry being created so when they
> > > transition to an active state VFS lookups can create an
> > > associated
> > > dentry is a natural way.
> > > 
> > > Signed-off-by: Ian Kent <raven@themaw.net>
> > > ---
> > >  fs/kernfs/dir.c |    9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > Does this fix a specific commit and need a "Fixes:" tag?
> 
> Oh, of course yes, apologies, my bad.
> I re-post it.

But in case your ok to add it on my behalf it should be:
Fixes: c7e7c04274b1 ("kernfs: use VFS negative dentry caching")

> 
> 
> Ian