On Thu, Jun 26, 2014 at 03:12:02PM -0400, Jeff Layton wrote:
> From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>
> We want to use the nfsd4_compound_state to cache the nfs4_client
> in order to optimise away extra lookups of the clid.
Should mention that this is only for 4.0. Actually, kooking at
"nfsd: Allow struct nfsd4_compound_state to cache the nfs4_client" that
patch seems to mostly be a few random code cleanups that should have
a different title so it might make more sense to move the caching
of the client for 4.1+ to this patch as well.
And please write proper, detailed changelogs :)
> +static __be32 lookup_clientid(clientid_t *clid,
> + struct nfsd4_compound_state *cstate,
> + struct nfsd_net *nn)
Now that we don't look up the client idea for the normal case but just
verify it we should probably rename this function to
nfsd4_verify_clientid or something similar.
While it doesn't belong into this patch: why don't we cache the
nfsd_net in the nfsd4_compound_state
> {
> struct nfs4_client *found;
>
> + if (cstate->clp != NULL) {
> + found = cstate->clp;
> + if (!same_clid(&found->cl_clientid, clid))
> + return nfserr_stale_clientid;
> + } else {
> + if (STALE_CLIENTID(clid, nn))
> + return nfserr_stale_clientid;
> + /*
> + * Usually for v4.1+ we get the client in the SEQUENCE op, so
Usually implices there might be a case where we don't get it. Which
case would that be?
> + * if we don't have one cached already then we know this is for
> + * is for v4.0 and "sessions" will be false.
> + */
> + found = find_confirmed_client(clid, false, nn);
> + /* Cache the nfs4_client in cstate! */
> + if (found) {
> + cstate->clp = found;
> + atomic_inc(&found->cl_refcount);
> + }
> + }
> return found ? nfs_ok : nfserr_expired;
The whole thing could be simplified a little more:
if (cstate->clp) {
if (!same_clid(&cstate->clp->cl_clientid, clid))
return nfserr_stale_clientid;
retur nfs_ok;
}
if (STALE_CLIENTID(clid, nn))
return nfserr_stale_clientid;
found = find_confirmed_client(clid, false, nn);
if (!found)
return nfserr_expired;
cstate->clp = found;
atomic_inc(&found->cl_refcount);
return nfs_ok;
> + status = lookup_clientid(&stateid->si_opaque.so_clid, cstate, nn);
> if (status == nfserr_stale_clientid) {
> + if (cstate->session)
> return nfserr_bad_stateid;
> return nfserr_stale_stateid;
> }
Why do we return a different error here for the 4.1+ case? And why not
in other places using lookup_clientid?
--
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
