2014-07-21 19:11:40

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 1/2] SUNRPC: Enforce an upper limit on the number of cached credentials

In some cases where the credentials are not often reused, we may want
to limit their total number just in order to make the negative lookups
in the hash table more manageable.

Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/auth.c | 44 +++++++++++++++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 2bc7bb82b162..360decdddc78 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -80,6 +80,10 @@ static struct kernel_param_ops param_ops_hashtbl_sz = {
module_param_named(auth_hashtable_size, auth_hashbits, hashtbl_sz, 0644);
MODULE_PARM_DESC(auth_hashtable_size, "RPC credential cache hashtable size");

+static unsigned long auth_max_cred_cachesize = ULONG_MAX;
+module_param(auth_max_cred_cachesize, ulong, 0644);
+MODULE_PARM_DESC(auth_max_cred_cachesize, "RPC credential maximum total cache size");
+
static u32
pseudoflavor_to_flavor(u32 flavor) {
if (flavor > RPC_AUTH_MAXFLAVOR)
@@ -481,6 +485,20 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
return freed;
}

+static unsigned long
+rpcauth_cache_do_shrink(int nr_to_scan)
+{
+ LIST_HEAD(free);
+ unsigned long freed;
+
+ spin_lock(&rpc_credcache_lock);
+ freed = rpcauth_prune_expired(&free, nr_to_scan);
+ spin_unlock(&rpc_credcache_lock);
+ rpcauth_destroy_credlist(&free);
+
+ return freed;
+}
+
/*
* Run memory cache shrinker.
*/
@@ -488,9 +506,6 @@ static unsigned long
rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)

{
- LIST_HEAD(free);
- unsigned long freed;
-
if ((sc->gfp_mask & GFP_KERNEL) != GFP_KERNEL)
return SHRINK_STOP;

@@ -498,12 +513,7 @@ rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
if (list_empty(&cred_unused))
return SHRINK_STOP;

- spin_lock(&rpc_credcache_lock);
- freed = rpcauth_prune_expired(&free, sc->nr_to_scan);
- spin_unlock(&rpc_credcache_lock);
- rpcauth_destroy_credlist(&free);
-
- return freed;
+ return rpcauth_cache_do_shrink(sc->nr_to_scan);
}

static unsigned long
@@ -513,6 +523,21 @@ rpcauth_cache_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
return (number_cred_unused / 100) * sysctl_vfs_cache_pressure;
}

+static void
+rpcauth_cache_enforce_limit(void)
+{
+ unsigned long diff;
+ unsigned int nr_to_scan;
+
+ if (number_cred_unused <= auth_max_cred_cachesize)
+ return;
+ diff = number_cred_unused - auth_max_cred_cachesize;
+ nr_to_scan = 100;
+ if (diff < nr_to_scan)
+ nr_to_scan = diff;
+ rpcauth_cache_do_shrink(nr_to_scan);
+}
+
/*
* Look up a process' credentials in the authentication cache
*/
@@ -566,6 +591,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
} else
list_add_tail(&new->cr_lru, &free);
spin_unlock(&cache->lock);
+ rpcauth_cache_enforce_limit();
found:
if (test_bit(RPCAUTH_CRED_NEW, &cred->cr_flags) &&
cred->cr_ops->cr_init != NULL &&
--
1.9.3



2014-07-21 21:09:05

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/2] SUNRPC: Enforce an upper limit on the number of cached credentials

On Mon, Jul 21, 2014 at 03:11:41PM -0400, Trond Myklebust wrote:
> In some cases where the credentials are not often reused, we may want
> to limit their total number just in order to make the negative lookups
> in the hash table more manageable.

Out of curiosity--how would somebody know that might need to set this?

--b.

