Commit 7827c81f authored by Chuck Lever's avatar Chuck Lever
Browse files

Revert "SUNRPC: Use RMW bitops in single-threaded hot paths" 



The premise that "Once an svc thread is scheduled and executing an
RPC, no other processes will touch svc_rqst::rq_flags" is false.
svc_xprt_enqueue() examines the RQ_BUSY flag in scheduled nfsd
threads when determining which thread to wake up next.

Found via KCSAN.

Fixes: 28df0988 ("SUNRPC: Use RMW bitops in single-threaded hot paths")
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 0b3a551f

fs/nfsd/nfs4proc.c

+3 −4
Original line number Diff line number Diff line
@@ -937,7 +937,7 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, 
	 * the client wants us to do more in this compound:

	 */

	if (!nfsd4_last_compound_op(rqstp))

		__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);

		clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);



	/* check stateid */

	status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh,
@@ -2607,12 +2607,11 @@ nfsd4_proc_compound(struct svc_rqst *rqstp) 
	cstate->minorversion = args->minorversion;

	fh_init(current_fh, NFS4_FHSIZE);

	fh_init(save_fh, NFS4_FHSIZE);



	/*

	 * Don't use the deferral mechanism for NFSv4; compounds make it

	 * too hard to avoid non-idempotency problems.

	 */

	__clear_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);

	clear_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);



	/*

	 * According to RFC3010, this takes precedence over all other errors.
@@ -2734,7 +2733,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp) 
out:

	cstate->status = status;

	/* Reset deferral mechanism for RPC deferrals */

	__set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);

	set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);

	return rpc_success;

}

fs/nfsd/nfs4xdr.c

+1 −1
Original line number Diff line number Diff line
@@ -2523,7 +2523,7 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp) 
	argp->rqstp->rq_cachetype = cachethis ? RC_REPLBUFF : RC_NOCACHE;



	if (readcount > 1 || max_reply > PAGE_SIZE - auth_slack)

		__clear_bit(RQ_SPLICE_OK, &argp->rqstp->rq_flags);

		clear_bit(RQ_SPLICE_OK, &argp->rqstp->rq_flags);



	return true;

}

net/sunrpc/auth_gss/svcauth_gss.c

+2 −2
Original line number Diff line number Diff line
@@ -923,7 +923,7 @@ unwrap_integ_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct g 
	 * rejecting the server-computed MIC in this somewhat rare case,

	 * do not use splice with the GSS integrity service.

	 */

	__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);

	clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);



	/* Did we already verify the signature on the original pass through? */

	if (rqstp->rq_deferred)
@@ -990,7 +990,7 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs 
	int pad, remaining_len, offset;

	u32 rseqno;



	__clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);

	clear_bit(RQ_SPLICE_OK, &rqstp->rq_flags);



	priv_len = svc_getnl(&buf->head[0]);

	if (rqstp->rq_deferred) {

net/sunrpc/svc.c

+3 −3
Original line number Diff line number Diff line
@@ -1243,10 +1243,10 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) 
		goto err_short_len;



	/* Will be turned off by GSS integrity and privacy services */

	__set_bit(RQ_SPLICE_OK, &rqstp->rq_flags);

	set_bit(RQ_SPLICE_OK, &rqstp->rq_flags);

	/* Will be turned off only when NFSv4 Sessions are used */

	__set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);

	__clear_bit(RQ_DROPME, &rqstp->rq_flags);

	set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);

	clear_bit(RQ_DROPME, &rqstp->rq_flags);



	svc_putu32(resv, rqstp->rq_xid);

net/sunrpc/svc_xprt.c

+1 −1
Original line number Diff line number Diff line
@@ -1238,7 +1238,7 @@ static struct cache_deferred_req *svc_defer(struct cache_req *req) 
	trace_svc_defer(rqstp);

	svc_xprt_get(rqstp->rq_xprt);

	dr->xprt = rqstp->rq_xprt;

	__set_bit(RQ_DROPME, &rqstp->rq_flags);

	set_bit(RQ_DROPME, &rqstp->rq_flags);



	dr->handle.revisit = svc_revisit;

	return &dr->handle;