On Fri, Jun 02, 2017 at 03:03:45PM -0700, Eduardo Valentin wrote:
> 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?
what do you expect? The function name explains it very well actually.
> > +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?
I don't know what kind of explanation you expect. This is quite obvious
actually. If the count == 0, we don't increase the ref count, so we don't
decrease the ref count later (in kernfs_put).
> > + 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
