* [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f()
@ 2014-08-05 21:46 ` Fengguang Wu
0 siblings, 0 replies; 17+ messages in thread
From: Fengguang Wu @ 2014-08-05 21:46 UTC (permalink / raw)
To: Peter Zijlstra, netdev; +Cc: LKML, lkp
Greetings,
Here is a microcode/load_module error triggered by debug check commit
64c2181bc433b17f04da8fe8592aa83cceac9606 ("sched: Debug nested sleeps"):
[main] Setsockopt(1 8 80d1000 4) on fd 21 [1:2:1]
[main] Setsockopt(1 2f 80d1000 4) on fd 22 [4:2:60]
[ 14.027148] ------------[ cut here ]------------
[ 14.027864] WARNING: CPU: 0 PID: 210 at kernel/sched/core.c:7088 __might_sleep+0x40/0x68()
[ 14.029295] do not call blocking ops when !TASK_RUNNING; state=2 set at [<c144e379>] prepare_to_wait+0x35/0x56
[ 14.030590] Modules linked in:
[ 14.031136] CPU: 0 PID: 210 Comm: trinity-main Not tainted 3.16.0-02167-g254135e #972
[ 14.032263] 00000000 c0f4de4c c0f4de24 c196630c c0f4de3c c142f01a c1447632 c0f1dbb0
[ 14.033480] 00000002 b0066140 c0f4de54 c142f057 00000009 c0f4de4c c1b3bac8 c0f4de68
[ 14.034640] c0f4de88 c1447632 c1b3bb12 00001bb0 c1b3bac8 00000002 c144e379 c144e379
[ 14.035983] Call Trace:
[ 14.036355] [<c196630c>] dump_stack+0x16/0x18
[ 14.037005] [<c142f01a>] warn_slowpath_common+0x55/0x6c
[ 14.037715] [<c1447632>] ? __might_sleep+0x40/0x68
[ 14.038372] [<c142f057>] warn_slowpath_fmt+0x26/0x2a
[ 14.039097] [<c1447632>] __might_sleep+0x40/0x68
[ 14.039787] [<c144e379>] ? prepare_to_wait+0x35/0x56
[ 14.040595] [<c144e379>] ? prepare_to_wait+0x35/0x56
[ 14.041272] [<c14a837e>] kmem_cache_alloc+0x39/0xb0
[ 14.041934] [<c18fa2de>] ? __alloc_skb+0x3c/0x154
[ 14.042572] [<c18fa2de>] __alloc_skb+0x3c/0x154
[ 14.043339] [<c145117a>] ? mark_held_locks+0x44/0x60
[ 14.044141] [<c1946093>] sigd_enq2+0x2a/0xff
[ 14.044836] [<c1946188>] sigd_enq+0x20/0x2a
[ 14.045405] [<c19467fb>] svc_listen+0x8b/0x11f
[ 14.046009] [<c144e5a6>] ? __wake_up_sync+0xd/0xd
[ 14.046653] [<c18f4132>] SyS_listen+0x37/0x51
[ 14.047423] [<c18f4ce5>] SyS_socketcall+0x90/0x1c0
[ 14.048328] [<c145136e>] ? trace_hardirqs_on+0xb/0xd
[ 14.049061] [<c19729f6>] ? restore_all+0xf/0xf
[ 14.049665] [<c19729bd>] syscall_call+0x7/0x7
[ 14.050253] [<c1970000>] ? __ww_mutex_lock_interruptible+0x165/0x573
[ 14.051147] ---[ end trace 6f1365c63eafedde ]---
[main] Setsockopt(1 2d 80d1000 f0) on fd 25 [1:1:1]
Thanks,
Fengguang
^ permalink raw reply [flat|nested] 17+ messages in thread* [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-05 21:46 ` Fengguang Wu 0 siblings, 0 replies; 17+ messages in thread From: Fengguang Wu @ 2014-08-05 21:46 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 2243 bytes --] Greetings, Here is a microcode/load_module error triggered by debug check commit 64c2181bc433b17f04da8fe8592aa83cceac9606 ("sched: Debug nested sleeps"): [main] Setsockopt(1 8 80d1000 4) on fd 21 [1:2:1] [main] Setsockopt(1 2f 80d1000 4) on fd 22 [4:2:60] [ 14.027148] ------------[ cut here ]------------ [ 14.027864] WARNING: CPU: 0 PID: 210 at kernel/sched/core.c:7088 __might_sleep+0x40/0x68() [ 14.029295] do not call blocking ops when !TASK_RUNNING; state=2 set at [<c144e379>] prepare_to_wait+0x35/0x56 [ 14.030590] Modules linked in: [ 14.031136] CPU: 0 PID: 210 Comm: trinity-main Not tainted 3.16.0-02167-g254135e #972 [ 14.032263] 00000000 c0f4de4c c0f4de24 c196630c c0f4de3c c142f01a c1447632 c0f1dbb0 [ 14.033480] 00000002 b0066140 c0f4de54 c142f057 00000009 c0f4de4c c1b3bac8 c0f4de68 [ 14.034640] c0f4de88 c1447632 c1b3bb12 00001bb0 c1b3bac8 00000002 c144e379 c144e379 [ 14.035983] Call Trace: [ 14.036355] [<c196630c>] dump_stack+0x16/0x18 [ 14.037005] [<c142f01a>] warn_slowpath_common+0x55/0x6c [ 14.037715] [<c1447632>] ? __might_sleep+0x40/0x68 [ 14.038372] [<c142f057>] warn_slowpath_fmt+0x26/0x2a [ 14.039097] [<c1447632>] __might_sleep+0x40/0x68 [ 14.039787] [<c144e379>] ? prepare_to_wait+0x35/0x56 [ 14.040595] [<c144e379>] ? prepare_to_wait+0x35/0x56 [ 14.041272] [<c14a837e>] kmem_cache_alloc+0x39/0xb0 [ 14.041934] [<c18fa2de>] ? __alloc_skb+0x3c/0x154 [ 14.042572] [<c18fa2de>] __alloc_skb+0x3c/0x154 [ 14.043339] [<c145117a>] ? mark_held_locks+0x44/0x60 [ 14.044141] [<c1946093>] sigd_enq2+0x2a/0xff [ 14.044836] [<c1946188>] sigd_enq+0x20/0x2a [ 14.045405] [<c19467fb>] svc_listen+0x8b/0x11f [ 14.046009] [<c144e5a6>] ? __wake_up_sync+0xd/0xd [ 14.046653] [<c18f4132>] SyS_listen+0x37/0x51 [ 14.047423] [<c18f4ce5>] SyS_socketcall+0x90/0x1c0 [ 14.048328] [<c145136e>] ? trace_hardirqs_on+0xb/0xd [ 14.049061] [<c19729f6>] ? restore_all+0xf/0xf [ 14.049665] [<c19729bd>] syscall_call+0x7/0x7 [ 14.050253] [<c1970000>] ? __ww_mutex_lock_interruptible+0x165/0x573 [ 14.051147] ---[ end trace 6f1365c63eafedde ]--- [main] Setsockopt(1 2d 80d1000 f0) on fd 25 [1:1:1] Thanks, Fengguang ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-05 21:46 ` Fengguang Wu @ 2014-08-05 21:54 ` Fengguang Wu -1 siblings, 0 replies; 17+ messages in thread From: Fengguang Wu @ 2014-08-05 21:54 UTC (permalink / raw) To: Peter Zijlstra, netdev; +Cc: LKML, lkp On Wed, Aug 06, 2014 at 05:46:24AM +0800, Fengguang Wu wrote: > Greetings, > > Here is a microcode/load_module error triggered by debug check commit Sorry for the copy&paste error! This warning is not related to microcode/load_module.. Thanks, Fengguang ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-05 21:54 ` Fengguang Wu 0 siblings, 0 replies; 17+ messages in thread From: Fengguang Wu @ 2014-08-05 21:54 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 267 bytes --] On Wed, Aug 06, 2014 at 05:46:24AM +0800, Fengguang Wu wrote: > Greetings, > > Here is a microcode/load_module error triggered by debug check commit Sorry for the copy&paste error! This warning is not related to microcode/load_module.. Thanks, Fengguang ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-05 21:54 ` Fengguang Wu (?) @ 2014-08-06 13:52 ` Nick Krause -1 siblings, 0 replies; 17+ messages in thread From: Nick Krause @ 2014-08-06 13:52 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 1067 bytes --] On Tue, Aug 5, 2014 at 5:54 PM, Fengguang Wu <fengguang.wu@intel.com> wrote: > On Wed, Aug 06, 2014 at 05:46:24AM +0800, Fengguang Wu wrote: >> Greetings, >> >> Here is a microcode/load_module error triggered by debug check commit > > Sorry for the copy&paste error! This warning is not related to > microcode/load_module.. > > Thanks, > Fengguang > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ Hey Fenngaug, I looking through this trace. 213 skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node); 214 if (!skb) 215 goto out; This lines seem to be an issue as after tracing to were you are getting a warning, might_sleep is written as might_sleep_if(flags & __GFP_WAIT); This means that the flag for GFP_WAIT is probably set and sure enough you would be getting an error after sleeping in __alloc_skb. Regards Nick ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-05 21:46 ` Fengguang Wu @ 2014-08-07 15:17 ` Peter Zijlstra -1 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 15:17 UTC (permalink / raw) To: Fengguang Wu; +Cc: netdev, LKML, lkp, Chas Williams, linux-atm-general [-- Attachment #1: Type: text/plain, Size: 3607 bytes --] On Wed, Aug 06, 2014 at 05:46:24AM +0800, Fengguang Wu wrote: > Greetings, > > Here is a microcode/load_module error triggered by debug check commit > 64c2181bc433b17f04da8fe8592aa83cceac9606 ("sched: Debug nested sleeps"): > > [main] Setsockopt(1 8 80d1000 4) on fd 21 [1:2:1] > [main] Setsockopt(1 2f 80d1000 4) on fd 22 [4:2:60] > [ 14.027148] ------------[ cut here ]------------ > [ 14.027864] WARNING: CPU: 0 PID: 210 at kernel/sched/core.c:7088 __might_sleep+0x40/0x68() > [ 14.029295] do not call blocking ops when !TASK_RUNNING; state=2 set at [<c144e379>] prepare_to_wait+0x35/0x56 > [ 14.030590] Modules linked in: > [ 14.031136] CPU: 0 PID: 210 Comm: trinity-main Not tainted 3.16.0-02167-g254135e #972 > [ 14.032263] 00000000 c0f4de4c c0f4de24 c196630c c0f4de3c c142f01a c1447632 c0f1dbb0 > [ 14.033480] 00000002 b0066140 c0f4de54 c142f057 00000009 c0f4de4c c1b3bac8 c0f4de68 > [ 14.034640] c0f4de88 c1447632 c1b3bb12 00001bb0 c1b3bac8 00000002 c144e379 c144e379 > [ 14.035983] Call Trace: > [ 14.036355] [<c196630c>] dump_stack+0x16/0x18 > [ 14.037005] [<c142f01a>] warn_slowpath_common+0x55/0x6c > [ 14.037715] [<c1447632>] ? __might_sleep+0x40/0x68 > [ 14.038372] [<c142f057>] warn_slowpath_fmt+0x26/0x2a > [ 14.039097] [<c1447632>] __might_sleep+0x40/0x68 > [ 14.039787] [<c144e379>] ? prepare_to_wait+0x35/0x56 > [ 14.040595] [<c144e379>] ? prepare_to_wait+0x35/0x56 > [ 14.041272] [<c14a837e>] kmem_cache_alloc+0x39/0xb0 > [ 14.041934] [<c18fa2de>] ? __alloc_skb+0x3c/0x154 > [ 14.042572] [<c18fa2de>] __alloc_skb+0x3c/0x154 > [ 14.043339] [<c145117a>] ? mark_held_locks+0x44/0x60 > [ 14.044141] [<c1946093>] sigd_enq2+0x2a/0xff > [ 14.044836] [<c1946188>] sigd_enq+0x20/0x2a > [ 14.045405] [<c19467fb>] svc_listen+0x8b/0x11f > [ 14.046009] [<c144e5a6>] ? __wake_up_sync+0xd/0xd > [ 14.046653] [<c18f4132>] SyS_listen+0x37/0x51 > [ 14.047423] [<c18f4ce5>] SyS_socketcall+0x90/0x1c0 > [ 14.048328] [<c145136e>] ? trace_hardirqs_on+0xb/0xd > [ 14.049061] [<c19729f6>] ? restore_all+0xf/0xf > [ 14.049665] [<c19729bd>] syscall_call+0x7/0x7 > [ 14.050253] [<c1970000>] ? __ww_mutex_lock_interruptible+0x165/0x573 > [ 14.051147] ---[ end trace 6f1365c63eafedde ]--- > [main] Setsockopt(1 2d 80d1000 f0) on fd 25 [1:1:1] --- Subject: atm: 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. In this instance sigd_enq() will possible sleep for alloc_skb(), now if I understand the code right, we do not actually need to call sigd_enq() after the initial prepare_to_wait(), because we test the termination condition before schedule() anyhow. So we can simply move it up a bit and avoid the entire confusion. Signed-off-by: Peter Zijlstra <peterz@infradead.org> --- net/atm/svc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/atm/svc.c b/net/atm/svc.c index d8e5d0c2ebbc..445ac238b69b 100644 --- a/net/atm/svc.c +++ b/net/atm/svc.c @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) goto out; } 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); + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { schedule(); prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-07 15:17 ` Peter Zijlstra 0 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 15:17 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 3607 bytes --] On Wed, Aug 06, 2014 at 05:46:24AM +0800, Fengguang Wu wrote: > Greetings, > > Here is a microcode/load_module error triggered by debug check commit > 64c2181bc433b17f04da8fe8592aa83cceac9606 ("sched: Debug nested sleeps"): > > [main] Setsockopt(1 8 80d1000 4) on fd 21 [1:2:1] > [main] Setsockopt(1 2f 80d1000 4) on fd 22 [4:2:60] > [ 14.027148] ------------[ cut here ]------------ > [ 14.027864] WARNING: CPU: 0 PID: 210 at kernel/sched/core.c:7088 __might_sleep+0x40/0x68() > [ 14.029295] do not call blocking ops when !TASK_RUNNING; state=2 set at [<c144e379>] prepare_to_wait+0x35/0x56 > [ 14.030590] Modules linked in: > [ 14.031136] CPU: 0 PID: 210 Comm: trinity-main Not tainted 3.16.0-02167-g254135e #972 > [ 14.032263] 00000000 c0f4de4c c0f4de24 c196630c c0f4de3c c142f01a c1447632 c0f1dbb0 > [ 14.033480] 00000002 b0066140 c0f4de54 c142f057 00000009 c0f4de4c c1b3bac8 c0f4de68 > [ 14.034640] c0f4de88 c1447632 c1b3bb12 00001bb0 c1b3bac8 00000002 c144e379 c144e379 > [ 14.035983] Call Trace: > [ 14.036355] [<c196630c>] dump_stack+0x16/0x18 > [ 14.037005] [<c142f01a>] warn_slowpath_common+0x55/0x6c > [ 14.037715] [<c1447632>] ? __might_sleep+0x40/0x68 > [ 14.038372] [<c142f057>] warn_slowpath_fmt+0x26/0x2a > [ 14.039097] [<c1447632>] __might_sleep+0x40/0x68 > [ 14.039787] [<c144e379>] ? prepare_to_wait+0x35/0x56 > [ 14.040595] [<c144e379>] ? prepare_to_wait+0x35/0x56 > [ 14.041272] [<c14a837e>] kmem_cache_alloc+0x39/0xb0 > [ 14.041934] [<c18fa2de>] ? __alloc_skb+0x3c/0x154 > [ 14.042572] [<c18fa2de>] __alloc_skb+0x3c/0x154 > [ 14.043339] [<c145117a>] ? mark_held_locks+0x44/0x60 > [ 14.044141] [<c1946093>] sigd_enq2+0x2a/0xff > [ 14.044836] [<c1946188>] sigd_enq+0x20/0x2a > [ 14.045405] [<c19467fb>] svc_listen+0x8b/0x11f > [ 14.046009] [<c144e5a6>] ? __wake_up_sync+0xd/0xd > [ 14.046653] [<c18f4132>] SyS_listen+0x37/0x51 > [ 14.047423] [<c18f4ce5>] SyS_socketcall+0x90/0x1c0 > [ 14.048328] [<c145136e>] ? trace_hardirqs_on+0xb/0xd > [ 14.049061] [<c19729f6>] ? restore_all+0xf/0xf > [ 14.049665] [<c19729bd>] syscall_call+0x7/0x7 > [ 14.050253] [<c1970000>] ? __ww_mutex_lock_interruptible+0x165/0x573 > [ 14.051147] ---[ end trace 6f1365c63eafedde ]--- > [main] Setsockopt(1 2d 80d1000 f0) on fd 25 [1:1:1] --- Subject: atm: 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. In this instance sigd_enq() will possible sleep for alloc_skb(), now if I understand the code right, we do not actually need to call sigd_enq() after the initial prepare_to_wait(), because we test the termination condition before schedule() anyhow. So we can simply move it up a bit and avoid the entire confusion. Signed-off-by: Peter Zijlstra <peterz@infradead.org> --- net/atm/svc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/atm/svc.c b/net/atm/svc.c index d8e5d0c2ebbc..445ac238b69b 100644 --- a/net/atm/svc.c +++ b/net/atm/svc.c @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) goto out; } 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); + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { schedule(); prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); [-- Attachment #2: attachment.sig --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-07 15:17 ` Peter Zijlstra @ 2014-08-07 16:59 ` chas williams - CONTRACTOR -1 siblings, 0 replies; 17+ messages in thread From: chas williams - CONTRACTOR @ 2014-08-07 16:59 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Fengguang Wu, netdev, LKML, lkp, linux-atm-general On Thu, 7 Aug 2014 17:17:41 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > Subject: atm: 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. > > In this instance sigd_enq() will possible sleep for alloc_skb(), now if > I understand the code right, we do not actually need to call sigd_enq() > after the initial prepare_to_wait(), because we test the termination > condition before schedule() anyhow. > > So we can simply move it up a bit and avoid the entire confusion. > > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > --- > net/atm/svc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/atm/svc.c b/net/atm/svc.c > index d8e5d0c2ebbc..445ac238b69b 100644 > --- a/net/atm/svc.c > +++ b/net/atm/svc.c > @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) > goto out; > } > 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); > + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { > schedule(); > prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); This isn't the only place that we queue a message for the signalling daemon after a prepare_to_wait() uninterruptibly so this patch would be incomplete as is. What bothers me is the TASK_UNINTERRUPTIBLE -- I don't have a good reason why any of these should be sleeping uninterruptibly. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-07 16:59 ` chas williams - CONTRACTOR 0 siblings, 0 replies; 17+ messages in thread From: chas williams - CONTRACTOR @ 2014-08-07 16:59 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 1682 bytes --] On Thu, 7 Aug 2014 17:17:41 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > Subject: atm: 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. > > In this instance sigd_enq() will possible sleep for alloc_skb(), now if > I understand the code right, we do not actually need to call sigd_enq() > after the initial prepare_to_wait(), because we test the termination > condition before schedule() anyhow. > > So we can simply move it up a bit and avoid the entire confusion. > > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > --- > net/atm/svc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/atm/svc.c b/net/atm/svc.c > index d8e5d0c2ebbc..445ac238b69b 100644 > --- a/net/atm/svc.c > +++ b/net/atm/svc.c > @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) > goto out; > } > 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); > + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { > schedule(); > prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); This isn't the only place that we queue a message for the signalling daemon after a prepare_to_wait() uninterruptibly so this patch would be incomplete as is. What bothers me is the TASK_UNINTERRUPTIBLE -- I don't have a good reason why any of these should be sleeping uninterruptibly. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-07 16:59 ` chas williams - CONTRACTOR @ 2014-08-07 17:25 ` Peter Zijlstra -1 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 17:25 UTC (permalink / raw) To: chas williams - CONTRACTOR Cc: Fengguang Wu, netdev, LKML, lkp, linux-atm-general [-- Attachment #1: Type: text/plain, Size: 2093 bytes --] On Thu, Aug 07, 2014 at 12:59:48PM -0400, chas williams - CONTRACTOR wrote: > On Thu, 7 Aug 2014 17:17:41 +0200 > Peter Zijlstra <peterz@infradead.org> wrote: > > > Subject: atm: 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. > > > > In this instance sigd_enq() will possible sleep for alloc_skb(), now if > > I understand the code right, we do not actually need to call sigd_enq() > > after the initial prepare_to_wait(), because we test the termination > > condition before schedule() anyhow. > > > > So we can simply move it up a bit and avoid the entire confusion. > > > > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > > --- > > net/atm/svc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/atm/svc.c b/net/atm/svc.c > > index d8e5d0c2ebbc..445ac238b69b 100644 > > --- a/net/atm/svc.c > > +++ b/net/atm/svc.c > > @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) > > goto out; > > } > > 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); > > + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > > while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { > > schedule(); > > prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > > > This isn't the only place that we queue a message for the signalling > daemon after a prepare_to_wait() uninterruptibly so this patch would > be incomplete as is. I'm not sure I follow, this is the only place you do so while then going to sleep. All other sites don't sleep while they're enqueued on the waitqueue. > What bothers me is the TASK_UNINTERRUPTIBLE -- I don't have a good > reason why any of these should be sleeping uninterruptibly. That's a whole different story, there's tons of ugly in there, but its all ancient code. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-07 17:25 ` Peter Zijlstra 0 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 17:25 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 2093 bytes --] On Thu, Aug 07, 2014 at 12:59:48PM -0400, chas williams - CONTRACTOR wrote: > On Thu, 7 Aug 2014 17:17:41 +0200 > Peter Zijlstra <peterz@infradead.org> wrote: > > > Subject: atm: 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. > > > > In this instance sigd_enq() will possible sleep for alloc_skb(), now if > > I understand the code right, we do not actually need to call sigd_enq() > > after the initial prepare_to_wait(), because we test the termination > > condition before schedule() anyhow. > > > > So we can simply move it up a bit and avoid the entire confusion. > > > > Signed-off-by: Peter Zijlstra <peterz@infradead.org> > > --- > > net/atm/svc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/atm/svc.c b/net/atm/svc.c > > index d8e5d0c2ebbc..445ac238b69b 100644 > > --- a/net/atm/svc.c > > +++ b/net/atm/svc.c > > @@ -297,8 +297,8 @@ static int svc_listen(struct socket *sock, int backlog) > > goto out; > > } > > 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); > > + prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > > while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { > > schedule(); > > prepare_to_wait(sk_sleep(sk), &wait, TASK_UNINTERRUPTIBLE); > > > This isn't the only place that we queue a message for the signalling > daemon after a prepare_to_wait() uninterruptibly so this patch would > be incomplete as is. I'm not sure I follow, this is the only place you do so while then going to sleep. All other sites don't sleep while they're enqueued on the waitqueue. > What bothers me is the TASK_UNINTERRUPTIBLE -- I don't have a good > reason why any of these should be sleeping uninterruptibly. That's a whole different story, there's tons of ugly in there, but its all ancient code. [-- Attachment #2: attachment.sig --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() 2014-08-07 17:25 ` Peter Zijlstra @ 2014-08-07 17:29 ` Peter Zijlstra -1 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 17:29 UTC (permalink / raw) To: chas williams - CONTRACTOR Cc: Fengguang Wu, netdev, LKML, lkp, linux-atm-general [-- Attachment #1: Type: text/plain, Size: 502 bytes --] On Thu, Aug 07, 2014 at 07:25:14PM +0200, Peter Zijlstra wrote: > > This isn't the only place that we queue a message for the signalling > > daemon after a prepare_to_wait() uninterruptibly so this patch would > > be incomplete as is. > > I'm not sure I follow, this is the only place you do so while then going > to sleep. All other sites don't sleep while they're enqueued on the > waitqueue. Argh, my brain is fried, you're quite right. I'll go have another stab at it tomorrow or so. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() @ 2014-08-07 17:29 ` Peter Zijlstra 0 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2014-08-07 17:29 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 502 bytes --] On Thu, Aug 07, 2014 at 07:25:14PM +0200, Peter Zijlstra wrote: > > This isn't the only place that we queue a message for the signalling > > daemon after a prepare_to_wait() uninterruptibly so this patch would > > be incomplete as is. > > I'm not sure I follow, this is the only place you do so while then going > to sleep. All other sites don't sleep while they're enqueued on the > waitqueue. Argh, my brain is fried, you're quite right. I'll go have another stab at it tomorrow or so. [-- Attachment #2: attachment.sig --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] atm/svc: Fix blocking in wait loop 2014-08-07 17:29 ` Peter Zijlstra @ 2014-08-12 12:12 ` chas williams - CONTRACTOR -1 siblings, 0 replies; 17+ messages in thread From: chas williams - CONTRACTOR @ 2014-08-12 12:12 UTC (permalink / raw) To: Peter Zijlstra; +Cc: Fengguang Wu, netdev, LKML, lkp, linux-atm-general, davem 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: Chas Williams <chas@cmf.n4rl.navy.mil> Acked-by: Peter Zijlstra <peterz@infradead.org> --- net/atm/svc.c | 60 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/net/atm/svc.c b/net/atm/svc.c index d8e5d0c2..1ba23f5 100644 --- a/net/atm/svc.c +++ b/net/atm/svc.c @@ -50,12 +50,12 @@ static void svc_disconnect(struct atm_vcc *vcc) pr_debug("%p\n", vcc); 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); - 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(); - prepare_to_wait(sk_sleep(sk), &wait, - TASK_UNINTERRUPTIBLE); } finish_wait(sk_sleep(sk), &wait); } @@ -126,11 +126,12 @@ static int svc_bind(struct socket *sock, struct sockaddr *sockaddr, } vcc->local = *addr; 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); clear_bit(ATM_VF_REGIS, &vcc->flags); /* doesn't count */ @@ -202,15 +203,14 @@ static int svc_connect(struct socket *sock, struct sockaddr *sockaddr, } vcc->remote = *addr; 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); if (flags & O_NONBLOCK) { - finish_wait(sk_sleep(sk), &wait); sock->state = SS_CONNECTING; error = -EINPROGRESS; goto out; } error = 0; + prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { schedule(); if (!signal_pending(current)) { @@ -297,11 +297,12 @@ static int svc_listen(struct socket *sock, int backlog) goto out; } 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) { @@ -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 */ 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); - 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); schedule(); lock_sock(sk); - prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait, - TASK_UNINTERRUPTIBLE); } finish_wait(sk_sleep(sk_atm(new_vcc)), &wait); if (!sigd) { @@ -433,12 +434,14 @@ int svc_change_qos(struct atm_vcc *vcc, struct atm_qos *qos) DEFINE_WAIT(wait); 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && - !test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) @@ -529,18 +532,18 @@ static int svc_addparty(struct socket *sock, struct sockaddr *sockaddr, lock_sock(sk); set_bit(ATM_VF_WAITING, &vcc->flags); - prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); sigd_enq(vcc, as_addparty, NULL, NULL, (struct sockaddr_atmsvc *) sockaddr); if (flags & O_NONBLOCK) { - finish_wait(sk_sleep(sk), &wait); error = -EINPROGRESS; goto out; } pr_debug("added wait queue\n"); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); error = xchg(&sk->sk_err_soft, 0); @@ -558,11 +561,12 @@ static int svc_dropparty(struct socket *sock, int ep_ref) lock_sock(sk); 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) { -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] atm/svc: Fix blocking in wait loop @ 2014-08-12 12:12 ` chas williams - CONTRACTOR 0 siblings, 0 replies; 17+ messages in thread From: chas williams - CONTRACTOR @ 2014-08-12 12:12 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 6098 bytes --] 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: Chas Williams <chas@cmf.n4rl.navy.mil> Acked-by: Peter Zijlstra <peterz@infradead.org> --- net/atm/svc.c | 60 +++++++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/net/atm/svc.c b/net/atm/svc.c index d8e5d0c2..1ba23f5 100644 --- a/net/atm/svc.c +++ b/net/atm/svc.c @@ -50,12 +50,12 @@ static void svc_disconnect(struct atm_vcc *vcc) pr_debug("%p\n", vcc); 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); - 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(); - prepare_to_wait(sk_sleep(sk), &wait, - TASK_UNINTERRUPTIBLE); } finish_wait(sk_sleep(sk), &wait); } @@ -126,11 +126,12 @@ static int svc_bind(struct socket *sock, struct sockaddr *sockaddr, } vcc->local = *addr; 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); clear_bit(ATM_VF_REGIS, &vcc->flags); /* doesn't count */ @@ -202,15 +203,14 @@ static int svc_connect(struct socket *sock, struct sockaddr *sockaddr, } vcc->remote = *addr; 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); if (flags & O_NONBLOCK) { - finish_wait(sk_sleep(sk), &wait); sock->state = SS_CONNECTING; error = -EINPROGRESS; goto out; } error = 0; + prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { schedule(); if (!signal_pending(current)) { @@ -297,11 +297,12 @@ static int svc_listen(struct socket *sock, int backlog) goto out; } 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) { @@ -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 */ 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); - 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); schedule(); lock_sock(sk); - prepare_to_wait(sk_sleep(sk_atm(new_vcc)), &wait, - TASK_UNINTERRUPTIBLE); } finish_wait(sk_sleep(sk_atm(new_vcc)), &wait); if (!sigd) { @@ -433,12 +434,14 @@ int svc_change_qos(struct atm_vcc *vcc, struct atm_qos *qos) DEFINE_WAIT(wait); 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && - !test_bit(ATM_VF_RELEASED, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) @@ -529,18 +532,18 @@ static int svc_addparty(struct socket *sock, struct sockaddr *sockaddr, lock_sock(sk); set_bit(ATM_VF_WAITING, &vcc->flags); - prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); sigd_enq(vcc, as_addparty, NULL, NULL, (struct sockaddr_atmsvc *) sockaddr); if (flags & O_NONBLOCK) { - finish_wait(sk_sleep(sk), &wait); error = -EINPROGRESS; goto out; } pr_debug("added wait queue\n"); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); error = xchg(&sk->sk_err_soft, 0); @@ -558,11 +561,12 @@ static int svc_dropparty(struct socket *sock, int ep_ref) lock_sock(sk); 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); - while (test_bit(ATM_VF_WAITING, &vcc->flags) && sigd) { - schedule(); + for (;;) { 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); if (!sigd) { -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] atm/svc: Fix blocking in wait loop 2014-08-12 12:12 ` chas williams - CONTRACTOR @ 2014-08-12 22:02 ` David Miller -1 siblings, 0 replies; 17+ messages in thread From: David Miller @ 2014-08-12 22:02 UTC (permalink / raw) To: chas; +Cc: peterz, fengguang.wu, netdev, linux-kernel, lkp, linux-atm-general From: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Date: Tue, 12 Aug 2014 08:12:26 -0400 > 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: Chas Williams <chas@cmf.n4rl.navy.mil> > Acked-by: Peter Zijlstra <peterz@infradead.org> Applied. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] atm/svc: Fix blocking in wait loop @ 2014-08-12 22:02 ` David Miller 0 siblings, 0 replies; 17+ messages in thread From: David Miller @ 2014-08-12 22:02 UTC (permalink / raw) To: lkp [-- Attachment #1: Type: text/plain, Size: 735 bytes --] From: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Date: Tue, 12 Aug 2014 08:12:26 -0400 > 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: Chas Williams <chas@cmf.n4rl.navy.mil> > Acked-by: Peter Zijlstra <peterz@infradead.org> Applied. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-08-12 22:02 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-05 21:46 [setsockopt] WARNING: CPU: 0 PID: 1444 at kernel/sched/core.c:7088 __might_sleep+0x51/0x16f() Fengguang Wu 2014-08-05 21:46 ` Fengguang Wu 2014-08-05 21:54 ` Fengguang Wu 2014-08-05 21:54 ` Fengguang Wu 2014-08-06 13:52 ` Nick Krause 2014-08-07 15:17 ` Peter Zijlstra 2014-08-07 15:17 ` Peter Zijlstra 2014-08-07 16:59 ` chas williams - CONTRACTOR 2014-08-07 16:59 ` chas williams - CONTRACTOR 2014-08-07 17:25 ` Peter Zijlstra 2014-08-07 17:25 ` Peter Zijlstra 2014-08-07 17:29 ` Peter Zijlstra 2014-08-07 17:29 ` Peter Zijlstra 2014-08-12 12:12 ` [PATCH] atm/svc: Fix blocking in wait loop chas williams - CONTRACTOR 2014-08-12 12:12 ` chas williams - CONTRACTOR 2014-08-12 22:02 ` David Miller 2014-08-12 22:02 ` 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.