On Sat, Nov 12, 2022 at 12:06 AM Stephen Brennan <stephen.s.brennan@xxxxxxxxxx> wrote: > > Rather than iterating over the inode's i_dentry (requiring holding the > i_lock for the entire duration of the function), we know that there > should be only one item in the list. Use d_find_any_alias() and no > longer hold i_lock. > > Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> > --- > > Notes: > Changes in v4: > - Bail out if d_find_any_alias() returns NULL > - Rebase on Amir's patch > Changes in v3: > - Add newlines in block comment > - d_find_any_alias() returns a reference, which I was leaking. Add > a dput(alias) at the end. > - Add Amir's R-b > > fs/notify/fsnotify.c | 46 ++++++++++++++++++++++---------------------- > 1 file changed, 23 insertions(+), 23 deletions(-) > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index 2c50e9e50d35..409d479cbbc6 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -105,35 +105,35 @@ void fsnotify_sb_delete(struct super_block *sb) > */ > void fsnotify_update_children_dentry_flags(struct inode *inode, bool watched) > { > - struct dentry *alias; > + struct dentry *alias, *child; > > if (!S_ISDIR(inode->i_mode)) > return; > > - spin_lock(&inode->i_lock); > - /* run all of the dentries associated with this inode. Since this is a > - * directory, there damn well better only be one item on this list */ > - hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) { > - struct dentry *child; > - > - /* run all of the children of the original inode and fix their > - * d_flags to indicate parental interest (their parent is the > - * original inode) */ > - spin_lock(&alias->d_lock); > - list_for_each_entry(child, &alias->d_subdirs, d_child) { > - if (!child->d_inode) > - continue; > + /* Since this is a directory, there damn well better only be one child */ > + alias = d_find_any_alias(inode); > + if (!alias) > + return; > > - spin_lock_nested(&child->d_lock, DENTRY_D_LOCK_NESTED); > - if (watched) > - child->d_flags |= DCACHE_FSNOTIFY_PARENT_WATCHED; > - else > - child->d_flags &= ~DCACHE_FSNOTIFY_PARENT_WATCHED; > - spin_unlock(&child->d_lock); > - } > - spin_unlock(&alias->d_lock); > + /* > + * run all of the children of the original inode and fix their > + * d_flags to indicate parental interest (their parent is the > + * original inode) nit: this comment can probably fit in two nicer lines > + */ > + spin_lock(&alias->d_lock); > + list_for_each_entry(child, &alias->d_subdirs, d_child) { > + if (!child->d_inode) > + continue; > + > + spin_lock_nested(&child->d_lock, DENTRY_D_LOCK_NESTED); > + if (watched) > + child->d_flags |= DCACHE_FSNOTIFY_PARENT_WATCHED; > + else > + child->d_flags &= ~DCACHE_FSNOTIFY_PARENT_WATCHED; > + spin_unlock(&child->d_lock); > } > - spin_unlock(&inode->i_lock); > + spin_unlock(&alias->d_lock); > + dput(alias); > } > > /* > -- > 2.34.1 >