Re: [PATCH 2/3] nfsd: break out comparator into separate function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 
> 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
> 
--
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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux