On 2024-11-25 15:49:34 [+0100], Michal Koutný wrote: > Hello. Hi Michal, > On Thu, Nov 21, 2024 at 06:52:50PM GMT, Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote: > > > - kernfs_rename_ns() is only using kernfs_rename_lock if the parents are > > different. All users users use either RCU or kernfs_rwsem. > > - kernfs_fop_readdir() drops kernfs_root::kernfs_rwsem while holding a > > reference to name and invoking dir_emit(). This has been changed and > > lock is held. > > - kernfs_notify_workfn() access kernfs_node::name without any > > protection. Added kernfs_root::kernfs_rwsem for the iteration. > > - kernfs_get_parent_dentry() acquires now kernfs_root::kernfs_rwsem > > while accessing the parent node. > > - kernfs_node_dentry() acquires now kernfs_root::kernfs_rwsem while > > parent is accessed and the name looked up. > > Why is the kernfs_root::kernfs_rwsem newly R-taken? Shouldn't be RCU > read section sufficient for those users? Those users. If I skip/ left something out, please poke. kernfs_notify_workfn(). There is ilookup() -> wait_on_inode() which can sleep. kernfs_get_parent_dentry(). There is kernfs_get_inode() -> iget_locked() which can sleep. kernfs_node_dentry(). There is lookup_positive_unlocked() -> lookup_one_unlocked() -> lookup_slow() which might sleep. Assuming the parent can't vanish in these cases, name could during the invocation. I can't keep the RCU read section open while there is a sleep within the call chain. Therefore I added the lock so the rcu_dereference.*() is quiet. > (Perhaps it's related to second observation I have -- why there is > sometimes kernfs_rcu_get_parent() whereas there are other call sites > with mere rcu_dereference(kn->parent)?) rcu_dereference() is used where I was sure that there is always a RCU read section. I have kernfs_rcu_get_parent() when there is either a RCU read section or the kernfs_rwsem (or just the lock). Sebastian
