On Thu, 14 Feb 2013 17:08:02 -0500
"J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> On Thu, Feb 14, 2013 at 04:45:14PM -0500, Jeff Layton wrote:
> > Break 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.
>
> Why the checksum and length, as opposed to just the checksum?
>
> Seems to me that comparing the length is no more expensive than any of
> the other comparisons (xid, proc, etc.) that we're already doing, so
> we're less likely to wonder whether it's worth it.
>
> --b.
>
I guess I was thinking that the length was sort of a property of the
NFS hunk of the packet, so it belonged with the checksum. OTOH, you
have a good point...
It's fairly trivial to change, so I'll plan to respin this once others
have had a chance to comment.
Thanks...
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/nfsd/nfscache.c | 37 +++++++++++++++++++++++++++----------
> > 1 file changed, 27 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
> > index 2f9c2d2..bccf74f 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,30 @@ nfsd_cache_csum(struct svc_rqst *rqstp)
> > return csum;
> > }
> >
> > +static bool
> > +nfsd_cache_match(struct svc_rqst *rqstp, __wsum csum, struct svc_cacherep *rp)
> > +{
> > + __be32 xid = rqstp->rq_xid;
> > + u32 proto = rqstp->rq_prot,
> > + vers = rqstp->rq_vers,
> > + proc = rqstp->rq_proc;
> > +
> > + /* Check RPC header info first */
> > + if (xid != rp->c_xid || proc != rp->c_proc ||
> > + proto != rp->c_prot || vers != rp->c_vers ||
> > + !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;
> > +
> > + /* check contents of the NFS data */
> > + if (rqstp->rq_arg.len != rp->c_len || 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 +308,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
> >
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
