[PATCH v3 2/2] lockd: Serialize block retries against host teardown

Chuck Lever <[email protected]>
Newsgroups org.kernel.vger.linux-nfs
Message-ID <[email protected]>
nlmsvc_grant_blocked() unlinks a block from nlm_blocked before it
retries the lock, then re-inserts it. nlmsvc_traverse_blocks() skips a
block that is not on nlm_blocked, so a teardown scan that runs during a
retry passes it by and the retry puts it back. The surviving block pins
its host. lockd warns that it could not shut down the host module, and
the host outlives its network namespace.

Hold the file's f_mutex across the retry, and extend the scan's hold
across its unlink, so a scan and a retry of the same file can no longer
interleave. Drop the mutex before releasing a block reference, since
the last put takes f_mutex. A retry that waited out a scan re-checks
under nlm_blocked_lock that its block is still queued and due.

Reported-by: sashiko-bot <[email protected]>
Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
Signed-off-by: Chuck Lever <[email protected]>
---
 fs/lockd/svclock.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 8d83283d3e21..495eacb3264f 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -295,14 +295,17 @@ void nlmsvc_traverse_blocks(struct nlm_host *host,
 	list_for_each_entry_safe(block, next, &file->f_blocks, b_flist) {
 		if (!match(block->b_host, host))
 			continue;
-		/* Do not destroy blocks that are not on
-		 * the global retry list - why? */
+		/*
+		 * nlmsvc_retry_blocked() holds f_mutex while the block
+		 * is off nlm_blocked, so a block off the list here has
+		 * been retired.
+		 */
 		if (list_empty(&block->b_list))
 			continue;
 		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
-		mutex_unlock(&file->f_mutex);
 		nlmsvc_unlink_block(block);
+		mutex_unlock(&file->f_mutex);
 		nlmsvc_release_block(block);
 		goto restart;
 	}
@@ -1012,6 +1015,8 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 {
 	unsigned long	timeout = MAX_SCHEDULE_TIMEOUT;
 	struct nlm_block *block;
+	struct nlm_file *file;
+	bool due;
 
 	spin_lock(&nlm_blocked_lock);
 	while (!list_empty(&nlm_blocked) && !svc_thread_should_stop(rqstp)) {
@@ -1026,6 +1031,25 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
 
+		/*
+		 * Hold f_mutex so nlmsvc_traverse_blocks() cannot scan
+		 * the file while the retry has the block off nlm_blocked.
+		 */
+		file = block->b_file;
+		mutex_lock(&file->f_mutex);
+		spin_lock(&nlm_blocked_lock);
+		due = !list_empty(&block->b_list) &&
+		      block->b_when != NLM_NEVER &&
+		      !time_after(block->b_when, jiffies);
+		spin_unlock(&nlm_blocked_lock);
+
+		if (!due) {
+			mutex_unlock(&file->f_mutex);
+			nlmsvc_release_block(block);
+			spin_lock(&nlm_blocked_lock);
+			continue;
+		}
+
 		dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
 			block, block->b_when);
 		if (block->b_flags & B_QUEUED) {
@@ -1034,6 +1058,7 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 			retry_deferred_block(block);
 		} else
 			nlmsvc_grant_blocked(block);
+		mutex_unlock(&file->f_mutex);
 		nlmsvc_release_block(block);
 		spin_lock(&nlm_blocked_lock);
 	}

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.