Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
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
Message-Id: <20221018041233.376977-3-stephen.s.brennan@oracle.com>
In-Reply-To: <20221018041233.376977-1-stephen.s.brennan@oracle.com>
References: <20221013222719.277923-1-stephen.s.brennan@oracle.com>
 <20221018041233.376977-1-stephen.s.brennan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?4q7AH11isz7V+EpsANO9k2KKIcjE3+hIgXl0z3XlMySTDdqIWEFke9l9bvif?=
 =?us-ascii?Q?ROpYWxZgvia0kF3tWXofc0/A66FzxneDzeLiXmXnVXi+16KH7EVNXmBSeh+4?=
 =?us-ascii?Q?DnCkcbVd4MMlL64EWf5JOBceyQkGVKFo6ol1lZ9XDytol8BnB1euKzFjN2Vf?=
 =?us-ascii?Q?E9crGJyEOkkzBkSZ0y2pyhDoKV2J4bYk72lNdbJoRMBKOmic4FSUc7du8zsg?=
 =?us-ascii?Q?3yWEf2ipHUiMTDYmwjA1l0Danuq1yzRYNauBSs6b1HBcCgHU4bfFFk2LANp6?=
 =?us-ascii?Q?6Z6SlXEyWcUYP7aYP57tBCCwF6TcK/aaubkvFnIaZIPcm8WLWphAXQgxVqzu?=
 =?us-ascii?Q?VwO3Cr6r8kMRkp1VrySPv/bXo6Fx0RCns16ljkFgg8Mlvm3S8FoaxbCfSQg/?=
 =?us-ascii?Q?/5EieFKUWKJ9R7AXD9hTkx6ZXFuM7zovG3qamdjeHsr2Ig4TPyZEItACrqKC?=
 =?us-ascii?Q?3DpXjT+JGTY+bjTGQZiTNi1qm/7S+jEH0Uu6fH3pf2GKrlTQ7xJP6GSnopbW?=
 =?us-ascii?Q?bq98izBjBkzktfjI7hivJsWPaBrScsyJ82l2u1RfBTs8k2loIHDfbGpNpi1m?=
 =?us-ascii?Q?XGGR9nb5DCIGARaEdJed0LYi9tt5B7La603ohnFGE/bh/unITRy3LtDIaaXh?=
 =?us-ascii?Q?AdJEPtuR7H8u/ElxsmUy5ThHoJq6c4/XuPj/LzYV6A2NlVW10ZDWJBMhbgoO?=
 =?us-ascii?Q?zbNx74sDjS4hVD7Ve4rUpL7vD41kzQXobAK135THkXRc7ynRPEpvcWZ2E99Z?=
 =?us-ascii?Q?edQzZX9x+7NEqqUs9+rxBt0R+LRur/czMBRXu0gagWSbISqvI/nXnDwMVGHz?=
 =?us-ascii?Q?Gr8R1xhRhoSc7ywD0z00ZqzI0d4fvF4FUr0WpCWAU9GA5NpmtPPviFIkWbHu?=
 =?us-ascii?Q?/94p/MQTW4ICfS19CxIK8rqH/S/lVG3mSoNrr/GEVToAM8swVTDf6exF1oi0?=
 =?us-ascii?Q?85ao84Nl5bZ82Ef0+IYM1x/0P5s63xZwd8CCSrYPgISBpQGIG+uojjSroDHT?=
 =?us-ascii?Q?DT9f3GmVYEYnEpsorzrf1M2HJewBErUEKEHOUnr/AYiGrErSXREHYulr01LB?=
 =?us-ascii?Q?Rttcd0J/MNJiLMezkdlqKzIrxe88/S9h6HRsYkJ4/2ZGlk+nC/WNvScWxxuv?=
 =?us-ascii?Q?QvdOinfYBvZGsWVdGJ4u0FnKqiQFmzVXI503vGf3aKlfnBrHyn4uaWpV+OOm?=
 =?us-ascii?Q?7kS/9e4lKTHsM9VSUy0CVvTJq4E9tKpAROiArKnyhukd79i5mC49lC1gJri9?=
 =?us-ascii?Q?igv6X9/oT+2w9m1BLwF4l3wipwxeg5xhzUlpdb3CSIOFH5hijbZTmwgVcJsK?=
 =?us-ascii?Q?Yh70MntPKS39UkqIACckV+zZ1v+bXjdmj+UyMxQ2130Yxk7oL2thbJWQUwtM?=
 =?us-ascii?Q?b0NGxrKh7PT/vRie5qIv1eT+64GPAbdmlb9s9uaqJoW8ym9qzErcATneiuRU?=
 =?us-ascii?Q?dVTDWELoh0NfayCSSsBHnxT5MgjR9rte+Q0bkk1B1x2HrmdsKqedcgyVb/8w?=
 =?us-ascii?Q?ZqJ+n+sYJcGKk2OTKAXy2DHRttw6R4gopeQEcajVRN501sdkdrXrYzzTs3Am?=
 =?us-ascii?Q?uvfX16IgKVVhGKkI3+/JgOrwArWc5oW7ZVnyEdtwRs9dC9nWQ3poE1+Jc24+?=
 =?us-ascii?Q?UA=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fd3df2a1-5cbc-46e3-707e-08dab0befe65
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Oct 2022 04:12:38.5943
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: F49K0fWqHl24ld/fdh0dU6qqjrjQITogcytWSjwwsrDg+3VxgLy8JR/m6zVx2qFzzH9c2xDrPrZ3a34OgS7dLyQ/wTw+1rcthqmVDKqQ+7g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4567
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-17_13,2022-10-17_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 adultscore=0 mlxscore=0
 spamscore=0 bulkscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000
 definitions=main-2210180022
X-Proofpoint-ORIG-GUID: uHVjTIAz4b_Cv31BNQlUmH9Eq0K5Ig-N
X-Proofpoint-GUID: uHVjTIAz4b_Cv31BNQlUmH9Eq0K5Ig-N
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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