Commit de713b57 authored by chas williams - CONTRACTOR's avatar chas williams - CONTRACTOR Committed by David S. Miller

atm/svc: Fix blocking in wait loop

One should not call blocking primitives inside a wait loop, since both
require task_struct::state to sleep, so the inner will destroy the
outer state.

sigd_enq() will possibly sleep for alloc_skb().  Move sigd_enq() before
prepare_to_wait() to avoid sleeping while waiting interruptibly.  You do
not actually need to call sigd_enq() after the initial prepare_to_wait()
because we test the termination condition before calling schedule().

Based on suggestions from Peter Zijlstra.
Signed-off-by: default avatarChas Williams <chas@cmf.n4rl.navy.mil>
Acked-by: default avatarPeter Zijlstra <peterz@infradead.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 10545937
...@@ -50,12 +50,12 @@ static void svc_disconnect(struct atm_vcc *vcc) ...@@ -50,12 +50,12 @@ static void svc_disconnect(struct atm_vcc *vcc)
pr_debug("%p\n", vcc); pr_debug("%p\n", vcc);
if (test_bit(ATM_VF_REGIS, &vcc->flags)) { if (test_bit(ATM_VF_REGIS, &vcc->flags)) {
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
sigd_enq(vcc, as_close, NULL, NULL, NULL); sigd_enq(vcc, as_close, NULL, NULL, NULL);
while (!test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) { for (;;) {
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
if (test_bit(ATM_VF_RELEASED, &vcc->flags) || !sigd)
break;
schedule(); schedule();
prepare_to_wait(sk_sleep(sk), &wait,
TASK_UNINTERRUPTIBLE);
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
} }
...@@ -126,11 +126,12 @@ static int svc_bind(struct socket *sock, struct sockaddr *sockaddr, ...@@ -126,11 +126,12 @@ static int svc_bind(struct socket *sock, struct sockaddr *sockaddr,
} }
vcc->local = *addr; vcc->local = *addr;
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
sigd_enq(vcc, as_bind, NULL, NULL, &vcc->local); sigd_enq(vcc, as_bind, NULL, NULL, &vcc->local);
while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { for (;;) {
schedule();
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
break;
schedule();
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
clear_bit(ATM_VF_REGIS, &vcc->flags); /* doesn't count */ clear_bit(ATM_VF_REGIS, &vcc->flags); /* doesn't count */
...@@ -202,15 +203,14 @@ static int svc_connect(struct socket *sock, struct sockaddr *sockaddr, ...@@ -202,15 +203,14 @@ static int svc_connect(struct socket *sock, struct sockaddr *sockaddr,
} }
vcc->remote = *addr; vcc->remote = *addr;
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sigd_enq(vcc, as_connect, NULL, NULL, &vcc->remote); sigd_enq(vcc, as_connect, NULL, NULL, &vcc->remote);
if (flags & O_NONBLOCK) { if (flags & O_NONBLOCK) {
finish_wait(sk_sleep(sk), &wait);
sock->state = SS_CONNECTING; sock->state = SS_CONNECTING;
error = -EINPROGRESS; error = -EINPROGRESS;
goto out; goto out;
} }
error = 0; error = 0;
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) {
schedule(); schedule();
if (!signal_pending(current)) { if (!signal_pending(current)) {
...@@ -297,11 +297,12 @@ static int svc_listen(struct socket *sock, int backlog) ...@@ -297,11 +297,12 @@ static int svc_listen(struct socket *sock, int backlog)
goto out; goto out;
} }
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
sigd_enq(vcc, as_listen, NULL, NULL, &vcc->local); sigd_enq(vcc, as_listen, NULL, NULL, &vcc->local);
while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { for (;;) {
schedule();
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
break;
schedule();
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
if (!sigd) { if (!sigd) {
...@@ -387,15 +388,15 @@ static int svc_accept(struct socket *sock, struct socket *newsock, int flags) ...@@ -387,15 +388,15 @@ static int svc_accept(struct socket *sock, struct socket *newsock, int flags)
} }
/* wait should be short, so we ignore the non-blocking flag */ /* wait should be short, so we ignore the non-blocking flag */
set_bit(ATM_VF_WAITING, &new_vcc->flags); set_bit(ATM_VF_WAITING, &new_vcc->flags);
prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
TASK_UNINTERRUPTIBLE);
sigd_enq(new_vcc, as_accept, old_vcc, NULL, NULL); sigd_enq(new_vcc, as_accept, old_vcc, NULL, NULL);
while (test_bit(ATM_VF_WAITING, &new_vcc->flags) && sigd) { for (;;) {
prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
TASK_UNINTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &new_vcc->flags) || !sigd)
break;
release_sock(sk); release_sock(sk);
schedule(); schedule();
lock_sock(sk); lock_sock(sk);
prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait,
TASK_UNINTERRUPTIBLE);
} }
finish_wait(sk_sleep(sk_atm(new_vcc)), &wait); finish_wait(sk_sleep(sk_atm(new_vcc)), &wait);
if (!sigd) { if (!sigd) {
...@@ -433,12 +434,14 @@ int svc_change_qos(struct atm_vcc *vcc, struct atm_qos *qos) ...@@ -433,12 +434,14 @@ int svc_change_qos(struct atm_vcc *vcc, struct atm_qos *qos)
DEFINE_WAIT(wait); DEFINE_WAIT(wait);
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
sigd_enq2(vcc, as_modify, NULL, NULL, &vcc->local, qos, 0); sigd_enq2(vcc, as_modify, NULL, NULL, &vcc->local, qos, 0);
while (test_bit(ATM_VF_WAITING, &vcc->flags) && for (;;) {
!test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) {
schedule();
prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &vcc->flags) ||
test_bit(ATM_VF_RELEASED, &vcc->flags) || !sigd) {
break;
}
schedule();
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
if (!sigd) if (!sigd)
...@@ -529,18 +532,18 @@ static int svc_addparty(struct socket *sock, struct sockaddr *sockaddr, ...@@ -529,18 +532,18 @@ static int svc_addparty(struct socket *sock, struct sockaddr *sockaddr,
lock_sock(sk); lock_sock(sk);
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sigd_enq(vcc, as_addparty, NULL, NULL, sigd_enq(vcc, as_addparty, NULL, NULL,
(struct sockaddr_atmsvc *) sockaddr); (struct sockaddr_atmsvc *) sockaddr);
if (flags & O_NONBLOCK) { if (flags & O_NONBLOCK) {
finish_wait(sk_sleep(sk), &wait);
error = -EINPROGRESS; error = -EINPROGRESS;
goto out; goto out;
} }
pr_debug("added wait queue\n"); pr_debug("added wait queue\n");
while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { for (;;) {
schedule();
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
break;
schedule();
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
error = xchg(&sk->sk_err_soft, 0); error = xchg(&sk->sk_err_soft, 0);
...@@ -558,11 +561,12 @@ static int svc_dropparty(struct socket *sock, int ep_ref) ...@@ -558,11 +561,12 @@ static int svc_dropparty(struct socket *sock, int ep_ref)
lock_sock(sk); lock_sock(sk);
set_bit(ATM_VF_WAITING, &vcc->flags); set_bit(ATM_VF_WAITING, &vcc->flags);
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
sigd_enq2(vcc, as_dropparty, NULL, NULL, NULL, NULL, ep_ref); sigd_enq2(vcc, as_dropparty, NULL, NULL, NULL, NULL, ep_ref);
while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { for (;;) {
schedule();
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
if (!test_bit(ATM_VF_WAITING, &vcc->flags) || !sigd)
break;
schedule();
} }
finish_wait(sk_sleep(sk), &wait); finish_wait(sk_sleep(sk), &wait);
if (!sigd) { if (!sigd) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment