On 2025-01-16 14:27:47 [+0100], To Tejun Heo wrote: > > > @@ -557,16 +568,18 @@ void kernfs_put(struct kernfs_node *kn) > > > if (!kn || !atomic_dec_and_test(&kn->count)) > > > return; > > > root = kernfs_root(kn); > > > + guard(rcu)(); > > > repeat: > > > /* > > > * Moving/renaming is always done while holding reference. > > > * kn->parent won't change beneath us. > > > */ > > > - parent = kn->parent; > > > + parent = rcu_dereference(kn->parent); > > > > I wonder whether it'd be better to encode the reference count rule (ie. add > > the condition kn->count == 0 to deref_check) in the kn->parent deref > > accessor. This function doesn't need RCU read lock and holding it makes it > > more confusing. > > You are saying that we don't need RCU here because if we drop the last > reference then nobody can rename the node anymore and so parent can't > change. That sounds right. > What about using rcu_dereference_protected() instead? Using > rcu_dereference(x, !atomic_read(&kn->count)) looks odd given that we > established that the counter is 0. Therefore I would suggest > rcu_access_pointer() but the reference drop might qualify as "locked". Ehm or indeed rcu_access_pointer() given that _protected() requires a second argument… Sebastian
