linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Brennan <stephen.s.brennan@oracle.com>
To: Jan Kara <jack@suse.cz>, Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Amir Goldstein <amir73il@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Stephen Brennan <stephen.s.brennan@oracle.com>
Subject: [PATCH 2/2] fsnotify: allow sleepable child flag update
Date: Mon, 17 Oct 2022 21:12:33 -0700	[thread overview]
Message-ID: <20221018041233.376977-3-stephen.s.brennan@oracle.com> (raw)
In-Reply-To: <20221018041233.376977-1-stephen.s.brennan@oracle.com>

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@oracle.com>
---
 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


  parent reply	other threads:[~2022-10-18  4:12 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-13 22:27 [RFC] fsnotify: allow sleepable child dentry flag update Stephen Brennan
2022-10-13 23:51 ` Al Viro
2022-11-01 21:47   ` Stephen Brennan
2022-10-14  8:01 ` Amir Goldstein
2022-10-17  7:59   ` Stephen Brennan
2022-10-17 11:44     ` Amir Goldstein
2022-10-17 16:59       ` Stephen Brennan
2022-10-17 17:42         ` Amir Goldstein
2022-10-17  9:09   ` Jan Kara
2022-10-18  4:12 ` [PATCH 0/2] fsnotify: fix softlockups iterating over d_subdirs Stephen Brennan
2022-10-18  4:12   ` [PATCH 1/2] fsnotify: Protect i_fsnotify_mask and child flags with inode rwsem Stephen Brennan
2022-10-18  7:39     ` Amir Goldstein
2022-10-21  0:33       ` Stephen Brennan
2022-10-21  7:22         ` Amir Goldstein
2022-10-18  4:12   ` Stephen Brennan [this message]
2022-10-18  5:36     ` [PATCH 2/2] fsnotify: allow sleepable child flag update Amir Goldstein
2022-10-27  7:50     ` kernel test robot
2022-10-27  8:44       ` Yujie Liu
2022-10-27 22:12         ` Stephen Brennan
2022-10-18  8:07   ` [PATCH 0/2] fsnotify: fix softlockups iterating over d_subdirs Amir Goldstein
2022-10-18 23:52     ` Stephen Brennan
2022-10-19  5:33       ` Amir Goldstein
2022-10-27 22:06         ` Stephen Brennan
2022-10-28  8:58           ` Amir Goldstein
2022-10-21  1:03   ` [PATCH v2 0/3] " Stephen Brennan
2022-10-21  1:03     ` [PATCH v2 1/3] fsnotify: Use d_find_any_alias to get dentry associated with inode Stephen Brennan
2022-10-21  9:25       ` Amir Goldstein
2022-10-21  1:03     ` [PATCH v2 2/3] fsnotify: Protect i_fsnotify_mask and child flags with inode rwsem Stephen Brennan
2022-10-21  4:01       ` kernel test robot
2022-10-21  8:22       ` Amir Goldstein
2022-10-21  9:18         ` Amir Goldstein
2022-10-25 18:02           ` Stephen Brennan
2022-10-26  5:41             ` Amir Goldstein
2022-10-21  9:17       ` Christian Brauner
2022-10-21  9:21         ` Amir Goldstein
2022-10-21  1:03     ` [PATCH v2 3/3] fsnotify: allow sleepable child flag update Stephen Brennan
2022-10-28  0:10     ` [PATCH v3 0/3] fsnotify: fix softlockups iterating over d_subdirs Stephen Brennan
2022-10-28  0:10       ` [PATCH v3 1/3] fsnotify: Use d_find_any_alias to get dentry associated with inode Stephen Brennan
2022-11-10  1:12         ` Stephen Brennan
2022-10-28  0:10       ` [PATCH v3 2/3] fsnotify: Protect i_fsnotify_mask and child flags with inode rwsem Stephen Brennan
2022-10-28  9:11         ` Amir Goldstein
2022-11-10  0:03         ` kernel test robot
2022-11-10  1:06           ` Stephen Brennan
2022-10-28  0:10       ` [PATCH v3 3/3] fsnotify: allow sleepable child flag update Stephen Brennan
2022-10-28  9:32         ` Amir Goldstein
2022-11-01 21:25           ` Stephen Brennan
2022-11-01 17:51       ` [PATCH v3 0/3] fsnotify: fix softlockups iterating over d_subdirs Jan Kara
2022-11-01 20:48         ` Stephen Brennan
2022-11-02  8:55           ` Amir Goldstein
2022-11-10 20:04             ` Stephen Brennan
2022-11-02 17:52           ` Jan Kara
2022-11-04 23:33             ` Stephen Brennan
2022-11-07 11:56               ` Jan Kara
2022-11-11 22:06       ` [PATCH v4 0/5] " Stephen Brennan
2022-11-11 22:06         ` [PATCH v4 1/5] fsnotify: clear PARENT_WATCHED flags lazily Stephen Brennan
2022-11-11 22:06         ` [PATCH v4 2/5] fsnotify: Use d_find_any_alias to get dentry associated with inode Stephen Brennan
2022-11-12  8:53           ` Amir Goldstein
2022-11-11 22:06         ` [PATCH v4 3/5] dnotify: move fsnotify_recalc_mask() outside spinlock Stephen Brennan
2022-11-12  9:06           ` Amir Goldstein
2022-11-11 22:06         ` [PATCH v4 4/5] fsnotify: allow sleepable child flag update Stephen Brennan
2022-11-12 10:00           ` Amir Goldstein
2022-11-15  7:10           ` kernel test robot
2022-11-11 22:06         ` [PATCH v4 5/5] fsnotify: require inode lock held during " Stephen Brennan
2022-11-12  9:42           ` Amir Goldstein
2022-11-11 22:08         ` [PATCH v4 0/5] fsnotify: fix softlockups iterating over d_subdirs Stephen Brennan
2022-11-22 11:50         ` Jan Kara
2022-11-22 14:03           ` Amir Goldstein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221018041233.376977-3-stephen.s.brennan@oracle.com \
    --to=stephen.s.brennan@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).