Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/auth.c | 147 ++++++++++++++++++++++++-----------------
net/sunrpc/auth_null.c | 2 +-
2 files changed, 87 insertions(+), 62 deletions(-)
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 32985aa515be..c1576b110974 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -291,25 +291,30 @@ rpcauth_release(struct rpc_auth *auth)
static DEFINE_SPINLOCK(rpc_credcache_lock);
-static void
+/*
+ * On success, the caller is responsible for freeing the reference
+ * held by the hashtable
+ */
+static bool
rpcauth_unhash_cred_locked(struct rpc_cred *cred)
{
+ if (!test_and_clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags))
+ return false;
hlist_del_rcu(&cred->cr_hash);
- smp_mb__before_atomic();
- clear_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
+ return true;
}
-static int
+static bool
rpcauth_unhash_cred(struct rpc_cred *cred)
{
spinlock_t *cache_lock;
- int ret;
+ bool ret;
+ if (!test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags))
+ return false;
cache_lock = &cred->cr_auth->au_credcache->lock;
spin_lock(cache_lock);
- ret = atomic_read(&cred->cr_count) == 0;
- if (ret)
- rpcauth_unhash_cred_locked(cred);
+ ret = rpcauth_unhash_cred_locked(cred);
spin_unlock(cache_lock);
return ret;
}
@@ -388,6 +393,44 @@ void rpcauth_destroy_credlist(struct list_head *head)
}
}
+static void
+rpcauth_lru_add_locked(struct rpc_cred *cred)
+{
+ if (!list_empty(&cred->cr_lru))
+ return;
+ number_cred_unused++;
+ list_add_tail(&cred->cr_lru, &cred_unused);
+}
+
+static void
+rpcauth_lru_add(struct rpc_cred *cred)
+{
+ if (!list_empty(&cred->cr_lru))
+ return;
+ spin_lock(&rpc_credcache_lock);
+ rpcauth_lru_add_locked(cred);
+ spin_unlock(&rpc_credcache_lock);
+}
+
+static void
+rpcauth_lru_remove_locked(struct rpc_cred *cred)
+{
+ if (list_empty(&cred->cr_lru))
+ return;
+ number_cred_unused--;
+ list_del_init(&cred->cr_lru);
+}
+
+static void
+rpcauth_lru_remove(struct rpc_cred *cred)
+{
+ if (list_empty(&cred->cr_lru))
+ return;
+ spin_lock(&rpc_credcache_lock);
+ rpcauth_lru_remove_locked(cred);
+ spin_unlock(&rpc_credcache_lock);
+}
+
/*
* Clear the RPC credential cache, and delete those credentials
* that are not referenced.
@@ -407,13 +450,10 @@ rpcauth_clear_credcache(struct rpc_cred_cache *cache)
head = &cache->hashtable[i];
while (!hlist_empty(head)) {
cred = hlist_entry(head->first, struct rpc_cred, cr_hash);
- get_rpccred(cred);
- if (!list_empty(&cred->cr_lru)) {
- list_del(&cred->cr_lru);
- number_cred_unused--;
- }
- list_add_tail(&cred->cr_lru, &free);
rpcauth_unhash_cred_locked(cred);
+ /* Note: We now hold a reference to cred */
+ rpcauth_lru_remove_locked(cred);
+ list_add_tail(&cred->cr_lru, &free);
}
}
spin_unlock(&cache->lock);
@@ -447,7 +487,6 @@ EXPORT_SYMBOL_GPL(rpcauth_destroy_credcache);
static long
rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
{
- spinlock_t *cache_lock;
struct rpc_cred *cred, *next;
unsigned long expired = jiffies - RPC_AUTH_EXPIRY_MORATORIUM;
long freed = 0;
@@ -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;
- cache_lock = &cred->cr_auth->au_credcache->lock;
- spin_lock(cache_lock);
- if (atomic_read(&cred->cr_count) == 0) {
- get_rpccred(cred);
- list_add_tail(&cred->cr_lru, free);
- rpcauth_unhash_cred_locked(cred);
- }
- spin_unlock(cache_lock);
+ rpcauth_lru_remove_locked(cred);
+ freed++;
+ list_add_tail(&cred->cr_lru, free);
}
- return freed;
+ return freed ? freed : SHRINK_STOP;
}
static unsigned long
@@ -595,6 +626,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
if (cred == NULL) {
cred = new;
set_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
+ atomic_inc(&cred->cr_count);
hlist_add_head_rcu(&cred->cr_hash, &cache->hashtable[nr]);
} else
list_add_tail(&new->cr_lru, &free);
@@ -709,36 +741,29 @@ put_rpccred(struct rpc_cred *cred)
{
if (cred == NULL)
return;
- /* Fast path for unhashed credentials */
- if (test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) == 0) {
+ rcu_read_lock();
+ if (atomic_dec_and_test(&cred->cr_count))
+ goto destroy;
+ if (atomic_read(&cred->cr_count) != 1 ||
+ !test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags))
+ goto out;
+ if (test_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags) != 0) {
+ cred->cr_expire = jiffies;
+ rpcauth_lru_add(cred);
+ /* Race breaker */
+ if (unlikely(!test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags)))
+ rpcauth_lru_remove(cred);
+ } else if (rpcauth_unhash_cred(cred)) {
+ rpcauth_lru_remove(cred);
if (atomic_dec_and_test(&cred->cr_count))
- cred->cr_ops->crdestroy(cred);
- return;
- }
-
- if (!atomic_dec_and_lock(&cred->cr_count, &rpc_credcache_lock))
- return;
- if (!list_empty(&cred->cr_lru)) {
- number_cred_unused--;
- list_del_init(&cred->cr_lru);
- }
- if (test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) != 0) {
- if (test_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags) != 0) {
- cred->cr_expire = jiffies;
- list_add_tail(&cred->cr_lru, &cred_unused);
- number_cred_unused++;
- goto out_nodestroy;
- }
- if (!rpcauth_unhash_cred(cred)) {
- /* We were hashed and someone looked us up... */
- goto out_nodestroy;
- }
+ goto destroy;
}
- spin_unlock(&rpc_credcache_lock);
- cred->cr_ops->crdestroy(cred);
+out:
+ rcu_read_unlock();
return;
-out_nodestroy:
- spin_unlock(&rpc_credcache_lock);
+destroy:
+ rcu_read_unlock();
+ cred->cr_ops->crdestroy(cred);
}
EXPORT_SYMBOL_GPL(put_rpccred);
diff --git a/net/sunrpc/auth_null.c b/net/sunrpc/auth_null.c
index 4b48228ee8c7..a7c00b4959f3 100644
--- a/net/sunrpc/auth_null.c
+++ b/net/sunrpc/auth_null.c
@@ -138,6 +138,6 @@ struct rpc_cred null_cred = {
.cr_lru = LIST_HEAD_INIT(null_cred.cr_lru),
.cr_auth = &null_auth,
.cr_ops = &null_credops,
- .cr_count = ATOMIC_INIT(1),
+ .cr_count = ATOMIC_INIT(2),
.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE,
};
--
2.17.2
We no longer need to worry about whether or not the entry is hashed in
order to figure out if the contents are valid. We only care whether or
not the refcount is non-zero.
Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/sunrpc/auth.h | 12 +++++-------
net/sunrpc/auth.c | 19 ++++++++-----------
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index 2c97a3933ef9..a7fc8f5a2dad 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -206,11 +206,11 @@ bool rpcauth_cred_key_to_expire(struct rpc_auth *, struct rpc_cred *);
char * rpcauth_stringify_acceptor(struct rpc_cred *);
static inline
-struct rpc_cred * get_rpccred(struct rpc_cred *cred)
+struct rpc_cred *get_rpccred(struct rpc_cred *cred)
{
- if (cred != NULL)
- atomic_inc(&cred->cr_count);
- return cred;
+ if (cred != NULL && atomic_inc_not_zero(&cred->cr_count))
+ return cred;
+ return NULL;
}
/**
@@ -226,9 +226,7 @@ struct rpc_cred * get_rpccred(struct rpc_cred *cred)
static inline struct rpc_cred *
get_rpccred_rcu(struct rpc_cred *cred)
{
- if (atomic_inc_not_zero(&cred->cr_count))
- return cred;
- return NULL;
+ return get_rpccred(cred);
}
#endif /* __KERNEL__ */
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index c1576b110974..77748e572686 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -588,19 +588,15 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
if (!entry->cr_ops->crmatch(acred, entry, flags))
continue;
if (flags & RPCAUTH_LOOKUP_RCU) {
- if (test_bit(RPCAUTH_CRED_HASHED, &entry->cr_flags) &&
- !test_bit(RPCAUTH_CRED_NEW, &entry->cr_flags))
- cred = entry;
+ if (test_bit(RPCAUTH_CRED_NEW, &entry->cr_flags) ||
+ atomic_read(&entry->cr_count) == 0)
+ continue;
+ cred = entry;
break;
}
- spin_lock(&cache->lock);
- if (test_bit(RPCAUTH_CRED_HASHED, &entry->cr_flags) == 0) {
- spin_unlock(&cache->lock);
- continue;
- }
cred = get_rpccred(entry);
- spin_unlock(&cache->lock);
- break;
+ if (cred)
+ break;
}
rcu_read_unlock();
@@ -621,7 +617,8 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
if (!entry->cr_ops->crmatch(acred, entry, flags))
continue;
cred = get_rpccred(entry);
- break;
+ if (cred)
+ break;
}
if (cred == NULL) {
cred = new;
--
2.17.2
Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/sunrpc/auth.h | 4 ++--
net/sunrpc/auth.c | 14 +++++++-------
net/sunrpc/auth_null.c | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index a7fc8f5a2dad..a71d4bd191e7 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -67,7 +67,7 @@ struct rpc_cred {
const struct rpc_credops *cr_ops;
unsigned long cr_expire; /* when to gc */
unsigned long cr_flags; /* various flags */
- atomic_t cr_count; /* ref count */
+ refcount_t cr_count; /* ref count */
kuid_t cr_uid;
@@ -208,7 +208,7 @@ char * rpcauth_stringify_acceptor(struct rpc_cred *);
static inline
struct rpc_cred *get_rpccred(struct rpc_cred *cred)
{
- if (cred != NULL && atomic_inc_not_zero(&cred->cr_count))
+ if (cred != NULL && refcount_inc_not_zero(&cred->cr_count))
return cred;
return NULL;
}
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 77748e572686..4903eda5dd61 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -495,7 +495,7 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
if (nr_to_scan-- == 0)
break;
- if (atomic_read(&cred->cr_count) > 1) {
+ if (refcount_read(&cred->cr_count) > 1) {
rpcauth_lru_remove_locked(cred);
continue;
}
@@ -589,7 +589,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
continue;
if (flags & RPCAUTH_LOOKUP_RCU) {
if (test_bit(RPCAUTH_CRED_NEW, &entry->cr_flags) ||
- atomic_read(&entry->cr_count) == 0)
+ refcount_read(&entry->cr_count) == 0)
continue;
cred = entry;
break;
@@ -623,7 +623,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
if (cred == NULL) {
cred = new;
set_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags);
- atomic_inc(&cred->cr_count);
+ refcount_inc(&cred->cr_count);
hlist_add_head_rcu(&cred->cr_hash, &cache->hashtable[nr]);
} else
list_add_tail(&new->cr_lru, &free);
@@ -670,7 +670,7 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
{
INIT_HLIST_NODE(&cred->cr_hash);
INIT_LIST_HEAD(&cred->cr_lru);
- atomic_set(&cred->cr_count, 1);
+ refcount_set(&cred->cr_count, 1);
cred->cr_auth = auth;
cred->cr_ops = ops;
cred->cr_expire = jiffies;
@@ -739,9 +739,9 @@ put_rpccred(struct rpc_cred *cred)
if (cred == NULL)
return;
rcu_read_lock();
- if (atomic_dec_and_test(&cred->cr_count))
+ if (refcount_dec_and_test(&cred->cr_count))
goto destroy;
- if (atomic_read(&cred->cr_count) != 1 ||
+ if (refcount_read(&cred->cr_count) != 1 ||
!test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags))
goto out;
if (test_bit(RPCAUTH_CRED_UPTODATE, &cred->cr_flags) != 0) {
@@ -752,7 +752,7 @@ put_rpccred(struct rpc_cred *cred)
rpcauth_lru_remove(cred);
} else if (rpcauth_unhash_cred(cred)) {
rpcauth_lru_remove(cred);
- if (atomic_dec_and_test(&cred->cr_count))
+ if (refcount_dec_and_test(&cred->cr_count))
goto destroy;
}
out:
diff --git a/net/sunrpc/auth_null.c b/net/sunrpc/auth_null.c
index a7c00b4959f3..ea816d7000a4 100644
--- a/net/sunrpc/auth_null.c
+++ b/net/sunrpc/auth_null.c
@@ -138,6 +138,6 @@ struct rpc_cred null_cred = {
.cr_lru = LIST_HEAD_INIT(null_cred.cr_lru),
.cr_auth = &null_auth,
.cr_ops = &null_credops,
- .cr_count = ATOMIC_INIT(2),
+ .cr_count = REFCOUNT_INIT(2),
.cr_flags = 1UL << RPCAUTH_CRED_UPTODATE,
};
--
2.17.2
Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/sunrpc/auth.h | 2 +-
net/sunrpc/auth.c | 2 +-
net/sunrpc/auth_generic.c | 2 +-
net/sunrpc/auth_gss/auth_gss.c | 4 ++--
net/sunrpc/auth_null.c | 4 ++--
net/sunrpc/auth_unix.c | 4 ++--
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
index a71d4bd191e7..c4db9424b63b 100644
--- a/include/linux/sunrpc/auth.h
+++ b/include/linux/sunrpc/auth.h
@@ -100,7 +100,7 @@ struct rpc_auth {
* differ from the flavor in
* au_ops->au_flavor in gss
* case) */
- atomic_t au_count; /* Reference counter */
+ refcount_t au_count; /* Reference counter */
struct rpc_cred_cache * au_credcache;
/* per-flavor data */
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 4903eda5dd61..ad8ead738981 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(rpcauth_create);
void
rpcauth_release(struct rpc_auth *auth)
{
- if (!atomic_dec_and_test(&auth->au_count))
+ if (!refcount_dec_and_test(&auth->au_count))
return;
auth->au_ops->destroy(auth);
}
diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
index f1df9837f1ac..d8831b988b1e 100644
--- a/net/sunrpc/auth_generic.c
+++ b/net/sunrpc/auth_generic.c
@@ -274,7 +274,7 @@ static const struct rpc_authops generic_auth_ops = {
static struct rpc_auth generic_auth = {
.au_ops = &generic_auth_ops,
- .au_count = ATOMIC_INIT(0),
+ .au_count = REFCOUNT_INIT(1),
};
static bool generic_key_to_expire(struct rpc_cred *cred)
diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index c898a7c75e84..30f970cdc7f6 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1058,7 +1058,7 @@ gss_create_new(const struct rpc_auth_create_args *args, struct rpc_clnt *clnt)
auth->au_flavor = flavor;
if (gss_pseudoflavor_to_datatouch(gss_auth->mech, flavor))
auth->au_flags |= RPCAUTH_AUTH_DATATOUCH;
- atomic_set(&auth->au_count, 1);
+ refcount_set(&auth->au_count, 1);
kref_init(&gss_auth->kref);
err = rpcauth_init_credcache(auth);
@@ -1187,7 +1187,7 @@ gss_auth_find_or_add_hashed(const struct rpc_auth_create_args *args,
if (strcmp(gss_auth->target_name, args->target_name))
continue;
}
- if (!atomic_inc_not_zero(&gss_auth->rpc_auth.au_count))
+ if (!refcount_inc_not_zero(&gss_auth->rpc_auth.au_count))
continue;
goto out;
}
diff --git a/net/sunrpc/auth_null.c b/net/sunrpc/auth_null.c
index ea816d7000a4..2694a1bc026b 100644
--- a/net/sunrpc/auth_null.c
+++ b/net/sunrpc/auth_null.c
@@ -21,7 +21,7 @@ static struct rpc_cred null_cred;
static struct rpc_auth *
nul_create(const struct rpc_auth_create_args *args, struct rpc_clnt *clnt)
{
- atomic_inc(&null_auth.au_count);
+ refcount_inc(&null_auth.au_count);
return &null_auth;
}
@@ -119,7 +119,7 @@ struct rpc_auth null_auth = {
.au_flags = RPCAUTH_AUTH_NO_CRKEY_TIMEOUT,
.au_ops = &authnull_ops,
.au_flavor = RPC_AUTH_NULL,
- .au_count = ATOMIC_INIT(0),
+ .au_count = REFCOUNT_INIT(1),
};
static
diff --git a/net/sunrpc/auth_unix.c b/net/sunrpc/auth_unix.c
index 185e56d4f9ae..4c1c7e56288f 100644
--- a/net/sunrpc/auth_unix.c
+++ b/net/sunrpc/auth_unix.c
@@ -34,7 +34,7 @@ unx_create(const struct rpc_auth_create_args *args, struct rpc_clnt *clnt)
{
dprintk("RPC: creating UNIX authenticator for client %p\n",
clnt);
- atomic_inc(&unix_auth.au_count);
+ refcount_inc(&unix_auth.au_count);
return &unix_auth;
}
@@ -239,7 +239,7 @@ struct rpc_auth unix_auth = {
.au_flags = RPCAUTH_AUTH_NO_CRKEY_TIMEOUT,
.au_ops = &authunix_ops,
.au_flavor = RPC_AUTH_UNIX,
- .au_count = ATOMIC_INIT(0),
+ .au_count = REFCOUNT_INIT(1),
};
static
--
2.17.2
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 <[email protected]>
---
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
On 2022-07-04 00:53:06, Dan Aloni wrote:
> 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.
v2: Made a better explanation in the commit message.
-- >8 --
Subject: [PATCH] sunrpc: fix expiry of auth creds
Before this commit, with a large enough LRU of expired items (100), the
loop skipped all the expired items and was entirely ineffectual in
trimming the LRU list.
Fixes: 95cd623250ad ('SUNRPC: Clean up the AUTH cache code')
Signed-off-by: Dan Aloni <[email protected]>
---
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