>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> net/sunrpc/auth.c | 44 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index 2bc7bb82b162..360decdddc78 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -80,6 +80,10 @@ static struct kernel_param_ops param_ops_hashtbl_sz = {
> module_param_named(auth_hashtable_size, auth_hashbits, hashtbl_sz, 0644);
> MODULE_PARM_DESC(auth_hashtable_size, "RPC credential cache hashtable size");
>
> +static unsigned long auth_max_cred_cachesize = ULONG_MAX;
> +module_param(auth_max_cred_cachesize, ulong, 0644);
> +MODULE_PARM_DESC(auth_max_cred_cachesize, "RPC credential maximum total cache size");
> +
> static u32
> pseudoflavor_to_flavor(u32 flavor) {
> if (flavor > RPC_AUTH_MAXFLAVOR)
> @@ -481,6 +485,20 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
> return freed;
> }
>
> +static unsigned long
> +rpcauth_cache_do_shrink(int nr_to_scan)
> +{
> + LIST_HEAD(free);
> + unsigned long freed;
> +
> + spin_lock(&rpc_credcache_lock);
> + freed = rpcauth_prune_expired(&free, nr_to_scan);
> + spin_unlock(&rpc_credcache_lock);
> + rpcauth_destroy_credlist(&free);
> +
> + return freed;
> +}
> +
> /*
> * Run memory cache shrinker.
> */
> @@ -488,9 +506,6 @@ static unsigned long
> rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>
> {
> - LIST_HEAD(free);
> - unsigned long freed;
> -
> if ((sc->gfp_mask & GFP_KERNEL) != GFP_KERNEL)
> return SHRINK_STOP;
>
> @@ -498,12 +513,7 @@ rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
> if (list_empty(&cred_unused))
> return SHRINK_STOP;
>
> - spin_lock(&rpc_credcache_lock);
> - freed = rpcauth_prune_expired(&free, sc->nr_to_scan);
> - spin_unlock(&rpc_credcache_lock);
> - rpcauth_destroy_credlist(&free);
> -
> - return freed;
> + return rpcauth_cache_do_shrink(sc->nr_to_scan);
> }
>
> static unsigned long
> @@ -513,6 +523,21 @@ rpcauth_cache_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
> return (number_cred_unused / 100) * sysctl_vfs_cache_pressure;
> }
>
> +static void
> +rpcauth_cache_enforce_limit(void)
> +{
> + unsigned long diff;
> + unsigned int nr_to_scan;
> +
> + if (number_cred_unused <= auth_max_cred_cachesize)
> + return;
> + diff = number_cred_unused - auth_max_cred_cachesize;
> + nr_to_scan = 100;
> + if (diff < nr_to_scan)
> + nr_to_scan = diff;
> + rpcauth_cache_do_shrink(nr_to_scan);
> +}
> +
> /*
> * Look up a process' credentials in the authentication cache
> */
> @@ -566,6 +591,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
> } else
> list_add_tail(&new->cr_lru, &free);
> spin_unlock(&cache->lock);
> + rpcauth_cache_enforce_limit();
> found:
> if (test_bit(RPCAUTH_CRED_NEW, &cred->cr_flags) &&
> cred->cr_ops->cr_init != NULL &&
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-07-21 22:33:11

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] SUNRPC: Enforce an upper limit on the number of cached credentials

On Mon, Jul 21, 2014 at 5:09 PM, J. Bruce Fields <[email protected]> wrote:
> On Mon, Jul 21, 2014 at 03:11:41PM -0400, Trond Myklebust wrote:
>> In some cases where the credentials are not often reused, we may want
>> to limit their total number just in order to make the negative lookups
>> in the hash table more manageable.
>
> Out of curiosity--how would somebody know that might need to set this?

