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>
---
fs/notify/fsnotify.c | 72 ++++++++++++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 22 deletions(-)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index e887a195983b..499b19272b32 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -102,10 +102,13 @@ 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
*/
void __fsnotify_update_child_dentry_flags(struct inode *inode)
{
- struct dentry *alias;
+ struct dentry *child, *alias, *last_ref = NULL;
+ struct list_head *p;
int watched;
if (!S_ISDIR(inode->i_mode))
@@ -114,30 +117,55 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
/* determine if the children should tell inode about their events */
watched = fsnotify_inode_watches_children(inode);
- 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;
+ alias = d_find_any_alias(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);
+ p = alias->d_subdirs.next;
- 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);
+ while (p != &alias->d_subdirs) {
+ child = list_entry(p, struct dentry, 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);
+ if (last_ref)
+ dput(last_ref);
+ last_ref = child;
+ cond_resched();
+ spin_lock(&alias->d_lock);
}
- spin_unlock(&alias->d_lock);
+ p = p->next;
+
+ 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);
+ if (last_ref)
+ dput(last_ref);
}
/* Are inode/sb/mount interested in parent and name info with this event? */
--
2.34.1
