From 14bfee9b62702ccac3014f1291943fabebafd5bc Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 28 Apr 2022 11:58:17 +0100 Subject: [PATCH 1/3] sock: dedup sock_def_write_space wmem_alloc checks Except for minor rounding differences the first ->sk_wmem_alloc test in sock_def_write_space() is a hand coded version of sock_writeable(). Replace it with the helper, and also kill the following if duplicating the check. Signed-off-by: Pavel Begunkov Signed-off-by: David S. Miller --- net/core/sock.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/net/core/sock.c b/net/core/sock.c index b164618f2cb6..6ad22fc2eb81 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -3191,15 +3191,14 @@ static void sock_def_write_space(struct sock *sk) /* Do not wake up a writer until he can make "significant" * progress. --DaveM */ - if ((refcount_read(&sk->sk_wmem_alloc) << 1) <= READ_ONCE(sk->sk_sndbuf)) { + if (sock_writeable(sk)) { wq = rcu_dereference(sk->sk_wq); if (skwq_has_sleeper(wq)) wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND); /* Should agree with poll, otherwise some programs break */ - if (sock_writeable(sk)) - sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT); + sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT); } rcu_read_unlock(); From 052ada096842a910327936b4ed203048906eb2c3 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 28 Apr 2022 11:58:18 +0100 Subject: [PATCH 2/3] sock: optimise UDP sock_wfree() refcounting For non SOCK_USE_WRITE_QUEUE sockets, sock_wfree() (atomically) puts ->sk_wmem_alloc twice. It's needed to keep the socket alive while calling ->sk_write_space() after the first put. However, some sockets, such as UDP, are freed by RCU (i.e. SOCK_RCU_FREE) and use already RCU-safe sock_def_write_space(). Carve a fast path for such sockets, put down all refs in one go before calling sock_def_write_space() but guard the socket from being freed by an RCU read section. note: because TCP sockets are marked with SOCK_USE_WRITE_QUEUE it doesn't add extra checks in its path. Signed-off-by: Pavel Begunkov Signed-off-by: David S. Miller --- net/core/sock.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/net/core/sock.c b/net/core/sock.c index 6ad22fc2eb81..ab865b04130b 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -146,6 +146,8 @@ static DEFINE_MUTEX(proto_list_mutex); static LIST_HEAD(proto_list); +static void sock_def_write_space(struct sock *sk); + /** * sk_ns_capable - General socket capability test * @sk: Socket to use a capability on or through @@ -2324,8 +2326,20 @@ void sock_wfree(struct sk_buff *skb) { struct sock *sk = skb->sk; unsigned int len = skb->truesize; + bool free; if (!sock_flag(sk, SOCK_USE_WRITE_QUEUE)) { + if (sock_flag(sk, SOCK_RCU_FREE) && + sk->sk_write_space == sock_def_write_space) { + rcu_read_lock(); + free = refcount_sub_and_test(len, &sk->sk_wmem_alloc); + sock_def_write_space(sk); + rcu_read_unlock(); + if (unlikely(free)) + __sk_free(sk); + return; + } + /* * Keep a reference on sk_wmem_alloc, this will be released * after sk_write_space() call From 0a8afd9f026a7f6c835be0fed2ab709d4133797f Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 28 Apr 2022 11:58:19 +0100 Subject: [PATCH 3/3] sock: optimise sock_def_write_space barriers Now we have a separate path for sock_def_write_space() and can go one step further. When it's called from sock_wfree() we know that there is a preceding atomic for putting down ->sk_wmem_alloc. We can use it to replace to replace smb_mb() with a less expensive smp_mb__after_atomic(). It also removes an extra RCU read lock/unlock as a small bonus. Signed-off-by: Pavel Begunkov Signed-off-by: David S. Miller --- net/core/sock.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/net/core/sock.c b/net/core/sock.c index ab865b04130b..be20a1af20e5 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -146,6 +146,7 @@ static DEFINE_MUTEX(proto_list_mutex); static LIST_HEAD(proto_list); +static void sock_def_write_space_wfree(struct sock *sk); static void sock_def_write_space(struct sock *sk); /** @@ -2333,7 +2334,7 @@ void sock_wfree(struct sk_buff *skb) sk->sk_write_space == sock_def_write_space) { rcu_read_lock(); free = refcount_sub_and_test(len, &sk->sk_wmem_alloc); - sock_def_write_space(sk); + sock_def_write_space_wfree(sk); rcu_read_unlock(); if (unlikely(free)) __sk_free(sk); @@ -3218,6 +3219,29 @@ static void sock_def_write_space(struct sock *sk) rcu_read_unlock(); } +/* An optimised version of sock_def_write_space(), should only be called + * for SOCK_RCU_FREE sockets under RCU read section and after putting + * ->sk_wmem_alloc. + */ +static void sock_def_write_space_wfree(struct sock *sk) +{ + /* Do not wake up a writer until he can make "significant" + * progress. --DaveM + */ + if (sock_writeable(sk)) { + struct socket_wq *wq = rcu_dereference(sk->sk_wq); + + /* rely on refcount_sub from sock_wfree() */ + smp_mb__after_atomic(); + if (wq && waitqueue_active(&wq->wait)) + wake_up_interruptible_sync_poll(&wq->wait, EPOLLOUT | + EPOLLWRNORM | EPOLLWRBAND); + + /* Should agree with poll, otherwise some programs break */ + sk_wake_async(sk, SOCK_WAKE_SPACE, POLL_OUT); + } +} + static void sock_def_destruct(struct sock *sk) { }