SUNRPC: fix race to sk_err after xs_error_report
Since commit4f8943f808
("SUNRPC: Replace direct task wakeups from softirq context") there has been a race to the value of the sk_err if both XPRT_SOCK_WAKE_ERROR and XPRT_SOCK_WAKE_DISCONNECT are set. In that case, we may end up losing the sk_err value that existed when xs_error_report was called. Fix this by reverting to the previous behavior: instead of using SO_ERROR to retrieve the value at a later time (which might also return sk_err_soft), copy the sk_err value onto struct sock_xprt, and use that value to wake pending tasks. Signed-off-by: Benjamin Coddington <bcodding@redhat.com> Fixes:4f8943f808
("SUNRPC: Replace direct task wakeups from softirq context") Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
This commit is contained in:
parent
1047ec8683
commit
af84537dbd
|
@ -61,6 +61,7 @@ struct sock_xprt {
|
||||||
struct mutex recv_mutex;
|
struct mutex recv_mutex;
|
||||||
struct sockaddr_storage srcaddr;
|
struct sockaddr_storage srcaddr;
|
||||||
unsigned short srcport;
|
unsigned short srcport;
|
||||||
|
int xprt_err;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* UDP socket buffer size parameters
|
* UDP socket buffer size parameters
|
||||||
|
|
|
@ -1249,19 +1249,21 @@ static void xs_error_report(struct sock *sk)
|
||||||
{
|
{
|
||||||
struct sock_xprt *transport;
|
struct sock_xprt *transport;
|
||||||
struct rpc_xprt *xprt;
|
struct rpc_xprt *xprt;
|
||||||
int err;
|
|
||||||
|
|
||||||
read_lock_bh(&sk->sk_callback_lock);
|
read_lock_bh(&sk->sk_callback_lock);
|
||||||
if (!(xprt = xprt_from_sock(sk)))
|
if (!(xprt = xprt_from_sock(sk)))
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
transport = container_of(xprt, struct sock_xprt, xprt);
|
transport = container_of(xprt, struct sock_xprt, xprt);
|
||||||
err = -sk->sk_err;
|
transport->xprt_err = -sk->sk_err;
|
||||||
if (err == 0)
|
if (transport->xprt_err == 0)
|
||||||
goto out;
|
goto out;
|
||||||
dprintk("RPC: xs_error_report client %p, error=%d...\n",
|
dprintk("RPC: xs_error_report client %p, error=%d...\n",
|
||||||
xprt, -err);
|
xprt, -transport->xprt_err);
|
||||||
trace_rpc_socket_error(xprt, sk->sk_socket, err);
|
trace_rpc_socket_error(xprt, sk->sk_socket, transport->xprt_err);
|
||||||
|
|
||||||
|
/* barrier ensures xprt_err is set before XPRT_SOCK_WAKE_ERROR */
|
||||||
|
smp_mb__before_atomic();
|
||||||
xs_run_error_worker(transport, XPRT_SOCK_WAKE_ERROR);
|
xs_run_error_worker(transport, XPRT_SOCK_WAKE_ERROR);
|
||||||
out:
|
out:
|
||||||
read_unlock_bh(&sk->sk_callback_lock);
|
read_unlock_bh(&sk->sk_callback_lock);
|
||||||
|
@ -2476,7 +2478,6 @@ static void xs_wake_write(struct sock_xprt *transport)
|
||||||
static void xs_wake_error(struct sock_xprt *transport)
|
static void xs_wake_error(struct sock_xprt *transport)
|
||||||
{
|
{
|
||||||
int sockerr;
|
int sockerr;
|
||||||
int sockerr_len = sizeof(sockerr);
|
|
||||||
|
|
||||||
if (!test_bit(XPRT_SOCK_WAKE_ERROR, &transport->sock_state))
|
if (!test_bit(XPRT_SOCK_WAKE_ERROR, &transport->sock_state))
|
||||||
return;
|
return;
|
||||||
|
@ -2485,9 +2486,7 @@ static void xs_wake_error(struct sock_xprt *transport)
|
||||||
goto out;
|
goto out;
|
||||||
if (!test_and_clear_bit(XPRT_SOCK_WAKE_ERROR, &transport->sock_state))
|
if (!test_and_clear_bit(XPRT_SOCK_WAKE_ERROR, &transport->sock_state))
|
||||||
goto out;
|
goto out;
|
||||||
if (kernel_getsockopt(transport->sock, SOL_SOCKET, SO_ERROR,
|
sockerr = xchg(&transport->xprt_err, 0);
|
||||||
(char *)&sockerr, &sockerr_len) != 0)
|
|
||||||
goto out;
|
|
||||||
if (sockerr < 0)
|
if (sockerr < 0)
|
||||||
xprt_wake_pending_tasks(&transport->xprt, sockerr);
|
xprt_wake_pending_tasks(&transport->xprt, sockerr);
|
||||||
out:
|
out:
|
||||||
|
|
Loading…
Reference in New Issue