This patchset adds some stats tracking to the DRC to hopefully allow
us to make better-informed decisions in the future on performance
improvements.
Some of these I've already posted, but reposting them now as a set
to allow Bruce to more easily merge them. I think these would be good
for 3.9, but they're obviously not critical.
Jeff Layton (5):
nfsd: break out comparator into separate function
nfsd: track memory utilization in the DRC
nfsd: add new reply_cache_stats file in nfsdfs
nfsd: report length of the longest hash chain in reply cache stats
nfsd: keep track of the max and average time to search the cache
fs/nfsd/cache.h | 1 +
fs/nfsd/nfscache.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++-------
fs/nfsd/nfsctl.c | 9 ++++
3 files changed, 117 insertions(+), 16 deletions(-)
--
1.7.11.7
Break out the function that compares the rqstp and checksum against a
reply cache entry. While we're at it, track the efficacy of the checksum
over the NFS data by tracking the cases where we would have incorrectly
matched a DRC entry if we had not tracked it or the length.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfscache.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index ca43664..8986568 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -25,6 +25,7 @@ static struct list_head lru_head;
static struct kmem_cache *drc_slab;
static unsigned int num_drc_entries;
static unsigned int max_drc_entries;
+static unsigned int csum_misses;
/*
* Calculate the hash index from an XID.
@@ -272,6 +273,26 @@ nfsd_cache_csum(struct svc_rqst *rqstp)
return csum;
}
+static bool
+nfsd_cache_match(struct svc_rqst *rqstp, __wsum csum, struct svc_cacherep *rp)
+{
+ /* Check RPC header info first */
+ if (rqstp->rq_xid != rp->c_xid || rqstp->rq_proc != rp->c_proc ||
+ rqstp->rq_prot != rp->c_prot || rqstp->rq_vers != rp->c_vers ||
+ rqstp->rq_arg.len != rp->c_len ||
+ !rpc_cmp_addr(svc_addr(rqstp), (struct sockaddr *)&rp->c_addr) ||
+ rpc_get_port(svc_addr(rqstp)) != rpc_get_port((struct sockaddr *)&rp->c_addr))
+ return false;
+
+ /* compare checksum of NFS data */
+ if (csum != rp->c_csum) {
+ ++csum_misses;
+ return false;
+ }
+
+ return true;
+}
+
/*
* Search the request hash for an entry that matches the given rqstp.
* Must be called with cache_lock held. Returns the found entry or
@@ -283,18 +304,10 @@ nfsd_cache_search(struct svc_rqst *rqstp, __wsum csum)
struct svc_cacherep *rp;
struct hlist_node *hn;
struct hlist_head *rh;
- __be32 xid = rqstp->rq_xid;
- u32 proto = rqstp->rq_prot,
- vers = rqstp->rq_vers,
- proc = rqstp->rq_proc;
- rh = &cache_hash[request_hash(xid)];
+ rh = &cache_hash[request_hash(rqstp->rq_xid)];
hlist_for_each_entry(rp, hn, rh, c_hash) {
- if (xid == rp->c_xid && proc == rp->c_proc &&
- proto == rp->c_prot && vers == rp->c_vers &&
- rqstp->rq_arg.len == rp->c_len && csum == rp->c_csum &&
- rpc_cmp_addr(svc_addr(rqstp), (struct sockaddr *)&rp->c_addr) &&
- rpc_get_port(svc_addr(rqstp)) == rpc_get_port((struct sockaddr *)&rp->c_addr))
+ if (nfsd_cache_match(rqstp, csum, rp))
return rp;
}
return NULL;
--
1.7.11.7
For presenting statistics relating to duplicate reply cache.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/cache.h | 1 +
fs/nfsd/nfscache.c | 37 +++++++++++++++++++++++++++++++++----
fs/nfsd/nfsctl.c | 9 +++++++++
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/cache.h b/fs/nfsd/cache.h
index 87fd141..d5c5b3e 100644
--- a/fs/nfsd/cache.h
+++ b/fs/nfsd/cache.h
@@ -82,6 +82,7 @@ int nfsd_reply_cache_init(void);
void nfsd_reply_cache_shutdown(void);
int nfsd_cache_lookup(struct svc_rqst *);
void nfsd_cache_update(struct svc_rqst *, int, __be32 *);
+int nfsd_reply_cache_stats_open(struct inode *, struct file *);
#ifdef CONFIG_NFSD_V4
void nfsd4_set_statp(struct svc_rqst *rqstp, __be32 *statp);
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index efed50e..a5ac9ab 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -23,12 +23,17 @@
static struct hlist_head * cache_hash;
static struct list_head lru_head;
static struct kmem_cache *drc_slab;
-static unsigned int num_drc_entries;
-static unsigned int max_drc_entries;
-static unsigned int drc_mem_usage;
-static unsigned int csum_misses;
/*
+ * Stats on the duplicate reply cache code. All of these and the "rc" fields
+ * in nfsdstats are protected by the cache_lock
+ */
+static unsigned int num_drc_entries; /* number of entries */
+static unsigned int max_drc_entries; /* max number of entries */
+static unsigned int drc_mem_usage; /* memory used by cache */
+static unsigned int csum_misses; /* cache misses due only to
+ csum comparison failures */
+/*
* Calculate the hash index from an XID.
*/
static inline u32 request_hash(u32 xid)
@@ -545,3 +550,27 @@ nfsd_cache_append(struct svc_rqst *rqstp, struct kvec *data)
vec->iov_len += data->iov_len;
return 1;
}
+
+/*
+ * Note that fields may be added, removed or reordered in the future. Programs
+ * scraping this file for info should test the labels to ensure they're
+ * getting the correct field.
+ */
+static int nfsd_reply_cache_stats_show(struct seq_file *m, void *v)
+{
+ spin_lock(&cache_lock);
+ seq_printf(m, "max entries: %u\n", max_drc_entries);
+ seq_printf(m, "num entries: %u\n", num_drc_entries);
+ seq_printf(m, "mem usage: %u\n", drc_mem_usage);
+ seq_printf(m, "cache hits: %u\n", nfsdstats.rchits);
+ seq_printf(m, "cache misses: %u\n", nfsdstats.rcmisses);
+ seq_printf(m, "not cached: %u\n", nfsdstats.rcnocache);
+ seq_printf(m, "checksum misses: %u\n", csum_misses);
+ spin_unlock(&cache_lock);
+ return 0;
+}
+
+int nfsd_reply_cache_stats_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, nfsd_reply_cache_stats_show, NULL);
+}
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 8ead2c2..c7238a5 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -35,6 +35,7 @@ enum {
NFSD_Threads,
NFSD_Pool_Threads,
NFSD_Pool_Stats,
+ NFSD_Reply_Cache_Stats,
NFSD_Versions,
NFSD_Ports,
NFSD_MaxBlkSize,
@@ -212,6 +213,13 @@ static const struct file_operations pool_stats_operations = {
.owner = THIS_MODULE,
};
+static struct file_operations reply_cache_stats_operations = {
+ .open = nfsd_reply_cache_stats_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
/*----------------------------------------------------------------------------*/
/*
* payload - write methods
@@ -1047,6 +1055,7 @@ static int nfsd_fill_super(struct super_block * sb, void * data, int silent)
[NFSD_Threads] = {"threads", &transaction_ops, S_IWUSR|S_IRUSR},
[NFSD_Pool_Threads] = {"pool_threads", &transaction_ops, S_IWUSR|S_IRUSR},
[NFSD_Pool_Stats] = {"pool_stats", &pool_stats_operations, S_IRUGO},
+ [NFSD_Reply_Cache_Stats] = {"reply_cache_stats", &reply_cache_stats_operations, S_IRUGO},
[NFSD_Versions] = {"versions", &transaction_ops, S_IWUSR|S_IRUSR},
[NFSD_Ports] = {"portlist", &transaction_ops, S_IWUSR|S_IRUGO},
[NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops, S_IWUSR|S_IRUGO},
--
1.7.11.7
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfscache.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 8986568..efed50e 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -25,6 +25,7 @@ static struct list_head lru_head;
static struct kmem_cache *drc_slab;
static unsigned int num_drc_entries;
static unsigned int max_drc_entries;
+static unsigned int drc_mem_usage;
static unsigned int csum_misses;
/*
@@ -101,11 +102,14 @@ nfsd_reply_cache_alloc(void)
static void
nfsd_reply_cache_free_locked(struct svc_cacherep *rp)
{
- if (rp->c_type == RC_REPLBUFF)
+ if (rp->c_type == RC_REPLBUFF && rp->c_replvec.iov_base) {
+ drc_mem_usage -= ksize(rp->c_replvec.iov_base);
kfree(rp->c_replvec.iov_base);
+ }
hlist_del(&rp->c_hash);
list_del(&rp->c_lru);
--num_drc_entries;
+ drc_mem_usage -= ksize(rp);
kmem_cache_free(drc_slab, rp);
}
@@ -367,6 +371,7 @@ nfsd_cache_lookup(struct svc_rqst *rqstp)
return RC_DOIT;
}
spin_lock(&cache_lock);
+ drc_mem_usage += ksize(rp);
++num_drc_entries;
/*
@@ -407,6 +412,7 @@ setup_entry:
/* release any buffer */
if (rp->c_type == RC_REPLBUFF) {
+ drc_mem_usage -= ksize(rp->c_replvec.iov_base);
kfree(rp->c_replvec.iov_base);
rp->c_replvec.iov_base = NULL;
}
@@ -475,6 +481,7 @@ nfsd_cache_update(struct svc_rqst *rqstp, int cachetype, __be32 *statp)
struct svc_cacherep *rp = rqstp->rq_cacherep;
struct kvec *resv = &rqstp->rq_res.head[0], *cachv;
int len;
+ size_t replbufsize = 0;
if (!rp)
return;
@@ -501,6 +508,7 @@ nfsd_cache_update(struct svc_rqst *rqstp, int cachetype, __be32 *statp)
nfsd_reply_cache_free(rp);
return;
}
+ replbufsize = ksize(cachv->iov_base);
cachv->iov_len = len << 2;
memcpy(cachv->iov_base, statp, len << 2);
break;
@@ -509,6 +517,7 @@ nfsd_cache_update(struct svc_rqst *rqstp, int cachetype, __be32 *statp)
return;
}
spin_lock(&cache_lock);
+ drc_mem_usage += replbufsize;
lru_put_end(rp);
rp->c_secure = rqstp->rq_secure;
rp->c_type = cachetype;
--
1.7.11.7
So we can get a feel for how effective the hashing function is.
As Chuck Lever pointed out to me, it's generally acceptable to do
"expensive" stuff when reading the stats since that's a relatively
rare activity. When we go to read the file, walk all of the hash
chains and count the number of entries in each. Report the length
of the longest one.
Cc: Chuck Lever <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfscache.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index a5ac9ab..52493cb 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -552,6 +552,28 @@ nfsd_cache_append(struct svc_rqst *rqstp, struct kvec *data)
}
/*
+ * Walk each hash chain and count the number of entries. Return the length of
+ * the longest one. Must be called with the cache_lock held.
+ */
+static unsigned int
+nfsd_repcache_max_chain_len(void)
+{
+ int i;
+ struct hlist_node *pos;
+ unsigned int max = 0;
+
+ for (i = 0; i < HASHSIZE; ++i) {
+ unsigned int cur = 0;
+
+ hlist_for_each(pos, &cache_hash[i])
+ ++cur;
+ max = max(cur, max);
+ }
+
+ return max;
+}
+
+/*
* Note that fields may be added, removed or reordered in the future. Programs
* scraping this file for info should test the labels to ensure they're
* getting the correct field.
@@ -566,6 +588,7 @@ static int nfsd_reply_cache_stats_show(struct seq_file *m, void *v)
seq_printf(m, "cache misses: %u\n", nfsdstats.rcmisses);
seq_printf(m, "not cached: %u\n", nfsdstats.rcnocache);
seq_printf(m, "checksum misses: %u\n", csum_misses);
+ seq_printf(m, "max chain len: %u\n", nfsd_repcache_max_chain_len());
spin_unlock(&cache_lock);
return 0;
}
--
1.7.11.7
Since we're moderately concerned about the potential increase in hash
chain lengths, keep a running average of the time and the max time to
search.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfscache.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
index 52493cb..ae6575d 100644
--- a/fs/nfsd/nfscache.c
+++ b/fs/nfsd/nfscache.c
@@ -33,6 +33,9 @@ static unsigned int max_drc_entries; /* max number of entries */
static unsigned int drc_mem_usage; /* memory used by cache */
static unsigned int csum_misses; /* cache misses due only to
csum comparison failures */
+static unsigned int max_search_time; /* max time to search */
+static unsigned int avg_search_time; /* avg time to search */
+
/*
* Calculate the hash index from an XID.
*/
@@ -310,16 +313,28 @@ nfsd_cache_match(struct svc_rqst *rqstp, __wsum csum, struct svc_cacherep *rp)
static struct svc_cacherep *
nfsd_cache_search(struct svc_rqst *rqstp, __wsum csum)
{
- struct svc_cacherep *rp;
+ struct svc_cacherep *rp, *ret = NULL;
struct hlist_node *hn;
struct hlist_head *rh;
+ ktime_t start;
+ unsigned int delta;
+ static unsigned int nsearches = 0;
+ unsigned int osearches;
+ start = ktime_get();
rh = &cache_hash[request_hash(rqstp->rq_xid)];
hlist_for_each_entry(rp, hn, rh, c_hash) {
- if (nfsd_cache_match(rqstp, csum, rp))
- return rp;
+ if (nfsd_cache_match(rqstp, csum, rp)) {
+ ret = rp;
+ break;
+ }
}
- return NULL;
+ delta = (unsigned int)ktime_to_ns(ktime_sub(ktime_get(), start));
+ osearches = nsearches++;
+ avg_search_time = (avg_search_time * osearches + delta) / nsearches;
+ max_search_time = max(max_search_time, delta);
+
+ return ret;
}
/*
@@ -589,6 +604,8 @@ static int nfsd_reply_cache_stats_show(struct seq_file *m, void *v)
seq_printf(m, "not cached: %u\n", nfsdstats.rcnocache);
seq_printf(m, "checksum misses: %u\n", csum_misses);
seq_printf(m, "max chain len: %u\n", nfsd_repcache_max_chain_len());
+ seq_printf(m, "avg search time: %uns\n", avg_search_time);
+ seq_printf(m, "max search time: %uns\n", max_search_time);
spin_unlock(&cache_lock);
return 0;
}
--
1.7.11.7
On Mon, Mar 04, 2013 at 01:56:35PM -0500, Jeff Layton wrote:
> This patchset adds some stats tracking to the DRC to hopefully allow
> us to make better-informed decisions in the future on performance
> improvements.
>
> Some of these I've already posted, but reposting them now as a set
> to allow Bruce to more easily merge them. I think these would be good
> for 3.9, but they're obviously not critical.
OK, thanks. I'll queue them up for 3.10.
--b.
>
> Jeff Layton (5):
> nfsd: break out comparator into separate function
> nfsd: track memory utilization in the DRC
> nfsd: add new reply_cache_stats file in nfsdfs
> nfsd: keep track of the max and average time to search the cache
> nfsd: keep stats on worst hash balancing seen so far
>
> fs/nfsd/cache.h | 1 +
> fs/nfsd/nfscache.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++-------
> fs/nfsd/nfsctl.c | 9 ++++
> 3 files changed, 137 insertions(+), 17 deletions(-)
>
> --
> 1.7.11.7
>