Re: [PATCH 03/11] kernfs: add an API to get kernfs node from inode number

[Eduardo Valentin]

On Fri, Jun 02, 2017 at 02:53:56PM -0700, Shaohua Li wrote:
> From: Shaohua Li <shli@xxxxxx>
> 
> Add an API to get kernfs node from inode number. We will need this to
> implement exportfs operations.
> 
> To make the API lock free, kernfs node is freed in RCU context. And we
> depend on kernfs_node count/ino number to filter stale kernfs nodes.
> 
> Signed-off-by: Shaohua Li <shli@xxxxxx>
> ---
>  fs/kernfs/dir.c             | 35 +++++++++++++++++++++++++++++++++++
>  fs/kernfs/kernfs-internal.h |  2 ++
>  fs/kernfs/mount.c           |  4 +++-
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 8e8545a..4c86e4c 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -643,6 +643,7 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
>  	kn->ino = ret;
>  	kn->generation = atomic_inc_return(&root->next_generation);
>  
> +	/* set ino first. Above atomic_inc_return has a barrier */
>  	atomic_set(&kn->count, 1);
>  	atomic_set(&kn->active, KN_DEACTIVATED_BIAS);
>  	RB_CLEAR_NODE(&kn->rb);
> @@ -674,6 +675,40 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
>  	return kn;
>  }
>  
> +/*
> + * kernfs_get_node_by_ino - get kernfs_node from inode number
> + * @root: the kernfs root
> + * @ino: inode number
> + *
> + * RETURNS:
> + * NULL on failure. Return a kernfs node with reference counter incremented
> + */

Is the above supposed to be a valid kernel doc entry?

> +struct kernfs_node *kernfs_get_node_by_ino(struct kernfs_root *root,
> +					   unsigned int ino)
> +{
> +	struct kernfs_node *kn;
> +
> +	rcu_read_lock();
> +	kn = idr_find(&root->ino_idr, ino);
> +	if (!kn)
> +		goto out;
> +	/* kernfs_put removes the ino after count is 0 */
> +	if (!atomic_inc_not_zero(&kn->count)) {
> +		kn = NULL;

Why do yo need to set kn to NULL?

> +		goto out;
> +	}
> +	/* If this node is reused, __kernfs_new_node sets ino before count */
> +	if (kn->ino != ino)
> +		goto out;
> +	rcu_read_unlock();
> +
> +	return kn;
> +out:
> +	rcu_read_unlock();
> +	kernfs_put(kn);
> +	return NULL;
> +}
> +
>  /**
>   *	kernfs_add_one - add kernfs_node to parent without warning
>   *	@kn: kernfs_node to be added
> diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
> index 2d5144a..3534cfe 100644
> --- a/fs/kernfs/kernfs-internal.h
> +++ b/fs/kernfs/kernfs-internal.h
> @@ -98,6 +98,8 @@ int kernfs_add_one(struct kernfs_node *kn);
>  struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
>  				    const char *name, umode_t mode,
>  				    unsigned flags);
> +struct kernfs_node *kernfs_get_node_by_ino(struct kernfs_root *root,
> +					   unsigned int ino);
>  
>  /*
>   * file.c
> diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
> index d5b149a..343dfeb 100644
> --- a/fs/kernfs/mount.c
> +++ b/fs/kernfs/mount.c
> @@ -332,5 +332,7 @@ void __init kernfs_init(void)
>  {
>  	kernfs_node_cache = kmem_cache_create("kernfs_node_cache",
>  					      sizeof(struct kernfs_node),
> -					      0, SLAB_PANIC, NULL);
> +					      0,
> +					      SLAB_PANIC | SLAB_TYPESAFE_BY_RCU,
> +					      NULL);
>  }
> -- 
> 2.9.3
> 
> 

-- 
All the best,
Eduardo Valentin