It would have to be for very unusual workloads (which is why we
haven't done it before). Something like a web-server that creates
uids/gids on the fly, uses them for a few transactions and then
doesn't need to use them again for a long while. You would need to use
something like 'perf' or a similar profiler to show you are spending a
lot of time in the rpc_auth lookup code.
Note that we do have the ability already to change the hash table size
to mitigate the above problem, but if the workload really is using
throw-away uid/gids, then this is going to work better.

Cheers
Trond

> --b.
>
>>
>> Signed-off-by: Trond Myklebust <[email protected]>
>> ---
>> net/sunrpc/auth.c | 44 +++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 35 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index 2bc7bb82b162..360decdddc78 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -80,6 +80,10 @@ static struct kernel_param_ops param_ops_hashtbl_sz = {
>> module_param_named(auth_hashtable_size, auth_hashbits, hashtbl_sz, 0644);
>> MODULE_PARM_DESC(auth_hashtable_size, "RPC credential cache hashtable size");
>>
>> +static unsigned long auth_max_cred_cachesize = ULONG_MAX;
>> +module_param(auth_max_cred_cachesize, ulong, 0644);
>> +MODULE_PARM_DESC(auth_max_cred_cachesize, "RPC credential maximum total cache size");
>> +
>> static u32
>> pseudoflavor_to_flavor(u32 flavor) {
>> if (flavor > RPC_AUTH_MAXFLAVOR)
>> @@ -481,6 +485,20 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
>> return freed;
>> }
>>
>> +static unsigned long
>> +rpcauth_cache_do_shrink(int nr_to_scan)
>> +{
>> + LIST_HEAD(free);
>> + unsigned long freed;
>> +
>> + spin_lock(&rpc_credcache_lock);
>> + freed = rpcauth_prune_expired(&free, nr_to_scan);
>> + spin_unlock(&rpc_credcache_lock);
>> + rpcauth_destroy_credlist(&free);
>> +
>> + return freed;
>> +}
>> +
>> /*
>> * Run memory cache shrinker.
>> */
>> @@ -488,9 +506,6 @@ static unsigned long
>> rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>>
>> {
>> - LIST_HEAD(free);
>> - unsigned long freed;
>> -
>> if ((sc->gfp_mask & GFP_KERNEL) != GFP_KERNEL)
>> return SHRINK_STOP;
>>
>> @@ -498,12 +513,7 @@ rpcauth_cache_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>> if (list_empty(&cred_unused))
>> return SHRINK_STOP;
>>
>> - spin_lock(&rpc_credcache_lock);
>> - freed = rpcauth_prune_expired(&free, sc->nr_to_scan);
>> - spin_unlock(&rpc_credcache_lock);
>> - rpcauth_destroy_credlist(&free);
>> -
>> - return freed;
>> + return rpcauth_cache_do_shrink(sc->nr_to_scan);
>> }
>>
>> static unsigned long
>> @@ -513,6 +523,21 @@ rpcauth_cache_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>> return (number_cred_unused / 100) * sysctl_vfs_cache_pressure;
>> }
>>
>> +static void
>> +rpcauth_cache_enforce_limit(void)
>> +{
>> + unsigned long diff;
>> + unsigned int nr_to_scan;
>> +
>> + if (number_cred_unused <= auth_max_cred_cachesize)
>> + return;
>> + diff = number_cred_unused - auth_max_cred_cachesize;
>> + nr_to_scan = 100;
>> + if (diff < nr_to_scan)
>> + nr_to_scan = diff;
>> + rpcauth_cache_do_shrink(nr_to_scan);
>> +}
>> +
>> /*
>> * Look up a process' credentials in the authentication cache
>> */
>> @@ -566,6 +591,7 @@ rpcauth_lookup_credcache(struct rpc_auth *auth, struct auth_cred * acred,
>> } else
>> list_add_tail(&new->cr_lru, &free);
>> spin_unlock(&cache->lock);
>> + rpcauth_cache_enforce_limit();
>> found:
>> if (test_bit(RPCAUTH_CRED_NEW, &cred->cr_flags) &&
>> cred->cr_ops->cr_init != NULL &&
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

[email protected]

2014-07-21 19:11:41

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 2/2] NFS: Enforce an upper limit on the number of cached access call

This may be used to limit the number of cached credentials building up
inside the access cache.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 40 ++++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 4a3d4ef76127..285392e2c946 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2028,6 +2028,10 @@ static DEFINE_SPINLOCK(nfs_access_lru_lock);
static LIST_HEAD(nfs_access_lru_list);
static atomic_long_t nfs_access_nr_entries;

+static unsigned long nfs_access_max_cachesize = ULONG_MAX;
+module_param(nfs_access_max_cachesize, ulong, 0644);
+MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS access maximum total cache length");
+
static void nfs_access_free_entry(struct nfs_access_entry *entry)
{
put_rpccred(entry->cred);
@@ -2049,18 +2053,13 @@ static void nfs_access_free_list(struct list_head *head)
}

unsigned long
-nfs_access_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
+nfs_do_access_cache_scan(unsigned int nr_to_scan)
{
LIST_HEAD(head);
struct nfs_inode *nfsi, *next;
struct nfs_access_entry *cache;
- int nr_to_scan = sc->nr_to_scan;
- gfp_t gfp_mask = sc->gfp_mask;
long freed = 0;

- if ((gfp_mask & GFP_KERNEL) != GFP_KERNEL)
- return SHRINK_STOP;
-
spin_lock(&nfs_access_lru_lock);
list_for_each_entry_safe(nfsi, next, &nfs_access_lru_list, access_cache_inode_lru) {
struct inode *inode;
@@ -2094,11 +2093,39 @@ remove_lru_entry:
}

unsigned long
+nfs_access_cache_scan(struct shrinker *shrink, struct shrink_control *sc)
+{
+ int nr_to_scan = sc->nr_to_scan;
+ gfp_t gfp_mask = sc->gfp_mask;
+
+ if ((gfp_mask & GFP_KERNEL) != GFP_KERNEL)
+ return SHRINK_STOP;
+ return nfs_do_access_cache_scan(nr_to_scan);
+}
+
+
+unsigned long
nfs_access_cache_count(struct shrinker *shrink, struct shrink_control *sc)
{
return vfs_pressure_ratio(atomic_long_read(&nfs_access_nr_entries));
}

+static void
+nfs_access_cache_enforce_limit(void)
+{
+ long nr_entries = atomic_long_read(&nfs_access_nr_entries);
+ unsigned long diff;
+ unsigned int nr_to_scan;
+
+ if (nr_entries < 0 || nr_entries <= nfs_access_max_cachesize)
+ return;
+ nr_to_scan = 100;
+ diff = nr_entries - nfs_access_max_cachesize;
+ if (diff < nr_to_scan)
+ nr_to_scan = diff;
+ nfs_do_access_cache_scan(nr_to_scan);
+}
+
static void __nfs_access_zap_cache(struct nfs_inode *nfsi, struct list_head *head)
{
struct rb_root *root_node = &nfsi->access_cache;
@@ -2244,6 +2271,7 @@ void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set)
&nfs_access_lru_list);
spin_unlock(&nfs_access_lru_lock);
}
+ nfs_access_cache_enforce_limit();
}
EXPORT_SYMBOL_GPL(nfs_access_add_cache);

--
1.9.3