]> www.infradead.org Git - users/hch/misc.git/commitdiff
sunrpc: fix race in cache cleanup causing stale nextcheck time
authorLong Li <leo.lilong@huawei.com>
Sat, 1 Mar 2025 06:48:36 +0000 (14:48 +0800)
committerChuck Lever <chuck.lever@oracle.com>
Sun, 11 May 2025 23:48:22 +0000 (19:48 -0400)
When cache cleanup runs concurrently with cache entry removal, a race
condition can occur that leads to incorrect nextcheck times. This can
delay cache cleanup for the cache_detail by up to 1800 seconds:

1. cache_clean() sets nextcheck to current time plus 1800 seconds
2. While scanning a non-empty bucket, concurrent cache entry removal can
   empty that bucket
3. cache_clean() finds no cache entries in the now-empty bucket to update
   the nextcheck time
4. This maybe delays the next scan of the cache_detail by up to 1800
   seconds even when it should be scanned earlier based on remaining
   entries

Fix this by moving the hash_lock acquisition earlier in cache_clean().
This ensures bucket emptiness checks and nextcheck updates happen
atomically, preventing the race between cleanup and entry removal.

Signed-off-by: Long Li <leo.lilong@huawei.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
net/sunrpc/cache.c

index bbaa77d7bbc81593e4436666d62e3875a52b2457..131090f31e6a8322cf819e004cde151c1e5c0b64 100644 (file)
@@ -464,24 +464,21 @@ static int cache_clean(void)
                }
        }
 
+       spin_lock(&current_detail->hash_lock);
+
        /* find a non-empty bucket in the table */
-       while (current_detail &&
-              current_index < current_detail->hash_size &&
+       while (current_index < current_detail->hash_size &&
               hlist_empty(&current_detail->hash_table[current_index]))
                current_index++;
 
        /* find a cleanable entry in the bucket and clean it, or set to next bucket */
-
-       if (current_detail && current_index < current_detail->hash_size) {
+       if (current_index < current_detail->hash_size) {
                struct cache_head *ch = NULL;
                struct cache_detail *d;
                struct hlist_head *head;
                struct hlist_node *tmp;
 
-               spin_lock(&current_detail->hash_lock);
-
                /* Ok, now to clean this strand */
-
                head = &current_detail->hash_table[current_index];
                hlist_for_each_entry_safe(ch, tmp, head, cache_list) {
                        if (current_detail->nextcheck > ch->expiry_time)
@@ -502,8 +499,10 @@ static int cache_clean(void)
                spin_unlock(&cache_list_lock);
                if (ch)
                        sunrpc_end_cache_remove_entry(ch, d);
-       } else
+       } else {
+               spin_unlock(&current_detail->hash_lock);
                spin_unlock(&cache_list_lock);
+       }
 
        return rv;
 }