Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Waiman Long <longman@redhat.com>
To:     Alexander Viro <viro@zeniv.linux.org.uk>,
        Jonathan Corbet <corbet@lwn.net>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Iurii Zaikin <yzaikin@google.com>
Cc:     linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-doc@vger.kernel.org,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        Eric Biggers <ebiggers@google.com>,
        Dave Chinner <david@fromorbit.com>,
        Eric Sandeen <sandeen@redhat.com>,
        Waiman Long <longman@redhat.com>
Subject: [PATCH 08/11] fs/dcache: Limit dentry reclaim count in negative_reclaim_workfn()
Date:   Wed, 26 Feb 2020 11:14:01 -0500
Message-Id: <20200226161404.14136-9-longman@redhat.com>
In-Reply-To: <20200226161404.14136-1-longman@redhat.com>
References: <20200226161404.14136-1-longman@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

To limit the d_lock hold time of directory dentry in the negative dentry
reclaim process, a quota (currently 64k) is added to limit the amount of
work that the work function can do and hence its execution time. This is
done to minimize impact on other processes that depend on that d_lock or
other work functions in the same work queue from excessive delay.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 fs/dcache.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 149c0a6c1a6e..0bd5d6974f75 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1374,10 +1374,25 @@ static inline void init_dentry_iname(struct dentry *dentry)
 	set_dentry_npositive(dentry, 0);
 }
 
+/*
+ * In the pathological case where a large number of negative dentries are
+ * generated in a short time in a given directory, there is a possibility
+ * that negative dentries reclaiming process will have many dentries to
+ * be dispose of. Thus the d_lock lock can be hold for too long impacting
+ * other running processes that need it.
+ *
+ * There is also the consideration that a long runtime will impact other
+ * work functions that need to be run in the same work queue. As a result,
+ * we have to limit the number of dentries that can be reclaimed in each
+ * invocation of the work function.
+ */
+#define MAX_DENTRY_RECLAIM	(1 << 16)
+
 /*
  * Reclaim excess negative dentries in a directory
+ * Return: true if the work function needs to be rescheduled, false otherwise
  */
-static void reclaim_negative_dentry(struct dentry *parent,
+static void reclaim_negative_dentry(struct dentry *parent, int *quota,
 				    struct list_head *dispose)
 {
 	struct dentry *child;
@@ -1394,9 +1409,16 @@ static void reclaim_negative_dentry(struct dentry *parent,
 	 */
 	if (cnt <= limit)
 		return;
+
+	npositive = 0;
 	cnt -= limit;
 	cnt += (limit >> 3);
-	npositive = 0;
+	if (cnt >= *quota) {
+		cnt = *quota;
+		*quota = 0;
+	} else {
+		*quota -= cnt;
+	}
 
 	/*
 	 * The d_subdirs is treated like a kind of LRU where
@@ -1462,6 +1484,8 @@ static void reclaim_negative_dentry(struct dentry *parent,
 	}
 	if (dentry_has_npositive(parent))
 		set_dentry_npositive(parent, npositive);
+
+	*quota += cnt;
 }
 
 /*
@@ -1472,6 +1496,7 @@ static void negative_reclaim_workfn(struct work_struct *work)
 	struct llist_node *nodes, *next;
 	struct dentry *parent;
 	struct reclaim_dentry *dentry_node;
+	int quota = MAX_DENTRY_RECLAIM;
 
 	/*
 	 * Collect excess negative dentries in dispose list & shrink them.
@@ -1486,10 +1511,10 @@ static void negative_reclaim_workfn(struct work_struct *work)
 		parent = dentry_node->parent_dir;
 		spin_lock(&parent->d_lock);
 
-		if (d_is_dir(parent) &&
+		if (d_is_dir(parent) && quota &&
 		    can_reclaim_dentry(parent->d_flags) &&
 		    (parent->d_flags & DCACHE_RECLAIMING))
-			reclaim_negative_dentry(parent, &dispose);
+			reclaim_negative_dentry(parent, &quota, &dispose);
 
 		if (!list_empty(&dispose)) {
 			spin_unlock(&parent->d_lock);
-- 
2.18.1