| Message ID | 162218363530.34379.16741129191900256265.stgit@web.messagingengine.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kernfs: proposed locking and concurrency improvement | expand |
Ian Kent <raven@themaw.net> writes: > While the dentry operation kernfs_dop_revalidate() is grouped with > dentry type functions it also has a strong affinity to the inode > operation ->lookup(). > > In order to take advantage of the VFS negative dentry caching that > can be used to reduce path lookup overhead on non-existent paths it > will need to call kernfs_find_ns(). So, to avoid a forward declaration, > move it to be near kernfs_iop_lookup(). > > There's no functional change from this patch. Does this patch compile independently? During the code movement kernfs_active is replaced by kernfs_active_read which does not exist yet. Eric > Signed-off-by: Ian Kent <raven@themaw.net> > --- > fs/kernfs/dir.c | 86 ++++++++++++++++++++++++++++--------------------------- > 1 file changed, 43 insertions(+), 43 deletions(-) > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > index 7e0e62deab53c..4c69e2af82dac 100644 > --- a/fs/kernfs/dir.c > +++ b/fs/kernfs/dir.c > @@ -548,49 +548,6 @@ void kernfs_put(struct kernfs_node *kn) > } > EXPORT_SYMBOL_GPL(kernfs_put); > > -static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) > -{ > - struct kernfs_node *kn; > - > - if (flags & LOOKUP_RCU) > - return -ECHILD; > - > - /* Always perform fresh lookup for negatives */ > - if (d_really_is_negative(dentry)) > - goto out_bad_unlocked; > - > - kn = kernfs_dentry_node(dentry); > - mutex_lock(&kernfs_mutex); > - > - /* The kernfs node has been deactivated */ > - if (!kernfs_active(kn)) > - goto out_bad; > - > - /* The kernfs node has been moved? */ > - if (kernfs_dentry_node(dentry->d_parent) != kn->parent) > - goto out_bad; > - > - /* The kernfs node has been renamed */ > - if (strcmp(dentry->d_name.name, kn->name) != 0) > - goto out_bad; > - > - /* The kernfs node has been moved to a different namespace */ > - if (kn->parent && kernfs_ns_enabled(kn->parent) && > - kernfs_info(dentry->d_sb)->ns != kn->ns) > - goto out_bad; > - > - mutex_unlock(&kernfs_mutex); > - return 1; > -out_bad: > - mutex_unlock(&kernfs_mutex); > -out_bad_unlocked: > - return 0; > -} > - > -const struct dentry_operations kernfs_dops = { > - .d_revalidate = kernfs_dop_revalidate, > -}; > - > /** > * kernfs_node_from_dentry - determine kernfs_node associated with a dentry > * @dentry: the dentry in question > @@ -1073,6 +1030,49 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent, > return ERR_PTR(rc); > } > > +static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) > +{ > + struct kernfs_node *kn; > + > + if (flags & LOOKUP_RCU) > + return -ECHILD; > + > + /* Always perform fresh lookup for negatives */ > + if (d_really_is_negative(dentry)) > + goto out_bad_unlocked; > + > + kn = kernfs_dentry_node(dentry); > + mutex_lock(&kernfs_mutex); > + > + /* The kernfs node has been deactivated */ > + if (!kernfs_active_read(kn)) > + goto out_bad; > + > + /* The kernfs node has been moved? */ > + if (kernfs_dentry_node(dentry->d_parent) != kn->parent) > + goto out_bad; > + > + /* The kernfs node has been renamed */ > + if (strcmp(dentry->d_name.name, kn->name) != 0) > + goto out_bad; > + > + /* The kernfs node has been moved to a different namespace */ > + if (kn->parent && kernfs_ns_enabled(kn->parent) && > + kernfs_info(dentry->d_sb)->ns != kn->ns) > + goto out_bad; > + > + mutex_unlock(&kernfs_mutex); > + return 1; > +out_bad: > + mutex_unlock(&kernfs_mutex); > +out_bad_unlocked: > + return 0; > +} > + > +const struct dentry_operations kernfs_dops = { > + .d_revalidate = kernfs_dop_revalidate, > +}; > + > static struct dentry *kernfs_iop_lookup(struct inode *dir, > struct dentry *dentry, > unsigned int flags)
On Thu, 2021-06-03 at 09:50 -0500, Eric W. Biederman wrote: > Ian Kent <raven@themaw.net> writes: > > > While the dentry operation kernfs_dop_revalidate() is grouped with > > dentry type functions it also has a strong affinity to the inode > > operation ->lookup(). > > > > In order to take advantage of the VFS negative dentry caching that > > can be used to reduce path lookup overhead on non-existent paths it > > will need to call kernfs_find_ns(). So, to avoid a forward > > declaration, > > move it to be near kernfs_iop_lookup(). > > > > There's no functional change from this patch. > > Does this patch compile independently? Doubt it. > > During the code movement kernfs_active is replaced > by kernfs_active_read which does not exist yet. Oops, that was a consequence of reordering the series which I didn't catch. I'll fix that when I post a v5 which I'm going to have to do. Thanks for looking at this Eric, Ian > > Eric > > > Signed-off-by: Ian Kent <raven@themaw.net> > > --- > > fs/kernfs/dir.c | 86 ++++++++++++++++++++++++++++--------------- > > ------------ > > 1 file changed, 43 insertions(+), 43 deletions(-) > > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > > index 7e0e62deab53c..4c69e2af82dac 100644 > > --- a/fs/kernfs/dir.c > > +++ b/fs/kernfs/dir.c > > @@ -548,49 +548,6 @@ void kernfs_put(struct kernfs_node *kn) > > } > > EXPORT_SYMBOL_GPL(kernfs_put); > > > > -static int kernfs_dop_revalidate(struct dentry *dentry, unsigned > > int flags) > > -{ > > - struct kernfs_node *kn; > > - > > - if (flags & LOOKUP_RCU) > > - return -ECHILD; > > - > > - /* Always perform fresh lookup for negatives */ > > - if (d_really_is_negative(dentry)) > > - goto out_bad_unlocked; > > - > > - kn = kernfs_dentry_node(dentry); > > - mutex_lock(&kernfs_mutex); > > - > > - /* The kernfs node has been deactivated */ > > - if (!kernfs_active(kn)) > > - goto out_bad; > > - > > - /* The kernfs node has been moved? */ > > - if (kernfs_dentry_node(dentry->d_parent) != kn->parent) > > - goto out_bad; > > - > > - /* The kernfs node has been renamed */ > > - if (strcmp(dentry->d_name.name, kn->name) != 0) > > - goto out_bad; > > - > > - /* The kernfs node has been moved to a different namespace > > */ > > - if (kn->parent && kernfs_ns_enabled(kn->parent) && > > - kernfs_info(dentry->d_sb)->ns != kn->ns) > > - goto out_bad; > > - > > - mutex_unlock(&kernfs_mutex); > > - return 1; > > -out_bad: > > - mutex_unlock(&kernfs_mutex); > > -out_bad_unlocked: > > - return 0; > > -} > > - > > -const struct dentry_operations kernfs_dops = { > > - .d_revalidate = kernfs_dop_revalidate, > > -}; > > - > > /** > > * kernfs_node_from_dentry - determine kernfs_node associated with > > a dentry > > * @dentry: the dentry in question > > @@ -1073,6 +1030,49 @@ struct kernfs_node > > *kernfs_create_empty_dir(struct kernfs_node *parent, > > return ERR_PTR(rc); > > } > > > > +static int kernfs_dop_revalidate(struct dentry *dentry, unsigned > > int flags) > > +{ > > + struct kernfs_node *kn; > > + > > + if (flags & LOOKUP_RCU) > > + return -ECHILD; > > + > > + /* Always perform fresh lookup for negatives */ > > + if (d_really_is_negative(dentry)) > > + goto out_bad_unlocked; > > + > > + kn = kernfs_dentry_node(dentry); > > + mutex_lock(&kernfs_mutex); > > + > > + /* The kernfs node has been deactivated */ > > + if (!kernfs_active_read(kn)) > > + goto out_bad; > > + > > + /* The kernfs node has been moved? */ > > + if (kernfs_dentry_node(dentry->d_parent) != kn->parent) > > + goto out_bad; > > + > > + /* The kernfs node has been renamed */ > > + if (strcmp(dentry->d_name.name, kn->name) != 0) > > + goto out_bad; > > + > > + /* The kernfs node has been moved to a different namespace > > */ > > + if (kn->parent && kernfs_ns_enabled(kn->parent) && > > + kernfs_info(dentry->d_sb)->ns != kn->ns) > > + goto out_bad; > > + > > + mutex_unlock(&kernfs_mutex); > > + return 1; > > +out_bad: > > + mutex_unlock(&kernfs_mutex); > > +out_bad_unlocked: > > + return 0; > > +} > > + > > +const struct dentry_operations kernfs_dops = { > > + .d_revalidate = kernfs_dop_revalidate, > > +}; > > + > > static struct dentry *kernfs_iop_lookup(struct inode *dir, > > struct dentry *dentry, > > unsigned int flags)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 7e0e62deab53c..4c69e2af82dac 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -548,49 +548,6 @@ void kernfs_put(struct kernfs_node *kn) } EXPORT_SYMBOL_GPL(kernfs_put); -static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) -{ - struct kernfs_node *kn; - - if (flags & LOOKUP_RCU) - return -ECHILD; - - /* Always perform fresh lookup for negatives */ - if (d_really_is_negative(dentry)) - goto out_bad_unlocked; - - kn = kernfs_dentry_node(dentry); - mutex_lock(&kernfs_mutex); - - /* The kernfs node has been deactivated */ - if (!kernfs_active(kn)) - goto out_bad; - - /* The kernfs node has been moved? */ - if (kernfs_dentry_node(dentry->d_parent) != kn->parent) - goto out_bad; - - /* The kernfs node has been renamed */ - if (strcmp(dentry->d_name.name, kn->name) != 0) - goto out_bad; - - /* The kernfs node has been moved to a different namespace */ - if (kn->parent && kernfs_ns_enabled(kn->parent) && - kernfs_info(dentry->d_sb)->ns != kn->ns) - goto out_bad; - - mutex_unlock(&kernfs_mutex); - return 1; -out_bad: - mutex_unlock(&kernfs_mutex); -out_bad_unlocked: - return 0; -} - -const struct dentry_operations kernfs_dops = { - .d_revalidate = kernfs_dop_revalidate, -}; - /** * kernfs_node_from_dentry - determine kernfs_node associated with a dentry * @dentry: the dentry in question @@ -1073,6 +1030,49 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent, return ERR_PTR(rc); } +static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags) +{ + struct kernfs_node *kn; + + if (flags & LOOKUP_RCU) + return -ECHILD; + + /* Always perform fresh lookup for negatives */ + if (d_really_is_negative(dentry)) + goto out_bad_unlocked; + + kn = kernfs_dentry_node(dentry); + mutex_lock(&kernfs_mutex); + + /* The kernfs node has been deactivated */ + if (!kernfs_active_read(kn)) + goto out_bad; + + /* The kernfs node has been moved? */ + if (kernfs_dentry_node(dentry->d_parent) != kn->parent) + goto out_bad; + + /* The kernfs node has been renamed */ + if (strcmp(dentry->d_name.name, kn->name) != 0) + goto out_bad; + + /* The kernfs node has been moved to a different namespace */ + if (kn->parent && kernfs_ns_enabled(kn->parent) && + kernfs_info(dentry->d_sb)->ns != kn->ns) + goto out_bad; + + mutex_unlock(&kernfs_mutex); + return 1; +out_bad: + mutex_unlock(&kernfs_mutex); +out_bad_unlocked: + return 0; +} + +const struct dentry_operations kernfs_dops = { + .d_revalidate = kernfs_dop_revalidate, +}; + static struct dentry *kernfs_iop_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
While the dentry operation kernfs_dop_revalidate() is grouped with dentry type functions it also has a strong affinity to the inode operation ->lookup(). In order to take advantage of the VFS negative dentry caching that can be used to reduce path lookup overhead on non-existent paths it will need to call kernfs_find_ns(). So, to avoid a forward declaration, move it to be near kernfs_iop_lookup(). There's no functional change from this patch. Signed-off-by: Ian Kent <raven@themaw.net> --- fs/kernfs/dir.c | 86 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 43 insertions(+), 43 deletions(-)