On 05/14/10 01:13 PM, Trond Myklebust wrote:
On Fri, 2010-05-14 at 12:34 -0400, Chuck Lever wrote:
On 05/13/10 05:08 PM, Trond Myklebust wrote:
Signed-off-by: Trond Myklebust<Trond.Myklebust@xxxxxxxxxx>
---
net/sunrpc/auth.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 2213dc5..5fb02ac 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -236,6 +236,8 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
list_for_each_entry_safe(cred, next,&cred_unused, cr_lru) {
+ if (nr_to_scan-- == 0)
+ break;
/*
* Enforce a 60 second garbage collection moratorium
* Note that the cred_unused list must be time-ordered.
@@ -255,11 +257,8 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
get_rpccred(cred);
list_add_tail(&cred->cr_lru, free);
rpcauth_unhash_cred_locked(cred);
- nr_to_scan--;
}
spin_unlock(cache_lock);
- if (nr_to_scan == 0)
- break;
}
return (number_cred_unused / 100) * sysctl_vfs_cache_pressure;
}
It looks to me like the mm calls our cache shrinker with nr_to_scan set
to zero when it just wants this return value, and nothing more. But the
logic here seems to assume that nr_to_scan == 0 means shrink as much as
you can. Am I reading this correctly?
Look more carefully: the comparison contains a post-decrement operation,
so if the nr_to_scan == 0, then we immediately exit the loop (after
decrementing nr_to_scan, but who cares about that)...
When mm calls with nr_to_scan set to zero, it doesn't expect a -1
return, it just uses the returned value. mm checks for a -1 return only
when a non-zero scan count argument is passed.
So the check you added here (and in the access cache shrinker) to return
-1 when the gfp_mask doesn't contain GFP_KERNEL could cause some
trouble. It would be safer if we return -1 _after_ checking for
nr_to_scan == 0.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
