Hi Ian,
On Tue, Dec 22, 2020 at 3:48 PM Ian Kent <raven@xxxxxxxxxx> wrote:
>
> During path walks in sysfs (kernfs) needing to take a reference to
> a mount doesn't happen often since the walk won't be crossing mount
> point boundaries.
>
> Also while staying in rcu-walk mode where possible wouldn't normally
> give much improvement.
>
> But when there are many concurrent path walks and there is high d_lock
> contention dget() will often need to resort to taking a spin lock to
> get the reference. And that could happen each time the reference is
> passed from component to component.
>
> So, in the high contention case, it will contribute to the contention.
>
> Therefore staying in rcu-walk mode when possible will reduce contention.
>
> Signed-off-by: Ian Kent <raven@xxxxxxxxxx>
> ---
> fs/kernfs/dir.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index fdeae2c6e7ba..50c5c8c886af 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1048,8 +1048,54 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
> struct kernfs_node *parent;
> struct kernfs_node *kn;
>
> - if (flags & LOOKUP_RCU)
> + if (flags & LOOKUP_RCU) {
> + parent = kernfs_dentry_node(dentry->d_parent);
> +
> + /* Directory node changed, no, then don't search? */
> + if (!kernfs_dir_changed(parent, dentry))
> + return 1;
> +
> + kn = kernfs_dentry_node(dentry);
> + if (!kn) {
> + /* Negative hashed dentry, tell the VFS to switch to
> + * ref-walk mode and call us again so that node
> + * existence can be checked.
> + */
> + if (!d_unhashed(dentry))
> + return -ECHILD;
> +
> + /* Negative unhashed dentry, this shouldn't happen
> + * because this case occurs in ref-walk mode after
> + * dentry allocation which is followed by a call
> + * to ->loopup(). But if it does happen the dentry
> + * is surely invalid.
> + */
> + return 0;
> + }
> +
> + /* Since the dentry is positive (we got the kernfs node) a
> + * kernfs node reference was held at the time. Now if the
> + * dentry reference count is still greater than 0 it's still
> + * positive so take a reference to the node to perform an
> + * active check.
> + */
> + if (d_count(dentry) <= 0 || !atomic_inc_not_zero(&kn->count))
> + return -ECHILD;
> +
> + /* The kernfs node reference count was greater than 0, if
> + * it's active continue in rcu-walk mode.
> + */
> + if (kernfs_active_read(kn)) {
We are in RCU-walk mode, kernfs_rwsem should not be held, however,
kernfs_active_read will assert the readlock is held. I believe it
should be kernfs_active(kn) here. Am I wrong??
> + kernfs_put(kn);
> + return 1;
> + }
> +
> + /* Otherwise, just tell the VFS to switch to ref-walk mode
> + * and call us again so the kernfs node can be validated.
> + */
> + kernfs_put(kn);
> return -ECHILD;
> + }
>
> down_read(&kernfs_rwsem);
>
>
>
thanks,
fox
