Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 4 Jul 2022 00:53:06 +0300
From:   Dan Aloni <dan.aloni@vastdata.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/4] SUNRPC: Clean up the AUTH cache code
Message-ID: <20220703214628.2zzixg62do6d7jhh@gmail.com>
References: <20181023163404.27777-1-trond.myklebust@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181023163404.27777-1-trond.myklebust@hammerspace.com>
Precedence: bulk

On 2018-10-23 12:34:01, Trond Myklebust wrote:
> @@ -456,32 +495,24 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
>  
>  		if (nr_to_scan-- == 0)
>  			break;
> +		if (atomic_read(&cred->cr_count) > 1) {
> +			rpcauth_lru_remove_locked(cred);
> +			continue;
> +		}
>  		/*
>  		 * Enforce a 60 second garbage collection moratorium
>  		 * Note that the cred_unused list must be time-ordered.
>  		 */
> -		if (time_in_range(cred->cr_expire, expired, jiffies) &&
> -		    test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) != 0) {
> -			freed = SHRINK_STOP;
> -			break;
> -		}
> -
> -		list_del_init(&cred->cr_lru);
> -		number_cred_unused--;
> -		freed++;
> -		if (atomic_read(&cred->cr_count) != 0)
> +		if (!time_in_range(cred->cr_expire, expired, jiffies))
> +			continue;
> +		if (!rpcauth_unhash_cred(cred))
>  			continue;

With `!` flipping the `time_in_range(...)` condition in this past
change, looks like we are skipping the head of the LRU which should be
pruned, so actual expiry does not happen at all in case there are more
than about 100 old items in the LRU.

Reverting this, I saw the correct behavior taking place.


-- >8 --
Subject: [PATCH] sunrpc: fix expiry of auth creds

Due to an inverted condition, instead of pruning the head of the LRU,
the loop stopped short at the beginning.

Fixes: 95cd623250ad ('SUNRPC: Clean up the AUTH cache code')
Signed-off-by: Dan Aloni <dan.aloni@vastdata.com>
---
 net/sunrpc/auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 682fcd24bf43..2324d1e58f21 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -445,7 +445,7 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
 		 * Enforce a 60 second garbage collection moratorium
 		 * Note that the cred_unused list must be time-ordered.
 		 */
-		if (!time_in_range(cred->cr_expire, expired, jiffies))
+		if (time_in_range(cred->cr_expire, expired, jiffies))
 			continue;
 		if (!rpcauth_unhash_cred(cred))
 			continue;
-- 
2.23.0

-- 
Dan Aloni