* [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep()
@ 2015-03-16 19:19 Eric Dumazet
2015-03-16 21:12 ` David Miller
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eric Dumazet @ 2015-03-16 19:19 UTC (permalink / raw)
To: Peter Zijlstra, David Miller; +Cc: netdev, linux-kernel
From: Eric Dumazet <edumazet@google.com>
I got the following trace with current net-next kernel :
[14723.885290] WARNING: CPU: 26 PID: 22658 at kernel/sched/core.c:7285 __might_sleep+0x89/0xa0()
[14723.885325] do not call blocking ops when !TASK_RUNNING; state=1 set at [<ffffffff810e8734>] prepare_to_wait_exclusive+0x34/0xa0
[14723.885355] CPU: 26 PID: 22658 Comm: netserver Not tainted 4.0.0-dbg-DEV #1379
[14723.885359] ffffffff81a223a8 ffff881fae9e7ca8 ffffffff81650b5d 0000000000000001
[14723.885364] ffff881fae9e7cf8 ffff881fae9e7ce8 ffffffff810a72e7 0000000000000000
[14723.885367] ffffffff81a57620 000000000000093a 0000000000000000 ffff881fae9e7e64
[14723.885371] Call Trace:
[14723.885377] [<ffffffff81650b5d>] dump_stack+0x4c/0x65
[14723.885382] [<ffffffff810a72e7>] warn_slowpath_common+0x97/0xe0
[14723.885386] [<ffffffff810a73e6>] warn_slowpath_fmt+0x46/0x50
[14723.885390] [<ffffffff810f4c5d>] ? trace_hardirqs_on_caller+0x10d/0x1d0
[14723.885393] [<ffffffff810e8734>] ? prepare_to_wait_exclusive+0x34/0xa0
[14723.885396] [<ffffffff810e8734>] ? prepare_to_wait_exclusive+0x34/0xa0
[14723.885399] [<ffffffff810ccdc9>] __might_sleep+0x89/0xa0
[14723.885403] [<ffffffff81581846>] lock_sock_nested+0x36/0xb0
[14723.885406] [<ffffffff815829a3>] ? release_sock+0x173/0x1c0
[14723.885411] [<ffffffff815ea1f7>] inet_csk_accept+0x157/0x2a0
[14723.885415] [<ffffffff810e8900>] ? abort_exclusive_wait+0xc0/0xc0
[14723.885419] [<ffffffff8161b96d>] inet_accept+0x2d/0x150
[14723.885424] [<ffffffff8157db6f>] SYSC_accept4+0xff/0x210
[14723.885428] [<ffffffff8165a451>] ? retint_swapgs+0xe/0x44
[14723.885431] [<ffffffff810f4c5d>] ? trace_hardirqs_on_caller+0x10d/0x1d0
[14723.885437] [<ffffffff81369c0e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[14723.885441] [<ffffffff8157ef40>] SyS_accept+0x10/0x20
[14723.885444] [<ffffffff81659872>] system_call_fastpath+0x12/0x17
[14723.885447] ---[ end trace ff74cd83355b1873 ]---
In commit 26cabd31259ba43f68026ce3f62b78094124333f
Peter added a sched_annotate_sleep() in sk_wait_event()
Is the following patch needed as well ?
Alternative would be to use sk_wait_event() from inet_csk_wait_for_connect()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/inet_connection_sock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 14d02ea905b6..3e44b9b0b78e 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -268,6 +268,7 @@ static int inet_csk_wait_for_connect(struct sock *sk, long timeo)
release_sock(sk);
if (reqsk_queue_empty(&icsk->icsk_accept_queue))
timeo = schedule_timeout(timeo);
+ sched_annotate_sleep();
lock_sock(sk);
err = 0;
if (!reqsk_queue_empty(&icsk->icsk_accept_queue))
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep()
2015-03-16 19:19 [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep() Eric Dumazet
@ 2015-03-16 21:12 ` David Miller
2015-03-16 22:24 ` Eric Dumazet
2015-03-17 7:35 ` Peter Zijlstra
2015-03-17 19:04 ` David Miller
2 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2015-03-16 21:12 UTC (permalink / raw)
To: eric.dumazet; +Cc: peterz, netdev, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 16 Mar 2015 12:19:24 -0700
> In commit 26cabd31259ba43f68026ce3f62b78094124333f
> Peter added a sched_annotate_sleep() in sk_wait_event()
>
> Is the following patch needed as well ?
Who is asking this question and who is it directed at?
It looks like something that needs to be resolved before I apply this
patch, right?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep()
2015-03-16 21:12 ` David Miller
@ 2015-03-16 22:24 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2015-03-16 22:24 UTC (permalink / raw)
To: David Miller; +Cc: peterz, netdev, linux-kernel
On Mon, 2015-03-16 at 17:12 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 16 Mar 2015 12:19:24 -0700
>
> > In commit 26cabd31259ba43f68026ce3f62b78094124333f
> > Peter added a sched_annotate_sleep() in sk_wait_event()
> >
> > Is the following patch needed as well ?
>
> Who is asking this question and who is it directed at?
>
> It looks like something that needs to be resolved before I apply this
> patch, right?
I was asking the question to Peter ;)
It's a patch that should enter one of your tree, but I would like a
comment from Peter or anyone fully understanding the issue.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep()
2015-03-16 19:19 [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep() Eric Dumazet
2015-03-16 21:12 ` David Miller
@ 2015-03-17 7:35 ` Peter Zijlstra
2015-03-17 19:04 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2015-03-17 7:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, linux-kernel
On Mon, Mar 16, 2015 at 12:19:24PM -0700, Eric Dumazet wrote:
>
> In commit 26cabd31259ba43f68026ce3f62b78094124333f
> Peter added a sched_annotate_sleep() in sk_wait_event()
>
> Is the following patch needed as well ?
Yes this is fine.
If we had indeed gone through the schedule and got woken we'd have had
TASK_RUNNING here, also when we retry the loop the prepare_to_wait call
will (re)set the TASK_INTERRUPTIBLE state.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/inet_connection_sock.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 14d02ea905b6..3e44b9b0b78e 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -268,6 +268,7 @@ static int inet_csk_wait_for_connect(struct sock *sk, long timeo)
> release_sock(sk);
> if (reqsk_queue_empty(&icsk->icsk_accept_queue))
> timeo = schedule_timeout(timeo);
> + sched_annotate_sleep();
> lock_sock(sk);
> err = 0;
> if (!reqsk_queue_empty(&icsk->icsk_accept_queue))
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep()
2015-03-16 19:19 [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep() Eric Dumazet
2015-03-16 21:12 ` David Miller
2015-03-17 7:35 ` Peter Zijlstra
@ 2015-03-17 19:04 ` David Miller
2 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2015-03-17 19:04 UTC (permalink / raw)
To: eric.dumazet; +Cc: peterz, netdev, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 16 Mar 2015 12:19:24 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> I got the following trace with current net-next kernel :
...
> In commit 26cabd31259ba43f68026ce3f62b78094124333f
> Peter added a sched_annotate_sleep() in sk_wait_event()
>
> Is the following patch needed as well ?
>
> Alternative would be to use sk_wait_event() from inet_csk_wait_for_connect()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-17 19:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-16 19:19 [PATCH] inet: Clean up inet_csk_wait_for_connect() vs. might_sleep() Eric Dumazet
2015-03-16 21:12 ` David Miller
2015-03-16 22:24 ` Eric Dumazet
2015-03-17 7:35 ` Peter Zijlstra
2015-03-17 19:04 ` 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.