With very large d_subdirs lists, iteration can take a long time. Since iteration needs to hold parent->d_lock, this can trigger soft lockups. It would be best to make this iteration sleepable. Since we have the inode locked exclusive, we can drop the parent->d_lock and sleep, holding a reference to a child dentry, and continue iteration once we wake. Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx> --- Notes: v3: - removed if statements around dput() v2: - added a check for child->d_parent != alias and retry logic fs/notify/fsnotify.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index ccb8a3a6c522..34e5d18235a7 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -102,10 +102,12 @@ void fsnotify_sb_delete(struct super_block *sb) * on a child we run all of our children and set a dentry flag saying that the * parent cares. Thus when an event happens on a child it can quickly tell * if there is a need to find a parent and send the event to the parent. + * + * Context: inode locked exclusive */ static bool __fsnotify_update_children_dentry_flags(struct inode *inode) { - struct dentry *alias, *child; + struct dentry *child, *alias, *last_ref = NULL; int watched; if (!S_ISDIR(inode->i_mode)) @@ -120,12 +122,37 @@ static bool __fsnotify_update_children_dentry_flags(struct inode *inode) alias = d_find_any_alias(inode); /* - * run all of the children of the original inode and fix their - * d_flags to indicate parental interest (their parent is the - * original inode) + * These lists can get very long, so we may need to sleep during + * iteration. Normally this would be impossible without a cursor, + * but since we have the inode locked exclusive, we're guaranteed + * that the directory won't be modified, so whichever dentry we + * pick to sleep on won't get moved. So, start a manual iteration + * over d_subdirs which will allow us to sleep. */ spin_lock(&alias->d_lock); +retry: list_for_each_entry(child, &alias->d_subdirs, d_child) { + if (need_resched()) { + /* + * We need to hold a reference while we sleep. But when + * we wake, dput() could free the dentry, invalidating + * the list pointers. We can't look at the list pointers + * until we re-lock the parent, and we can't dput() once + * we have the parent locked. So the solution is to hold + * onto our reference and free it the *next* time we drop + * alias->d_lock: either at the end of the function, or + * at the time of the next sleep. + */ + dget(child); + spin_unlock(&alias->d_lock); + dput(last_ref); + last_ref = child; + cond_resched(); + spin_lock(&alias->d_lock); + if (child->d_parent != alias) + goto retry; + } + if (!child->d_inode) continue; @@ -137,6 +164,7 @@ static bool __fsnotify_update_children_dentry_flags(struct inode *inode) spin_unlock(&child->d_lock); } spin_unlock(&alias->d_lock); + dput(last_ref); dput(alias); return watched; } -- 2.34.1