Hello, On Tue, Jul 03, 2018 at 03:22:47PM +1000, Benjamin Herrenschmidt wrote: > +bool kernfs_has_children(struct kernfs_node *kn) > +{ > + bool has_children = false; > + struct kernfs_node *pos; > + > + /* Lockless shortcut */ > + if (RB_EMPTY_NODE(&kn->rb)) > + return false;
Hmm... shouldn't it be testing !rb_first(kn->dir.children)? The above would test whether @kn itself is unlinked. > + > + /* Now check for active children */ > + mutex_lock(&kernfs_mutex); > + pos = NULL; > + while ((pos = kernfs_next_descendant_post(pos, kn)) && pos != kn) { > + if (kernfs_active(pos)) { > + has_children = true; > + break; > + } > + } > + mutex_unlock(&kernfs_mutex); > + > + return has_children; The active ref is there to synchronize removal against in-flight reads so that kernfs_remove() can wait for them to drain. On return from kernfs_remove(), the node is guaranteed to be off the rbtree and the above test isn't necessary. !rb_first() test should be enough (provided that there's external synchronization, of course). Thanks. -- tejun