Skip to content

Commit 9a34921

Browse files
davem330davem330
committed
Merge branch 'AF_VSOCK-missed-wakeups'
Claudio Imbrenda says: ==================== AF_VSOCK: Shrink the area influenced by prepare_to_wait This patchset applies on net-next. I think I found a problem with the patch submitted by Laura Abbott ( https://lkml.org/lkml/2016/2/4/711 ): we might miss wakeups. Since the condition is not checked between the prepare_to_wait and the schedule(), if a wakeup happens after the condition is checked but before the sleep happens, and we miss it. ( A description of the problem can be found here: http://www.makelinux.net/ldd3/chp-6-sect-2 ). The first patch reverts the previous broken patch, while the second patch properly fixes the sleep-while-waiting issue. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 9a0384c + f7f9b5e commit 9a34921

File tree

1 file changed

+87
-68
lines changed

1 file changed

+87
-68
lines changed

‎net/vmw_vsock/af_vsock.c

Lines changed: 87 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,31 +1209,32 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
12091209

12101210
if (signal_pending(current)) {
12111211
err = sock_intr_errno(timeout);
1212-
goto out_wait_error;
1212+
sk->sk_state = SS_UNCONNECTED;
1213+
sock->state = SS_UNCONNECTED;
1214+
goto out_wait;
12131215
} else if (timeout == 0) {
12141216
err = -ETIMEDOUT;
1215-
goto out_wait_error;
1217+
sk->sk_state = SS_UNCONNECTED;
1218+
sock->state = SS_UNCONNECTED;
1219+
goto out_wait;
12161220
}
12171221

12181222
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
12191223
}
12201224

12211225
if (sk->sk_err) {
12221226
err = -sk->sk_err;
1223-
goto out_wait_error;
1224-
} else
1227+
sk->sk_state = SS_UNCONNECTED;
1228+
sock->state = SS_UNCONNECTED;
1229+
} else {
12251230
err = 0;
1231+
}
12261232

12271233
out_wait:
12281234
finish_wait(sk_sleep(sk), &wait);
12291235
out:
12301236
release_sock(sk);
12311237
return err;
1232-
1233-
out_wait_error:
1234-
sk->sk_state = SS_UNCONNECTED;
1235-
sock->state = SS_UNCONNECTED;
1236-
goto out_wait;
12371238
}
12381239

12391240
static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
@@ -1270,18 +1271,20 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
12701271
listener->sk_err == 0) {
12711272
release_sock(listener);
12721273
timeout = schedule_timeout(timeout);
1274+
finish_wait(sk_sleep(listener), &wait);
12731275
lock_sock(listener);
12741276

12751277
if (signal_pending(current)) {
12761278
err = sock_intr_errno(timeout);
1277-
goto out_wait;
1279+
goto out;
12781280
} else if (timeout == 0) {
12791281
err = -EAGAIN;
1280-
goto out_wait;
1282+
goto out;
12811283
}
12821284

12831285
prepare_to_wait(sk_sleep(listener), &wait, TASK_INTERRUPTIBLE);
12841286
}
1287+
finish_wait(sk_sleep(listener), &wait);
12851288

12861289
if (listener->sk_err)
12871290
err = -listener->sk_err;
@@ -1301,19 +1304,15 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
13011304
*/
13021305
if (err) {
13031306
vconnected->rejected = true;
1304-
release_sock(connected);
1305-
sock_put(connected);
1306-
goto out_wait;
1307+
} else {
1308+
newsock->state = SS_CONNECTED;
1309+
sock_graft(connected, newsock);
13071310
}
13081311

1309-
newsock->state = SS_CONNECTED;
1310-
sock_graft(connected, newsock);
13111312
release_sock(connected);
13121313
sock_put(connected);
13131314
}
13141315

1315-
out_wait:
1316-
finish_wait(sk_sleep(listener), &wait);
13171316
out:
13181317
release_sock(listener);
13191318
return err;
@@ -1557,9 +1556,11 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
15571556
if (err < 0)
15581557
goto out;
15591558

