| Message ID | 163288467430.30015.16308604689059471602.stgit@mickey.themaw.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kernfs: don't create a negative dentry if inactive node exists | expand |
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
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
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
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);
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(-)