| Message ID | 1426533564.11398.229.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)) > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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))