1559+
15601560
while (total_written < len) {
15611561
ssize_t written;
15621562

1563+
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
15631564
while (vsock_stream_has_space(vsk) == 0 &&
15641565
sk->sk_err == 0 &&
15651566
!(sk->sk_shutdown & SEND_SHUTDOWN) &&
@@ -1568,44 +1569,50 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
15681569
/* Don't wait for non-blocking sockets. */
15691570
if (timeout == 0) {
15701571
err = -EAGAIN;
1571-
goto out_wait;
1572+
finish_wait(sk_sleep(sk), &wait);
1573+
goto out_err;
15721574
}
15731575

15741576
err = transport->notify_send_pre_block(vsk, &send_data);
1575-
if (err < 0)
1576-
goto out_wait;
1577+
if (err < 0) {
1578+
finish_wait(sk_sleep(sk), &wait);
1579+
goto out_err;
1580+
}
15771581

15781582
release_sock(sk);
1579-
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
15801583
timeout = schedule_timeout(timeout);
1581-
finish_wait(sk_sleep(sk), &wait);
15821584
lock_sock(sk);
15831585
if (signal_pending(current)) {
15841586
err = sock_intr_errno(timeout);
1585-
goto out_wait;
1587+
finish_wait(sk_sleep(sk), &wait);
1588+
goto out_err;
15861589
} else if (timeout == 0) {
15871590
err = -EAGAIN;
1588-
goto out_wait;
1591+
finish_wait(sk_sleep(sk), &wait);
1592+
goto out_err;
15891593
}
15901594

1595+
prepare_to_wait(sk_sleep(sk), &wait,
1596+
TASK_INTERRUPTIBLE);
15911597
}
1598+
finish_wait(sk_sleep(sk), &wait);
15921599

15931600
/* These checks occur both as part of and after the loop
15941601
* conditional since we need to check before and after
15951602
* sleeping.
15961603
*/
15971604
if (sk->sk_err) {
15981605
err = -sk->sk_err;
1599-
goto out_wait;
1606+
goto out_err;
16001607
} else if ((sk->sk_shutdown & SEND_SHUTDOWN) ||
16011608
(vsk->peer_shutdown & RCV_SHUTDOWN)) {
16021609
err = -EPIPE;
1603-
goto out_wait;
1610+
goto out_err;
16041611
}
16051612

16061613
err = transport->notify_send_pre_enqueue(vsk, &send_data);
16071614
if (err < 0)
1608-
goto out_wait;
1615+
goto out_err;
16091616

16101617
/* Note that enqueue will only write as many bytes as are free
16111618
* in the produce queue, so we don't need to ensure len is
@@ -1618,19 +1625,19 @@ static int vsock_stream_sendmsg(struct socket *sock, struct msghdr *msg,
16181625
len - total_written);
16191626
if (written < 0) {
16201627
err = -ENOMEM;
1621-
goto out_wait;
1628+
goto out_err;
16221629
}
16231630

16241631
total_written += written;
16251632

16261633
err = transport->notify_send_post_enqueue(
16271634
vsk, written, &send_data);
16281635
if (err < 0)
1629-
goto out_wait;
1636+
goto out_err;
16301637

16311638
}
16321639

1633-
out_wait:
1640+
out_err:
16341641
if (total_written > 0)
16351642
err = total_written;
16361643
out:
@@ -1715,18 +1722,59 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
17151722

17161723

17171724
while (1) {
1718-
s64 ready = vsock_stream_has_data(vsk);
1725+
s64 ready;
17191726

1720-
if (ready < 0) {
1721-
/* Invalid queue pair content. XXX This should be
1722-
* changed to a connection reset in a later change.
1723-
*/
1727+
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
1728+
ready = vsock_stream_has_data(vsk);
17241729

1725-
err = -ENOMEM;
1726-
goto out;
1727-
} else if (ready > 0) {
1730+
if (ready == 0) {
1731+
if (sk->sk_err != 0 ||
1732+
(sk->sk_shutdown & RCV_SHUTDOWN) ||
1733+
(vsk->peer_shutdown & SEND_SHUTDOWN)) {
1734+
finish_wait(sk_sleep(sk), &wait);
1735+
break;
1736+
}
1737+
/* Don't wait for non-blocking sockets. */
1738+
if (timeout == 0) {
1739+
err = -EAGAIN;
1740+
finish_wait(sk_sleep(sk), &wait);
1741+
break;
1742+
}
1743+
1744+
err = transport->notify_recv_pre_block(
1745+
vsk, target, &recv_data);
1746+
if (err < 0) {
1747+
finish_wait(sk_sleep(sk), &wait);
1748+
break;
1749+
}
1750+
release_sock(sk);
1751+
timeout = schedule_timeout(timeout);
1752+
lock_sock(sk);
1753+
1754+
if (signal_pending(current)) {
1755+
err = sock_intr_errno(timeout);
1756+
finish_wait(sk_sleep(sk), &wait);
1757+
break;
1758+
} else if (timeout == 0) {
1759+
err = -EAGAIN;
1760+
finish_wait(sk_sleep(sk), &wait);
1761+
break;
1762+
}
1763+
} else {
17281764
ssize_t read;
17291765

1766+
finish_wait(sk_sleep(sk), &wait);
1767+
1768+
if (ready < 0) {
1769+
/* Invalid queue pair content. XXX This should
1770+
* be changed to a connection reset in a later
1771+
* change.
1772+
*/
1773+
1774+
err = -ENOMEM;
1775+
goto out;
1776+
}
1777+
17301778
err = transport->notify_recv_pre_dequeue(
17311779
vsk, target, &recv_data);
17321780
if (err < 0)
@@ -1752,35 +1800,6 @@ vsock_stream_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
17521800
break;
17531801

17541802
target -= read;
1755-
} else {
1756-
if (sk->sk_err != 0 || (sk->sk_shutdown & RCV_SHUTDOWN)
1757-
|| (vsk->peer_shutdown & SEND_SHUTDOWN)) {
1758-
break;
1759-
}
1760-
/* Don't wait for non-blocking sockets. */
1761-
if (timeout == 0) {
1762-
err = -EAGAIN;
1763-
break;
1764-
}
1765-
1766-
err = transport->notify_recv_pre_block(
1767-
vsk, target, &recv_data);
1768-
if (err < 0)
1769-
break;
1770-
1771-
release_sock(sk);
1772-
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
1773-
timeout = schedule_timeout(timeout);
1774-
finish_wait(sk_sleep(sk), &wait);
1775-
lock_sock(sk);
1776-
1777-
if (signal_pending(current)) {
1778-
err = sock_intr_errno(timeout);
1779-
break;
1780-
} else if (timeout == 0) {
1781-
err = -EAGAIN;
1782-
break;
1783-
}
17841803
}
17851804
}
17861805

0 commit comments

Comments
 (0)