On Fri, May 15, 2009 at 7:05 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> On Tue, Apr 28, 2009 at 12:59:39PM -0400, andros@xxxxxxxxxx wrote:
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>> CREATE_SESSION can be preceeded by a SEQUENCE operation and the
>> create session single slot cache must be maintained. Encode the results of
>> a create session call into the cache at the end of processing.
>>
>> The create session result will also be encoded into the RPC response, and if
>> it is preceeded by a SEQUENCE operation, will also be encoded into the
>> session slot table cache.
>>
>> Errors that do not change the create session cache:
>> A create session NFS4ERR_STALE_CLIENTID error means that a client record
>> (and associated create session slot) could not be found and therefore can't
>> be changed. NFSERR_SEQ_MISORDERED errors do not change the slot cache.
>>
>> All other errors get cached.
>>
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> ---
>> fs/nfsd/nfs4state.c | 6 ++++--
>> fs/nfsd/nfs4xdr.c | 18 ++++++++++++++++++
>> include/linux/nfsd/xdr4.h | 2 ++
>> 3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index fee6bf0..142c460 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -1376,7 +1376,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>> if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
>> (ip_addr != unconf->cl_addr)) {
>> status = nfserr_clid_inuse;
>> - goto out;
>> + goto out_cache;
>> }
>>
>> slot = &unconf->cl_slot;
>> @@ -1418,7 +1418,9 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>> memcpy(cr_ses->sessionid.data, conf->cl_sessionid.data,
>> NFS4_MAX_SESSIONID_LEN);
>> cr_ses->seqid = slot->sl_seqid;
>> -
>> +out_cache:
>> + /* cache solo and embedded create sessions under the state lock */
>> + nfsd4_cache_create_session(cr_ses, slot, status);
>> out:
>> nfs4_unlock_state();
>> dprintk("%s returns %d\n", __func__, ntohl(status));
>> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
>> index b2f8d74..1dfec1d 100644
>> --- a/fs/nfsd/nfs4xdr.c
>> +++ b/fs/nfsd/nfs4xdr.c
>> @@ -3046,6 +3046,24 @@ nfsd4_encode_create_session(struct nfsd4_compoundres *resp, int nfserr,
>> return 0;
>> }
>>
>> +/*
>> + * Encode the create_session result into the create session single DRC
>> + * slot cache. Do this for solo or embedded create session operations.
>> + */
>> +void
>> +nfsd4_cache_create_session(struct nfsd4_create_session *cr_ses,
>> + struct nfsd4_clid_slot *slot, int nfserr)
>> +{
>> + __be32 *p = (__be32 *)slot->sl_data;
>> + struct nfsd4_compoundres tmp = {
>> + .p = p,
>> + .end = p + XDR_QUADLEN(CS_MAX_ENC_SZ),
>> + }, *resp = &tmp;
>
> bfields@pig:~/local/linux-2.6$ gdb vmlinux
> GNU gdb 6.8-debian
> ...
> This GDB was configured as "i486-linux-gnu"...
> (gdb) p sizeof(struct nfsd4_compoundres)
> $1 = 588
>
> So that's a 588 byte structure on my machine--better not to put it on
> the stack if we can avoid it.
How did Benny put it -- oh yeah.
Ouch.
Should not be on the stack.
>
> This trick of faking up a temporary nfsd4_compoundres to be able to use
> nfsd4_encode_create_session() unmodified is sort of brilliant but
> possibly also sort of nuts? I don't know. Anyway, maybe:
>
> - rewrite nfsd4_encode_create_session() to call a common
> encode_create_session that takes just the start and end
> pointers, then call that? Or
> - forget the xdr encoding entirely and just save the values we
> care about from nfsd4_create_session? (Heck, there's no
> pointers in there or anything--you could just make a copy of
> the whole struct.)
>
> I'm leaning towards the latter--the xdr encoding seems unnecessary, and
> introduces error-handling for a case (overflowing the CS_MAX_ENC_SZ-size
> buffer) that we'll never hit. And if we're making the clientid slot
> such a special case then I suppose there's no need to xdr-encode for
> uniform handling with the regular sessions slot case.
I like #2 as well - the same method used for the sessions operation. Good idea.
I'll call nfsd4_encode_create_session on replay.
-->Andy
>
> --b.
>
>> +
>> + slot->sl_status = nfsd4_encode_create_session(resp, nfserr, cr_ses);
>> + slot->sl_datalen = (char *)resp->p - (char *)p;
>> +}
>> +
>> static __be32
>> nfsd4_encode_destroy_session(struct nfsd4_compoundres *resp, int nfserr,
>> struct nfsd4_destroy_session *destroy_session)
>> diff --git a/include/linux/nfsd/xdr4.h b/include/linux/nfsd/xdr4.h
>> index afd4464..21e0b73 100644
>> --- a/include/linux/nfsd/xdr4.h
>> +++ b/include/linux/nfsd/xdr4.h
>> @@ -517,6 +517,8 @@ extern __be32 nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
>> extern void nfsd4_store_cache_entry(struct nfsd4_compoundres *resp);
>> extern __be32 nfsd4_replay_cache_entry(struct nfsd4_compoundres *resp,
>> struct nfsd4_sequence *seq);
>> +extern void nfsd4_cache_create_session(struct nfsd4_create_session *cr_ses,
>> + struct nfsd4_clid_slot *slot, int nfserr);
>> extern __be32 nfsd4_exchange_id(struct svc_rqst *rqstp,
>> struct nfsd4_compound_state *,
>> struct nfsd4_exchange_id *);
>> --
>> 1.5.4.3
>>
> _______________________________________________
> pNFS mailing list
> pNFS@xxxxxxxxxxxxx
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
--
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
