Commit fceb8734 authored by NeilBrown's avatar NeilBrown Committed by Chuck Lever
Browse files

nfsd: stop pretending that we cache the SEQUENCE reply. 



nfsd does not cache the reply to a SEQUENCE.  As the comment above
nfsd4_replay_cache_entry() says:

 * The sequence operation is not cached because we can use the slot and
 * session values.

The comment above nfsd4_cache_this() suggests otherwise.

 * The session reply cache only needs to cache replies that the client
 * actually asked us to.  But it's almost free for us to cache compounds
 * consisting of only a SEQUENCE op, so we may as well cache those too.
 * Also, the protocol doesn't give us a convenient response in the case
 * of a replay of a solo SEQUENCE op that wasn't cached

The code in nfsd4_store_cache_entry() makes it clear that only responses
beyond 'cstate.data_offset' are actually cached, and data_offset is set
at the end of nfsd4_encode_sequence() *after* the sequence response has
been encoded.

This patch simplifies code and removes the confusing comments.

- nfsd4_is_solo_sequence() is discarded as not-useful.
- nfsd4_cache_this() is now trivial so it too is discarded with the
  code placed in-line at the one call-site in nfsd4_store_cache_entry().
- nfsd4_enc_sequence_replay() is open-coded in to
  nfsd4_replay_cache_entry(), and then simplified to (hopefully) make
  the process of replaying a reply clearer.

Signed-off-by: default avatarNeilBrown <neil@brown.name>
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 8320b75b

fs/nfsd/nfs4state.c

+18 −40
Original line number Diff line number Diff line
@@ -3508,7 +3508,7 @@ nfsd4_store_cache_entry(struct nfsd4_compoundres *resp) 
	free_svc_cred(&slot->sl_cred);

	copy_cred(&slot->sl_cred, &resp->rqstp->rq_cred);



	if (!nfsd4_cache_this(resp)) {

	if (!(resp->cstate.slot->sl_flags & NFSD4_SLOT_CACHETHIS)) {

		slot->sl_flags &= ~NFSD4_SLOT_CACHED;

		return;

	}
@@ -3522,41 +3522,6 @@ nfsd4_store_cache_entry(struct nfsd4_compoundres *resp) 
	return;

}



/*

 * Encode the replay sequence operation from the slot values.

 * If cachethis is FALSE encode the uncached rep error on the next

 * operation which sets resp->p and increments resp->opcnt for

 * nfs4svc_encode_compoundres.

 *

 */

static __be32

nfsd4_enc_sequence_replay(struct nfsd4_compoundargs *args,

			  struct nfsd4_compoundres *resp)

{

	struct nfsd4_op *op;

	struct nfsd4_slot *slot = resp->cstate.slot;



	/* Encode the replayed sequence operation */

	op = &args->ops[resp->opcnt - 1];

	nfsd4_encode_operation(resp, op);



	if (slot->sl_flags & NFSD4_SLOT_CACHED)

		return op->status;

	if (args->opcnt == 1) {

		/*

		 * The original operation wasn't a solo sequence--we

		 * always cache those--so this retry must not match the

		 * original:

		 */

		op->status = nfserr_seq_false_retry;

	} else {

		op = &args->ops[resp->opcnt++];

		op->status = nfserr_retry_uncached_rep;

		nfsd4_encode_operation(resp, op);

	}

	return op->status;

}



/*

 * The sequence operation is not cached because we can use the slot and

 * session values.
@@ -3565,17 +3530,30 @@ static __be32 
nfsd4_replay_cache_entry(struct nfsd4_compoundres *resp,

			 struct nfsd4_sequence *seq)

{

	struct nfsd4_compoundargs *args = resp->rqstp->rq_argp;

	struct nfsd4_slot *slot = resp->cstate.slot;

	struct xdr_stream *xdr = resp->xdr;

	__be32 *p;

	__be32 status;



	dprintk("--> %s slot %p\n", __func__, slot);



	status = nfsd4_enc_sequence_replay(resp->rqstp->rq_argp, resp);

	if (status)

		return status;

	/* Always encode the SEQUENCE response. */

	nfsd4_encode_operation(resp, &args->ops[0]);

	if (args->opcnt == 1)

		/* A solo SEQUENCE - nothing was cached */

		return args->ops[0].status;



	if (!(slot->sl_flags & NFSD4_SLOT_CACHED)) {

		/* We weren't asked to cache this. */

		struct nfsd4_op *op;



		op = &args->ops[resp->opcnt++];

		op->status = nfserr_retry_uncached_rep;

		nfsd4_encode_operation(resp, op);

		return op->status;

	}



	/* return reply from cache */

	p = xdr_reserve_space(xdr, slot->sl_datalen);

	if (!p) {

		WARN_ON_ONCE(1);

fs/nfsd/xdr4.h

+0 −21
Original line number Diff line number Diff line
@@ -924,27 +924,6 @@ struct nfsd4_compoundres { 
	struct nfsd4_compound_state	cstate;

};



static inline bool nfsd4_is_solo_sequence(struct nfsd4_compoundres *resp)

{

	struct nfsd4_compoundargs *args = resp->rqstp->rq_argp;

	return resp->opcnt == 1 && args->ops[0].opnum == OP_SEQUENCE;

}



/*

 * The session reply cache only needs to cache replies that the client

 * actually asked us to.  But it's almost free for us to cache compounds

 * consisting of only a SEQUENCE op, so we may as well cache those too.

 * Also, the protocol doesn't give us a convenient response in the case

 * of a replay of a solo SEQUENCE op that wasn't cached

 * (RETRY_UNCACHED_REP can only be returned in the second op of a

 * compound).

 */

static inline bool nfsd4_cache_this(struct nfsd4_compoundres *resp)

{

	return (resp->cstate.slot->sl_flags & NFSD4_SLOT_CACHETHIS)

		|| nfsd4_is_solo_sequence(resp);

}



static inline bool nfsd4_last_compound_op(struct svc_rqst *rqstp)

{

	struct nfsd4_compoundres *resp = rqstp->rq_resp;