From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S945481AbcJaSBY (ORCPT ); Mon, 31 Oct 2016 14:01:24 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:34068 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S945310AbcJaSBP (ORCPT ); Mon, 31 Oct 2016 14:01:15 -0400 MIME-Version: 1.0 In-Reply-To: <1477833601.7065.297.camel@edumazet-glaptop3.roam.corp.google.com> References: <1477762981.7065.272.camel@edumazet-glaptop3.roam.corp.google.com> <1477764328.7065.284.camel@edumazet-glaptop3.roam.corp.google.com> <1477833601.7065.297.camel@edumazet-glaptop3.roam.corp.google.com> From: Cong Wang Date: Mon, 31 Oct 2016 11:00:54 -0700 Message-ID: Subject: Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep To: Eric Dumazet Cc: Andrey Konovalov , Peter Zijlstra , Gerrit Renker , "David S. Miller" , dccp@vger.kernel.org, netdev , LKML , Dmitry Vyukov , Eric Dumazet Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Oct 30, 2016 at 6:20 AM, Eric Dumazet wrote: > On Sun, 2016-10-30 at 05:41 +0100, Andrey Konovalov wrote: >> Sorry, the warning is still there. >> >> I'm not sure adding sched_annotate_sleep() does anything, since it's >> defined as (in case CONFIG_DEBUG_ATOMIC_SLEEP is not set): >> # define sched_annotate_sleep() do { } while (0) > > Thanks again for testing. > > But you do have CONFIG_DEBUG_ATOMIC_SLEEP set, which triggers a check in > __might_sleep() : > > WARN_ONCE(current->state != TASK_RUNNING && current->task_state_change, > > Relevant commit is 00845eb968ead28007338b2bb852b8beef816583 > ("sched: don't cause task state changes in nested sleep debugging") > > Another relevant commit was 26cabd31259ba43f68026ce3f62b78094124333f > ("sched, net: Clean up sk_wait_event() vs. might_sleep()") > > Before release_sock() could process the backlog in process context, only > lock_sock() could trigger the issue, so my fix at that time was commit > cb7cf8a33ff73cf638481d1edf883d8968f934f8 ("inet: Clean up > inet_csk_wait_for_connect() vs. might_sleep()") > Thanks for the context, but isn't the original warning reported by Andrey is from inet_wait_for_connect()? You seem only patch some dccp function which is why it is still there? It should be the following, no? diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 9648c97..bbd8159 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -544,6 +544,7 @@ static long inet_wait_for_connect(struct sock *sk, long timeo, int writebias) * without closing the socket. */ while ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) { + sched_annotate_sleep(); release_sock(sk); timeo = schedule_timeout(timeo); lock_sock(sk); From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Date: Mon, 31 Oct 2016 18:00:54 +0000 Subject: Re: net/dccp: warning in dccp_feat_clone_sp_val/__might_sleep Message-Id: List-Id: References: In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: dccp@vger.kernel.org On Sun, Oct 30, 2016 at 6:20 AM, Eric Dumazet wrote: > On Sun, 2016-10-30 at 05:41 +0100, Andrey Konovalov wrote: >> Sorry, the warning is still there. >> >> I'm not sure adding sched_annotate_sleep() does anything, since it's >> defined as (in case CONFIG_DEBUG_ATOMIC_SLEEP is not set): >> # define sched_annotate_sleep() do { } while (0) > > Thanks again for testing. > > But you do have CONFIG_DEBUG_ATOMIC_SLEEP set, which triggers a check in > __might_sleep() : > > WARN_ONCE(current->state != TASK_RUNNING && current->task_state_change, > > Relevant commit is 00845eb968ead28007338b2bb852b8beef816583 > ("sched: don't cause task state changes in nested sleep debugging") > > Another relevant commit was 26cabd31259ba43f68026ce3f62b78094124333f > ("sched, net: Clean up sk_wait_event() vs. might_sleep()") > > Before release_sock() could process the backlog in process context, only > lock_sock() could trigger the issue, so my fix at that time was commit > cb7cf8a33ff73cf638481d1edf883d8968f934f8 ("inet: Clean up > inet_csk_wait_for_connect() vs. might_sleep()") > Thanks for the context, but isn't the original warning reported by Andrey is from inet_wait_for_connect()? You seem only patch some dccp function which is why it is still there? It should be the following, no? diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 9648c97..bbd8159 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -544,6 +544,7 @@ static long inet_wait_for_connect(struct sock *sk, long timeo, int writebias) * without closing the socket. */ while ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) { + sched_annotate_sleep(); release_sock(sk); timeo = schedule_timeout(timeo); lock_sock(sk);