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>
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        Amir Goldstein <amir73il@gmail.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Stephen Brennan <stephen.s.brennan@oracle.com>
Subject: [PATCH v3 3/3] fsnotify: allow sleepable child flag update
Date:   Thu, 27 Oct 2022 17:10:16 -0700
Message-Id: <20221028001016.332663-4-stephen.s.brennan@oracle.com>
In-Reply-To: <20221028001016.332663-1-stephen.s.brennan@oracle.com>
References: <20221021010310.29521-1-stephen.s.brennan@oracle.com>
 <20221028001016.332663-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?bIdq1f3V9ig0Yn9HH1aenPrjNuRF8YuvILfUPkIF+d9phnMeNiBwZskwYvJS?=
 =?us-ascii?Q?BQHept4aEZuLrgjp5g5ZQasogopTbeUKketmwu0FrYkFAJX2f1ZVhlBMXPwJ?=
 =?us-ascii?Q?rAWJKlRxW3WLgD3tRHyjcXz3fkxParroKw++JT2Iok11+VO7/tprrOOX0hOB?=
 =?us-ascii?Q?CzIwj+vhWAqHK78g29PnBw2LVaq4f5L9kA0rCRNNlriREAN83E/CZnzNmKhB?=
 =?us-ascii?Q?uwsJwOMpXKJt0XjZutVtdyNJJOZwtWwPlgMcz/PTtLp3BnJVcHeThjJaZs4w?=
 =?us-ascii?Q?9UhFV9Dd9C6Xf49J3IM8MDi9Gmg4NhdIIGoUYt0RffQ4rkKo/qqAzC4zYK6+?=
 =?us-ascii?Q?cXw9mJh/Ruthj3bSeQsf/DoT1LO56ZCh6CyBLntTRP+iXGYJB6ZFrHsDczyz?=
 =?us-ascii?Q?jTKb56s0ZAK8xv0f+nxLyxVx5Lt6EoLI0X/KyS7SzuSd5zh+2mdE58P0QtC8?=
 =?us-ascii?Q?nHrFPUBYbY502BSxRHMK2ZCV0+EaSP2s7nkmy0YSkawyiR+jo1Ek/DFvD4oJ?=
 =?us-ascii?Q?ur2Wlm+kYlj1O11OAWPsN1DyQcGm56l8yfD6y2orhcYrakuj288ctxQiqAaj?=
 =?us-ascii?Q?xu8Ore62GBJ38+eRajsbGcyiDCLO5r33JtHkX+OJguCMM1T1IL7wvwPnW6Lb?=
 =?us-ascii?Q?oUYCJLiZy7P6OKhjz8IEqdvTtgIYVGxuTxFRzaLZEqm1eWEAT7FYOn6K76zP?=
 =?us-ascii?Q?Nw10oF7dAHESrG+TwNhYvAtEZ0kujbMCwHSdVKaUnicP0Mwr3mNDE8McQotD?=
 =?us-ascii?Q?nF/Rru4Tlzn4V3/wmc8DBD2FFOoBVzk0QomAMJp5qf8eiYvyTATJD5mKm2SY?=
 =?us-ascii?Q?WIIjNdNWxCnWvo0sBpnIs24zaa6gWeHn2WY7OV7FxS1TKgUoDUwPBUTFZS9X?=
 =?us-ascii?Q?nvKSLRijVtm4AI9edr8Z92H7uu3ex6h4O+FaJDrnU7hsi/djnfTxgNiTNoec?=
 =?us-ascii?Q?EpwOlR7OSU6aSYu3FCHNYxGcE0hfoHCq8fo8z5t7Zi7hahLYAyKFKqQ4hPQE?=
 =?us-ascii?Q?z6nLXGQAr42CXOR5sN8dhhzey0WnLTh6v4eJMzrsBQjxG2+w5J1gvHGYqnjx?=
 =?us-ascii?Q?FAbZDPoapoesGmVFCZbW1oOrK9K5AHRxyc9rMSxVNpU48PaD5aFFQ3BFbY2w?=
 =?us-ascii?Q?/SBHmsE7tr+q6z4hlHnKqX9S9vR6jjin/+BLy79nnTtpbRUmjL7W8o/NnyYZ?=
 =?us-ascii?Q?eKFfivcD19+INyfw7LsuBxONFtb7IQZ5bzth8Hz0ELX4A1fkbDWEUiyYzsIq?=
 =?us-ascii?Q?U+NVa8GW/boncN5Cbs6nGoEMHKIsIumWJv5NcyiKZIsdtOwppx1sa3cdGWv5?=
 =?us-ascii?Q?ivINHtxaY3KdX50bHc/Q4yI2x/v59k/ZG21LVa1TM0P6F7huE2CPda36fMfa?=
 =?us-ascii?Q?oVfQHjm0MKXsRPVan1km3h7HkhrBn3Ot117EUslv6P7Gnh0YSapWHiPixlq/?=
 =?us-ascii?Q?vkOy3HpztzwRXiOEuz/CULlbkTZKplkczaY+p4V8xuVMtEDmOMFG1K0/jeQZ?=
 =?us-ascii?Q?HbJiQWpUIaf0tGjHoqC5/Voi1QJ+T4bfxyWpZMdP/VzMMy+cYNGZfjM8wxSg?=
 =?us-ascii?Q?SqA67QB2+Sd5qgt6OcsXI+2GvugXLQxT0P+VRTIARjRM+EYrhQ1kjVnQ+OTl?=
 =?us-ascii?Q?rQ=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c3f7719e-94bb-4dfa-bcad-08dab878d094
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Oct 2022 00:10:26.2903
 (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: 0vcpEOhcbzXykz1naLG50HEHOyXLfv7SVMKlgOmU0l8gDPhG+u8fQfdZMSEt8ri4BBz46Gxaln6dpMjwg0dDo0xPQ/2dL6Jm+pclvKzISqU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4130
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-27_07,2022-10-27_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999
 suspectscore=0 mlxscore=0 spamscore=0 phishscore=0 malwarescore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2210270138
X-Proofpoint-GUID: AV33PWdkUCZRRun01IvCxU5IVK3chsLM
X-Proofpoint-ORIG-GUID: AV33PWdkUCZRRun01IvCxU5IVK3chsLM
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>
---

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