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