All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/1] net/smc: fix 2018-03-07
@ 2018-03-07 12:59 Ursula Braun
  2018-03-07 12:59 ` [PATCH net 1/1] net/smc: wakeup closing listen socket Ursula Braun
  0 siblings, 1 reply; 3+ messages in thread
From: Ursula Braun @ 2018-03-07 12:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

Hi Dave,

please apply the following smc bug fix for net.

Thanks, Ursula

Ursula Braun (1):
  net/smc: wakeup closing listen socket

 net/smc/af_smc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.13.5

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH net 1/1] net/smc: wakeup closing listen socket
  2018-03-07 12:59 [PATCH net 0/1] net/smc: fix 2018-03-07 Ursula Braun
@ 2018-03-07 12:59 ` Ursula Braun
  2018-03-08 17:43   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Ursula Braun @ 2018-03-07 12:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun

Closing of a listen socket wakes up kernel_accept() of
smc_tcp_listen_worker(), and then has to wait till smc_tcp_listen_worker()
gives up the internal clcsock. This wait is non-interruptible, and does
not fit to the interruptible wakeup in smc_tcp_listen_worker().
Thus wakeup fails, and waiting for the closed internal clcsock never ends
before the wait timeout is reached.
This patch introduces the matching non-interruptible wakeup.

Fixes: 127f49705823 ("net/smc: release clcsock from tcp_listen_worker")
Reported-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
 net/smc/af_smc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 8cc97834d4f6..ad4c43eb7e7f 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -946,6 +946,20 @@ static void smc_listen_work(struct work_struct *work)
 	goto enqueue; /* queue new sock with sk_err set */
 }
 
+/* a non-interruptible variant of sock_def_wakeup() for wakeup of
+ * smc_close_wait_listen_clcsock()
+ */
+static void smc_sock_wakeup(struct sock *sk)
+{
+	struct socket_wq *wq;
+
+	rcu_read_lock();
+	wq = rcu_dereference(sk->sk_wq);
+	if (skwq_has_sleeper(wq))
+		wake_up_all(&wq->wait);
+	rcu_read_unlock();
+}
+
 static void smc_tcp_listen_work(struct work_struct *work)
 {
 	struct smc_sock *lsmc = container_of(work, struct smc_sock,
@@ -981,7 +995,7 @@ static void smc_tcp_listen_work(struct work_struct *work)
 	/* no more listening, wake up smc_close_wait_listen_clcsock and
 	 * accept
 	 */
-	lsk->sk_state_change(lsk);
+	smc_sock_wakeup(lsk);
 	sock_put(&lsmc->sk); /* sock_hold in smc_listen */
 }
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net 1/1] net/smc: wakeup closing listen socket
  2018-03-07 12:59 ` [PATCH net 1/1] net/smc: wakeup closing listen socket Ursula Braun
@ 2018-03-08 17:43   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2018-03-08 17:43 UTC (permalink / raw)
  To: ubraun; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl

From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Wed,  7 Mar 2018 13:59:31 +0100

> Closing of a listen socket wakes up kernel_accept() of
> smc_tcp_listen_worker(), and then has to wait till
> smc_tcp_listen_worker() gives up the internal clcsock. This wait is
> non-interruptible, and does not fit to the interruptible wakeup in
> smc_tcp_listen_worker().

I am having a hard time understanding this.

A non-interruptible sleep just means that the sleeper can only be
woken up by the event itself.  Said another way, signals and other
unrelated events do not wake up the sleeper.

So as long as the socket close emits the wakeup event, the
non-interruptible sleeper will wake up.

It is also a huge red flag that you feel that you need custom handling
in this area and can't use the generic infrastructure and routines for
this.

Without a better understanding and explanation, there is no way I am
applying this.  It seems that you guys do a lot of things in custom
ways that really should follow the methods and mechanisms used by
other protocol families in the tree.

Thank you.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-03-08 17:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07 12:59 [PATCH net 0/1] net/smc: fix 2018-03-07 Ursula Braun
2018-03-07 12:59 ` [PATCH net 1/1] net/smc: wakeup closing listen socket Ursula Braun
2018-03-08 17:43   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.