Re: [Patch net] inet: fix sleeping inside inet_wait_for_connect()

[Eric Dumazet]

On Tue, 2016-11-01 at 16:04 -0700, Cong Wang wrote:
> Andrey reported this kernel warning:

> Unlike commit 26cabd31259ba43f68026ce3f62b78094124333f
> ("sched, net: Clean up sk_wait_event() vs. might_sleep()"), the
> sleeping function is called before schedule_timeout(), this is indeed
> a bug. Fix this by moving the wait logic to the new API, it is similar
> to commit ff960a731788a7408b6f66ec4fd772ff18833211
> ("netdev, sched/wait: Fix sleeping inside wait event").
> 
> Reported-by: Andrey Konovalov <andreyk...@google.com>
> Cc: Andrey Konovalov <andreyk...@google.com>
> Cc: Eric Dumazet <eric.duma...@gmail.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
> ---


Excellent.

I guess we could also define sk_wait_event_woken()
and use it instead of sk_wait_event(), and also in
inet_wait_for_connect()

+#define sk_wait_event_woken(__sk, __timeo, __condition, __wait)                
\
+       ({      int __rc;                                               \
+               release_sock(__sk);                                     \
+               __rc = __condition;                                     \
+               if (!__rc) {                                            \
+                       *(__timeo) = wait_woken(__wait, TASK_INTERRUPTIBLE, \
+                                               *(__timeo));            \
+               }                                                       \
+               lock_sock(__sk);                                        \
+               __rc = __condition;                                     \
+               __rc;                                                   \
+       })

sk_wait_data() would need :

@@ -2078,14 +2080,14 @@ void __sk_flush_backlog(struct sock *sk)
  */
 int sk_wait_data(struct sock *sk, long *timeo, const struct sk_buff *skb)
 {
+       DEFINE_WAIT_FUNC(wait, woken_wake_function);
        int rc;
-       DEFINE_WAIT(wait);
 
-       prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
+       add_wait_queue(sk_sleep(sk), &wait);
        sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
-       rc = sk_wait_event(sk, timeo, skb_peek_tail(&sk->sk_receive_queue) != 
skb);
+       rc = sk_wait_event_woken(sk, timeo, 
skb_peek_tail(&sk->sk_receive_queue) != skb, &wait);
        sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
-       finish_wait(sk_sleep(sk), &wait);
+       remove_wait_queue(sk_sleep(sk), &wait);
        return rc;
 }
 EXPORT_SYMBOL(sk_wait_data);