Skip to content

Commit

Permalink
Revert "SUNRPC: Use RMW bitops in single-threaded hot paths"
Browse files Browse the repository at this point in the history
commit 7827c81 upstream.

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: 28df098 ("SUNRPC: Use RMW bitops in single-threaded hot paths")
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
chucklever authored and gregkh committed Jan 14, 2023
1 parent 29fbaa4 commit 8cc0e63
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 16 deletions.
7 changes: 3 additions & 4 deletions fs/nfsd/nfs4proc.c
Expand Up @@ -928,7 +928,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,
Expand Down Expand Up @@ -2615,12 +2615,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.
Expand Down Expand Up @@ -2742,7 +2741,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;
}

Expand Down
2 changes: 1 addition & 1 deletion fs/nfsd/nfs4xdr.c
Expand Up @@ -2464,7 +2464,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;
}
Expand Down
4 changes: 2 additions & 2 deletions net/sunrpc/auth_gss/svcauth_gss.c
Expand Up @@ -900,7 +900,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)
Expand Down Expand Up @@ -972,7 +972,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) {
Expand Down
6 changes: 3 additions & 3 deletions net/sunrpc/svc.c
Expand Up @@ -1244,10 +1244,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);

Expand Down
2 changes: 1 addition & 1 deletion net/sunrpc/svc_xprt.c
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions net/sunrpc/svcsock.c
Expand Up @@ -298,9 +298,9 @@ static void svc_sock_setbufsize(struct svc_sock *svsk, unsigned int nreqs)
static void svc_sock_secure_port(struct svc_rqst *rqstp)
{
if (svc_port_is_privileged(svc_addr(rqstp)))
__set_bit(RQ_SECURE, &rqstp->rq_flags);
set_bit(RQ_SECURE, &rqstp->rq_flags);
else
__clear_bit(RQ_SECURE, &rqstp->rq_flags);
clear_bit(RQ_SECURE, &rqstp->rq_flags);
}

/*
Expand Down Expand Up @@ -1008,9 +1008,9 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)
rqstp->rq_xprt_ctxt = NULL;
rqstp->rq_prot = IPPROTO_TCP;
if (test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags))
__set_bit(RQ_LOCAL, &rqstp->rq_flags);
set_bit(RQ_LOCAL, &rqstp->rq_flags);
else
__clear_bit(RQ_LOCAL, &rqstp->rq_flags);
clear_bit(RQ_LOCAL, &rqstp->rq_flags);

p = (__be32 *)rqstp->rq_arg.head[0].iov_base;
calldir = p[1];
Expand Down
2 changes: 1 addition & 1 deletion net/sunrpc/xprtrdma/svc_rdma_transport.c
Expand Up @@ -602,7 +602,7 @@ static int svc_rdma_has_wspace(struct svc_xprt *xprt)

static void svc_rdma_secure_port(struct svc_rqst *rqstp)
{
__set_bit(RQ_SECURE, &rqstp->rq_flags);
set_bit(RQ_SECURE, &rqstp->rq_flags);
}

static void svc_rdma_kill_temp_xprt(struct svc_xprt *xprt)
Expand Down

0 comments on commit 8cc0e63

Please sign in to comment.