* [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect
@ 2018-06-20 8:07 Ursula Braun
2018-06-20 22:56 ` Cong Wang
2018-06-21 16:26 ` kbuild test robot
0 siblings, 2 replies; 3+ messages in thread
From: Ursula Braun @ 2018-06-20 8:07 UTC (permalink / raw)
To: davem
Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun,
xiyou.wangcong, hch
The recent poll change may lead to stalls for non-blocking connecting
SMC sockets, since sock_poll_wait is no longer performed on the
internal CLC socket, but on the outer SMC socket. kernel_connect() on
the internal CLC socket returns with -EINPROGRESS, but the wake up
logic does not work in all cases. If the internal CLC socket is still
in state TCP_SYN_SENT when polled, sock_poll_wait() from sock_poll()
does not sleep. It is supposed to sleep till the state of the internal
CLC socket switches to TCP_ESTABLISHED.
This patch temporarily propagates the wait queue from the internal
CLC sock to the SMC sock, till the non-blocking connect() is
finished.
In addition locking is reduced due to the removed poll waits.
Fixes: c0129a061442 ("smc: convert to ->poll_mask")
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 10 ++++++----
net/smc/smc.h | 1 +
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index da7f02edcd37..21c84b924ffb 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -605,6 +605,8 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
smc_copy_sock_settings_to_clc(smc);
tcp_sk(smc->clcsock->sk)->syn_smc = 1;
+ if (flags & O_NONBLOCK)
+ sock->sk->sk_wq = smc->clcsock->sk->sk_wq;
rc = kernel_connect(smc->clcsock, addr, alen, flags);
if (rc)
goto out;
@@ -1285,12 +1287,9 @@ static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
smc = smc_sk(sock->sk);
sock_hold(sk);
- lock_sock(sk);
if ((sk->sk_state == SMC_INIT) || smc->use_fallback) {
/* delegate to CLC child sock */
- release_sock(sk);
mask = smc->clcsock->ops->poll_mask(smc->clcsock, events);
- lock_sock(sk);
sk->sk_err = smc->clcsock->sk->sk_err;
if (sk->sk_err) {
mask |= EPOLLERR;
@@ -1299,7 +1298,10 @@ static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
if (sk->sk_state == SMC_INIT &&
mask & EPOLLOUT &&
smc->clcsock->sk->sk_state != TCP_CLOSE) {
+ sock->sk->sk_wq = smc->smcwq;
+ lock_sock(sk);
rc = __smc_connect(smc);
+ release_sock(sk);
if (rc < 0)
mask |= EPOLLERR;
/* success cases including fallback */
@@ -1334,7 +1336,6 @@ static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
mask |= EPOLLPRI;
}
- release_sock(sk);
sock_put(sk);
return mask;
@@ -1663,6 +1664,7 @@ static int smc_create(struct net *net, struct socket *sock, int protocol,
sk_common_release(sk);
goto out;
}
+ smc->smcwq = sk->sk_wq;
smc->sk.sk_sndbuf = max(smc->clcsock->sk->sk_sndbuf, SMC_BUF_MIN_SIZE);
smc->sk.sk_rcvbuf = max(smc->clcsock->sk->sk_rcvbuf, SMC_BUF_MIN_SIZE);
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 51ae1f10d81a..89d6d7ef973f 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -190,6 +190,7 @@ struct smc_connection {
struct smc_sock { /* smc sock container */
struct sock sk;
struct socket *clcsock; /* internal tcp socket */
+ struct socket_wq *smcwq; /* original smcsock wq */
struct smc_connection conn; /* smc connection */
struct smc_sock *listen_smc; /* listen parent */
struct work_struct tcp_listen_work;/* handle tcp socket accepts */
--
2.16.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect
2018-06-20 8:07 [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect Ursula Braun
@ 2018-06-20 22:56 ` Cong Wang
2018-06-21 16:26 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: Cong Wang @ 2018-06-20 22:56 UTC (permalink / raw)
To: Ursula Braun
Cc: David Miller, Linux Kernel Network Developers, linux-s390,
schwidefsky, Heiko Carstens, raspl, Christoph Hellwig
On Wed, Jun 20, 2018 at 1:07 AM, Ursula Braun <ubraun@linux.ibm.com> wrote:
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index da7f02edcd37..21c84b924ffb 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -605,6 +605,8 @@ static int smc_connect(struct socket *sock, struct sockaddr *addr,
>
> smc_copy_sock_settings_to_clc(smc);
> tcp_sk(smc->clcsock->sk)->syn_smc = 1;
> + if (flags & O_NONBLOCK)
> + sock->sk->sk_wq = smc->clcsock->sk->sk_wq;
> rc = kernel_connect(smc->clcsock, addr, alen, flags);
> if (rc)
> goto out;
> @@ -1285,12 +1287,9 @@ static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
>
> smc = smc_sk(sock->sk);
> sock_hold(sk);
> - lock_sock(sk);
> if ((sk->sk_state == SMC_INIT) || smc->use_fallback) {
> /* delegate to CLC child sock */
> - release_sock(sk);
> mask = smc->clcsock->ops->poll_mask(smc->clcsock, events);
> - lock_sock(sk);
> sk->sk_err = smc->clcsock->sk->sk_err;
> if (sk->sk_err) {
> mask |= EPOLLERR;
> @@ -1299,7 +1298,10 @@ static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
> if (sk->sk_state == SMC_INIT &&
> mask & EPOLLOUT &&
> smc->clcsock->sk->sk_state != TCP_CLOSE) {
> + sock->sk->sk_wq = smc->smcwq;
> + lock_sock(sk);
I think you need to use proper RCU API to protect these
assignment of sk->sk_wq.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect
2018-06-20 8:07 [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect Ursula Braun
2018-06-20 22:56 ` Cong Wang
@ 2018-06-21 16:26 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2018-06-21 16:26 UTC (permalink / raw)
To: Ursula Braun
Cc: kbuild-all, davem, netdev, linux-s390, schwidefsky,
heiko.carstens, raspl, ubraun, xiyou.wangcong, hch
Hi Ursula,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Ursula-Braun/net-smc-coordinate-wait-queues-for-nonblocking-connect/20180620-180901
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/smc/af_smc.c:1301:49: sparse: incorrect type in assignment (different address spaces) @@ expected struct socket_wq [noderef] <asn:4>*sk_wq @@ got [noderef] <asn:4>*sk_wq @@
net/smc/af_smc.c:1301:49: expected struct socket_wq [noderef] <asn:4>*sk_wq
net/smc/af_smc.c:1301:49: got struct socket_wq *smcwq
net/smc/smc_cdc.h:143:24: sparse: expression using sizeof(void)
net/smc/smc_cdc.h:146:16: sparse: expression using sizeof(void)
net/smc/smc_cdc.h:143:24: sparse: expression using sizeof(void)
net/smc/smc_cdc.h:146:16: sparse: expression using sizeof(void)
>> net/smc/af_smc.c:1667:20: sparse: incorrect type in assignment (different address spaces) @@ expected struct socket_wq *smcwq @@ got struct socket_wq struct socket_wq *smcwq @@
net/smc/af_smc.c:1667:20: expected struct socket_wq *smcwq
net/smc/af_smc.c:1667:20: got struct socket_wq [noderef] <asn:4>*sk_wq
net/smc/af_smc.c:1668:29: sparse: expression using sizeof(void)
net/smc/af_smc.c:1669:29: sparse: expression using sizeof(void)
vim +1301 net/smc/af_smc.c
1277
1278 static __poll_t smc_poll_mask(struct socket *sock, __poll_t events)
1279 {
1280 struct sock *sk = sock->sk;
1281 __poll_t mask = 0;
1282 struct smc_sock *smc;
1283 int rc;
1284
1285 if (!sk)
1286 return EPOLLNVAL;
1287
1288 smc = smc_sk(sock->sk);
1289 sock_hold(sk);
1290 if ((sk->sk_state == SMC_INIT) || smc->use_fallback) {
1291 /* delegate to CLC child sock */
1292 mask = smc->clcsock->ops->poll_mask(smc->clcsock, events);
1293 sk->sk_err = smc->clcsock->sk->sk_err;
1294 if (sk->sk_err) {
1295 mask |= EPOLLERR;
1296 } else {
1297 /* if non-blocking connect finished ... */
1298 if (sk->sk_state == SMC_INIT &&
1299 mask & EPOLLOUT &&
1300 smc->clcsock->sk->sk_state != TCP_CLOSE) {
> 1301 sock->sk->sk_wq = smc->smcwq;
1302 lock_sock(sk);
1303 rc = __smc_connect(smc);
1304 release_sock(sk);
1305 if (rc < 0)
1306 mask |= EPOLLERR;
1307 /* success cases including fallback */
1308 mask |= EPOLLOUT | EPOLLWRNORM;
1309 }
1310 }
1311 } else {
1312 if (sk->sk_err)
1313 mask |= EPOLLERR;
1314 if ((sk->sk_shutdown == SHUTDOWN_MASK) ||
1315 (sk->sk_state == SMC_CLOSED))
1316 mask |= EPOLLHUP;
1317 if (sk->sk_state == SMC_LISTEN) {
1318 /* woken up by sk_data_ready in smc_listen_work() */
1319 mask = smc_accept_poll(sk);
1320 } else {
1321 if (atomic_read(&smc->conn.sndbuf_space) ||
1322 sk->sk_shutdown & SEND_SHUTDOWN) {
1323 mask |= EPOLLOUT | EPOLLWRNORM;
1324 } else {
1325 sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
1326 set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
1327 }
1328 if (atomic_read(&smc->conn.bytes_to_rcv))
1329 mask |= EPOLLIN | EPOLLRDNORM;
1330 if (sk->sk_shutdown & RCV_SHUTDOWN)
1331 mask |= EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
1332 if (sk->sk_state == SMC_APPCLOSEWAIT1)
1333 mask |= EPOLLIN;
1334 }
1335 if (smc->conn.urg_state == SMC_URG_VALID)
1336 mask |= EPOLLPRI;
1337
1338 }
1339 sock_put(sk);
1340
1341 return mask;
1342 }
1343
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-21 16:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-20 8:07 [PATCH net 1/1] net/smc: coordinate wait queues for nonblocking connect Ursula Braun
2018-06-20 22:56 ` Cong Wang
2018-06-21 16:26 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).