Message-Id: <20250601232937.3510379-7-sashal@kernel.org>
Date: Sun,  1 Jun 2025 19:27:59 -0400
From: Sasha Levin <sashal@...nel.org>
To: patches@...ts.linux.dev,
	stable@...r.kernel.org
Cc: Long Li <leo.lilong@...wei.com>,
	Chuck Lever <chuck.lever@...cle.com>,
	Sasha Levin <sashal@...nel.org>,
	jlayton@...nel.org,
	trondmy@...nel.org,
	anna@...nel.org,
	davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	linux-nfs@...r.kernel.org,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH AUTOSEL 6.14 007/102] sunrpc: update nextcheck time when adding new cache entries

From: Long Li <leo.lilong@...wei.com>

[ Upstream commit 5ca00634c8bbb2979c73465588f486b9632f5ed5 ]

The cache_detail structure uses a "nextcheck" field to control hash table
scanning intervals. When a table scan begins, nextcheck is set to current
time plus 1800 seconds. During scanning, if cache_detail is not empty and
a cache entry's expiry time is earlier than the current nextcheck, the
nextcheck is updated to that expiry time.

This mechanism ensures that:
1) Empty cache_details are scanned every 1800 seconds to avoid unnecessary
   scans
2) Non-empty cache_details are scanned based on the earliest expiry time
   found

However, when adding a new cache entry to an empty cache_detail, the
nextcheck time was not being updated, remaining at 1800 seconds. This
could delay cache cleanup for up to 1800 seconds, potentially blocking
threads(such as nfsd) that are waiting for cache cleanup.

Fix this by updating the nextcheck time whenever a new cache entry is
added.

Signed-off-by: Long Li <leo.lilong@...wei.com>
Signed-off-by: Chuck Lever <chuck.lever@...cle.com>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---

Based on my analysis of the commit, the code changes, and the kernel
repository context, here is my assessment: **YES** This commit should be
backported to stable kernel trees. Here's my extensive analysis: ##
Analysis of the Problem The commit addresses a specific cache management
issue in the SUNRPC subsystem that can cause significant delays (up to
1800 seconds/30 minutes) in cache cleanup operations. The problem occurs
when: 1. A cache_detail starts empty, with nextcheck set to current time
+ 1800 seconds 2. A new cache entry is added to this empty cache_detail
3. The nextcheck time is NOT updated to reflect the new entry's expiry
time 4. This means cache cleanup won't occur until the original
1800-second timeout expires 5. During this delay, threads like nfsd that
depend on cache cleanup can be blocked ## Code Change Analysis The fix
is minimal and surgical: ```c + if (detail->nextcheck >
new->expiry_time) + detail->nextcheck = new->expiry_time + 1; ``` This
2-line addition in `sunrpc_cache_add_entry()` ensures that when a new
cache entry is added, if its expiry time is earlier than the current
nextcheck time, nextcheck is updated to trigger cleanup at the
appropriate time. ## Why This Qualifies for Stable Backporting **1. Bug
Fix Nature**: This clearly fixes a bug where cache cleanup could be
inappropriately delayed, affecting system functionality. **2. Minimal
Risk**: The change is extremely small and contained. It only adds a
simple conditional check that makes the cache cleanup more responsive.
There's no architectural change or complex logic. **3. Critical
Subsystem Impact**: The SUNRPC cache affects NFS operations, which are
critical for many production systems. A 1800-second delay in cache
cleanup can severely impact NFS server responsiveness. **4. Clear User
Impact**: The commit message explicitly mentions that this can block
threads like nfsd, which directly affects users experiencing NFS service
delays or hangs. **5. Historical Context**: Looking at the similar
commits provided as reference, this fix is actually addressing a more
fundamental timing issue than some of the other cache-related changes
that were marked as "NO" for backporting. Those commits dealt with
interface behavior changes or feature additions, while this fixes a
clear performance/delay bug. **6. Low Complexity**: The fix doesn't
introduce new features, change APIs, or modify behavioral contracts. It
simply ensures that cache cleanup happens when it should happen. **7.
Contained Scope**: The change is isolated to a single function
(`sunrpc_cache_add_entry`) and only affects the timing of cache cleanup,
not the cleanup logic itself. ## Comparison with Reference Commits
Unlike the reference commits marked "NO" which involved: - Interface
behavior changes (flush interface semantics) - Feature additions (new
cache operations) - Complex timing logic modifications - Architectural
changes This commit simply fixes a timing bug with a minimal, obvious
correction that ensures cache cleanup occurs at the appropriate time
rather than being unnecessarily delayed. The fix follows the stable tree
rules perfectly: it's an important bugfix with minimal risk that
addresses a clear user-visible issue in a critical kernel subsystem.

 net/sunrpc/cache.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 7ce5e28a6c031..bbaa77d7bbc81 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -135,6 +135,8 @@ static struct cache_head *sunrpc_cache_add_entry(struct cache_detail *detail,
 
 	hlist_add_head_rcu(&new->cache_list, head);
 	detail->entries++;
+	if (detail->nextcheck > new->expiry_time)
+		detail->nextcheck = new->expiry_time + 1;
 	cache_get(new);
 	spin_unlock(&detail->hash_lock);
 
-- 
2.39.5

Powered by blists - more mailing lists