* [PATCH net-next v2 0/2] net: cleanup datagram receive helpers
@ 2020-02-28 13:45 Paolo Abeni
2020-02-28 13:45 ` [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paolo Abeni @ 2020-02-28 13:45 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Kirill Tkhai
Several receive helpers have an optional destructor argument, which uglify
the code a bit and is taxed by retpoline overhead.
This series refactor the code so that we can drop such optional argument,
cleaning the helpers a bit and avoiding an indirect call in fast path.
The first patch refactor a bit the caller, so that the second patch
actually dropping the argument is more straight-forward
v1 -> v2:
- call scm_stat_del() only when not peeking - Kirill
- fix build issue with CONFIG_INET_ESPINTCP
Paolo Abeni (2):
unix: uses an atomic type for scm files accounting
net: datagram: drop 'destructor' argument from several helpers
include/linux/skbuff.h | 12 ++----------
include/net/af_unix.h | 2 +-
net/core/datagram.c | 25 +++++++------------------
net/ipv4/udp.c | 14 ++++++++------
net/unix/af_unix.c | 28 +++++++++++-----------------
net/xfrm/espintcp.c | 2 +-
6 files changed, 30 insertions(+), 53 deletions(-)
--
2.21.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting
2020-02-28 13:45 [PATCH net-next v2 0/2] net: cleanup datagram receive helpers Paolo Abeni
@ 2020-02-28 13:45 ` Paolo Abeni
2020-02-28 14:43 ` Kirill Tkhai
2020-02-28 13:45 ` [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
2020-02-28 20:13 ` [PATCH net-next v2 0/2] net: cleanup datagram receive helpers David Miller
2 siblings, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2020-02-28 13:45 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Kirill Tkhai
So the scm_stat_{add,del} helper can be invoked with no
additional lock held.
This clean-up the code a bit and will make the next
patch easier.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/af_unix.h | 2 +-
net/unix/af_unix.c | 21 ++++++---------------
2 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 17e10fba2152..5cb65227b7a9 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -42,7 +42,7 @@ struct unix_skb_parms {
} __randomize_layout;
struct scm_stat {
- u32 nr_fds;
+ atomic_t nr_fds;
};
#define UNIXCB(skb) (*(struct unix_skb_parms *)&((skb)->cb))
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index cbd7dc01e147..145a3965341e 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -689,7 +689,8 @@ static void unix_show_fdinfo(struct seq_file *m, struct socket *sock)
if (sk) {
u = unix_sk(sock->sk);
- seq_printf(m, "scm_fds: %u\n", READ_ONCE(u->scm_stat.nr_fds));
+ seq_printf(m, "scm_fds: %u\n",
+ atomic_read(&u->scm_stat.nr_fds));
}
}
@@ -1598,10 +1599,8 @@ static void scm_stat_add(struct sock *sk, struct sk_buff *skb)
struct scm_fp_list *fp = UNIXCB(skb).fp;
struct unix_sock *u = unix_sk(sk);
- lockdep_assert_held(&sk->sk_receive_queue.lock);
-
if (unlikely(fp && fp->count))
- u->scm_stat.nr_fds += fp->count;
+ atomic_add(fp->count, &u->scm_stat.nr_fds);
}
static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
@@ -1609,10 +1608,8 @@ static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
struct scm_fp_list *fp = UNIXCB(skb).fp;
struct unix_sock *u = unix_sk(sk);
- lockdep_assert_held(&sk->sk_receive_queue.lock);
-
if (unlikely(fp && fp->count))
- u->scm_stat.nr_fds -= fp->count;
+ atomic_sub(fp->count, &u->scm_stat.nr_fds);
}
/*
@@ -1801,10 +1798,8 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
if (sock_flag(other, SOCK_RCVTSTAMP))
__net_timestamp(skb);
maybe_add_creds(skb, sock, other);
- spin_lock(&other->sk_receive_queue.lock);
scm_stat_add(other, skb);
- __skb_queue_tail(&other->sk_receive_queue, skb);
- spin_unlock(&other->sk_receive_queue.lock);
+ skb_queue_tail(&other->sk_receive_queue, skb);
unix_state_unlock(other);
other->sk_data_ready(other);
sock_put(other);
@@ -1906,10 +1901,8 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
goto pipe_err_free;
maybe_add_creds(skb, sock, other);
- spin_lock(&other->sk_receive_queue.lock);
scm_stat_add(other, skb);
- __skb_queue_tail(&other->sk_receive_queue, skb);
- spin_unlock(&other->sk_receive_queue.lock);
+ skb_queue_tail(&other->sk_receive_queue, skb);
unix_state_unlock(other);
other->sk_data_ready(other);
sent += size;
@@ -2405,9 +2398,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
sk_peek_offset_bwd(sk, chunk);
if (UNIXCB(skb).fp) {
- spin_lock(&sk->sk_receive_queue.lock);
scm_stat_del(sk, skb);
- spin_unlock(&sk->sk_receive_queue.lock);
unix_detach_fds(&scm, skb);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers
2020-02-28 13:45 [PATCH net-next v2 0/2] net: cleanup datagram receive helpers Paolo Abeni
2020-02-28 13:45 ` [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
@ 2020-02-28 13:45 ` Paolo Abeni
2020-02-28 14:50 ` Kirill Tkhai
2020-02-28 20:13 ` [PATCH net-next v2 0/2] net: cleanup datagram receive helpers David Miller
2 siblings, 1 reply; 7+ messages in thread
From: Paolo Abeni @ 2020-02-28 13:45 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Willem de Bruijn, Kirill Tkhai
The only users for such argument are the UDP protocol and the UNIX
socket family. We can safely reclaim the accounted memory directly
from the UDP code and, after the previous patch, we can do scm
stats accounting outside the datagram helpers.
Overall this cleans up a bit some datagram-related helpers, and
avoids an indirect call per packet in the UDP receive path.
v1 -> v2:
- call scm_stat_del() only when not peeking - Kirill
- fix build issue with CONFIG_INET_ESPINTCP
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skbuff.h | 12 ++----------
net/core/datagram.c | 25 +++++++------------------
net/ipv4/udp.c | 14 ++++++++------
net/unix/af_unix.c | 7 +++++--
net/xfrm/espintcp.c | 2 +-
5 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5b50278c4bc8..21749b2cdc9b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3514,23 +3514,15 @@ int __skb_wait_for_more_packets(struct sock *sk, struct sk_buff_head *queue,
struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
struct sk_buff_head *queue,
unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
int *off, int *err,
struct sk_buff **last);
struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
struct sk_buff_head *queue,
- unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
- int *off, int *err,
+ unsigned int flags, int *off, int *err,
struct sk_buff **last);
struct sk_buff *__skb_recv_datagram(struct sock *sk,
struct sk_buff_head *sk_queue,
- unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
- int *off, int *err);
+ unsigned int flags, int *off, int *err);
struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags, int noblock,
int *err);
__poll_t datagram_poll(struct file *file, struct socket *sock,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index a78e7f864c1e..4213081c6ed3 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -166,8 +166,6 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
struct sk_buff_head *queue,
unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
int *off, int *err,
struct sk_buff **last)
{
@@ -198,8 +196,6 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
refcount_inc(&skb->users);
} else {
__skb_unlink(skb, queue);
- if (destructor)
- destructor(sk, skb);
}
*off = _off;
return skb;
@@ -212,7 +208,6 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
* @sk: socket
* @queue: socket queue from which to receive
* @flags: MSG\_ flags
- * @destructor: invoked under the receive lock on successful dequeue
* @off: an offset in bytes to peek skb from. Returns an offset
* within an skb where data actually starts
* @err: error code returned
@@ -245,10 +240,7 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
*/
struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
struct sk_buff_head *queue,
- unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
- int *off, int *err,
+ unsigned int flags, int *off, int *err,
struct sk_buff **last)
{
struct sk_buff *skb;
@@ -269,8 +261,8 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
* However, this function was correct in any case. 8)
*/
spin_lock_irqsave(&queue->lock, cpu_flags);
- skb = __skb_try_recv_from_queue(sk, queue, flags, destructor,
- off, &error, last);
+ skb = __skb_try_recv_from_queue(sk, queue, flags, off, &error,
+ last);
spin_unlock_irqrestore(&queue->lock, cpu_flags);
if (error)
goto no_packet;
@@ -293,10 +285,7 @@ EXPORT_SYMBOL(__skb_try_recv_datagram);
struct sk_buff *__skb_recv_datagram(struct sock *sk,
struct sk_buff_head *sk_queue,
- unsigned int flags,
- void (*destructor)(struct sock *sk,
- struct sk_buff *skb),
- int *off, int *err)
+ unsigned int flags, int *off, int *err)
{
struct sk_buff *skb, *last;
long timeo;
@@ -304,8 +293,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk,
timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
do {
- skb = __skb_try_recv_datagram(sk, sk_queue, flags, destructor,
- off, err, &last);
+ skb = __skb_try_recv_datagram(sk, sk_queue, flags, off, err,
+ &last);
if (skb)
return skb;
@@ -326,7 +315,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned int flags,
return __skb_recv_datagram(sk, &sk->sk_receive_queue,
flags | (noblock ? MSG_DONTWAIT : 0),
- NULL, &off, err);
+ &off, err);
}
EXPORT_SYMBOL(skb_recv_datagram);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 08a41f1e1cd2..a68e2ac37f26 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1671,10 +1671,11 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
error = -EAGAIN;
do {
spin_lock_bh(&queue->lock);
- skb = __skb_try_recv_from_queue(sk, queue, flags,
- udp_skb_destructor,
- off, err, &last);
+ skb = __skb_try_recv_from_queue(sk, queue, flags, off,
+ err, &last);
if (skb) {
+ if (!(flags & MSG_PEEK))
+ udp_skb_destructor(sk, skb);
spin_unlock_bh(&queue->lock);
return skb;
}
@@ -1692,9 +1693,10 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
spin_lock(&sk_queue->lock);
skb_queue_splice_tail_init(sk_queue, queue);
- skb = __skb_try_recv_from_queue(sk, queue, flags,
- udp_skb_dtor_locked,
- off, err, &last);
+ skb = __skb_try_recv_from_queue(sk, queue, flags, off,
+ err, &last);
+ if (skb && !(flags & MSG_PEEK))
+ udp_skb_dtor_locked(sk, skb);
spin_unlock(&sk_queue->lock);
spin_unlock_bh(&queue->lock);
if (skb)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 145a3965341e..103bf94d2ab5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2102,9 +2102,12 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
skip = sk_peek_offset(sk, flags);
skb = __skb_try_recv_datagram(sk, &sk->sk_receive_queue, flags,
- scm_stat_del, &skip, &err, &last);
- if (skb)
+ &skip, &err, &last);
+ if (skb) {
+ if (!(flags & MSG_PEEK))
+ scm_stat_del(sk, skb);
break;
+ }
mutex_unlock(&u->iolock);
diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index f15d6a564b0e..037ea156d2f9 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -100,7 +100,7 @@ static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
flags |= nonblock ? MSG_DONTWAIT : 0;
- skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
+ skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, &off, &err);
if (!skb)
return err;
--
2.21.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting
2020-02-28 13:45 ` [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
@ 2020-02-28 14:43 ` Kirill Tkhai
0 siblings, 0 replies; 7+ messages in thread
From: Kirill Tkhai @ 2020-02-28 14:43 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Willem de Bruijn
On 28.02.2020 16:45, Paolo Abeni wrote:
> So the scm_stat_{add,del} helper can be invoked with no
> additional lock held.
>
> This clean-up the code a bit and will make the next
> patch easier.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
> include/net/af_unix.h | 2 +-
> net/unix/af_unix.c | 21 ++++++---------------
> 2 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 17e10fba2152..5cb65227b7a9 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -42,7 +42,7 @@ struct unix_skb_parms {
> } __randomize_layout;
>
> struct scm_stat {
> - u32 nr_fds;
> + atomic_t nr_fds;
> };
>
> #define UNIXCB(skb) (*(struct unix_skb_parms *)&((skb)->cb))
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index cbd7dc01e147..145a3965341e 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -689,7 +689,8 @@ static void unix_show_fdinfo(struct seq_file *m, struct socket *sock)
>
> if (sk) {
> u = unix_sk(sock->sk);
> - seq_printf(m, "scm_fds: %u\n", READ_ONCE(u->scm_stat.nr_fds));
> + seq_printf(m, "scm_fds: %u\n",
> + atomic_read(&u->scm_stat.nr_fds));
> }
> }
>
> @@ -1598,10 +1599,8 @@ static void scm_stat_add(struct sock *sk, struct sk_buff *skb)
> struct scm_fp_list *fp = UNIXCB(skb).fp;
> struct unix_sock *u = unix_sk(sk);
>
> - lockdep_assert_held(&sk->sk_receive_queue.lock);
> -
> if (unlikely(fp && fp->count))
> - u->scm_stat.nr_fds += fp->count;
> + atomic_add(fp->count, &u->scm_stat.nr_fds);
> }
>
> static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
> @@ -1609,10 +1608,8 @@ static void scm_stat_del(struct sock *sk, struct sk_buff *skb)
> struct scm_fp_list *fp = UNIXCB(skb).fp;
> struct unix_sock *u = unix_sk(sk);
>
> - lockdep_assert_held(&sk->sk_receive_queue.lock);
> -
> if (unlikely(fp && fp->count))
> - u->scm_stat.nr_fds -= fp->count;
> + atomic_sub(fp->count, &u->scm_stat.nr_fds);
> }
>
> /*
> @@ -1801,10 +1798,8 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
> if (sock_flag(other, SOCK_RCVTSTAMP))
> __net_timestamp(skb);
> maybe_add_creds(skb, sock, other);
> - spin_lock(&other->sk_receive_queue.lock);
> scm_stat_add(other, skb);
> - __skb_queue_tail(&other->sk_receive_queue, skb);
> - spin_unlock(&other->sk_receive_queue.lock);
> + skb_queue_tail(&other->sk_receive_queue, skb);
> unix_state_unlock(other);
> other->sk_data_ready(other);
> sock_put(other);
> @@ -1906,10 +1901,8 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> goto pipe_err_free;
>
> maybe_add_creds(skb, sock, other);
> - spin_lock(&other->sk_receive_queue.lock);
> scm_stat_add(other, skb);
> - __skb_queue_tail(&other->sk_receive_queue, skb);
> - spin_unlock(&other->sk_receive_queue.lock);
> + skb_queue_tail(&other->sk_receive_queue, skb);
> unix_state_unlock(other);
> other->sk_data_ready(other);
> sent += size;
> @@ -2405,9 +2398,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
> sk_peek_offset_bwd(sk, chunk);
>
> if (UNIXCB(skb).fp) {
> - spin_lock(&sk->sk_receive_queue.lock);
> scm_stat_del(sk, skb);
> - spin_unlock(&sk->sk_receive_queue.lock);
> unix_detach_fds(&scm, skb);
> }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers
2020-02-28 13:45 ` [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
@ 2020-02-28 14:50 ` Kirill Tkhai
2020-02-28 16:10 ` Willem de Bruijn
0 siblings, 1 reply; 7+ messages in thread
From: Kirill Tkhai @ 2020-02-28 14:50 UTC (permalink / raw)
To: Paolo Abeni, netdev; +Cc: David S. Miller, Willem de Bruijn
On 28.02.2020 16:45, Paolo Abeni wrote:
> The only users for such argument are the UDP protocol and the UNIX
> socket family. We can safely reclaim the accounted memory directly
> from the UDP code and, after the previous patch, we can do scm
> stats accounting outside the datagram helpers.
>
> Overall this cleans up a bit some datagram-related helpers, and
> avoids an indirect call per packet in the UDP receive path.
>
> v1 -> v2:
> - call scm_stat_del() only when not peeking - Kirill
> - fix build issue with CONFIG_INET_ESPINTCP
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
> include/linux/skbuff.h | 12 ++----------
> net/core/datagram.c | 25 +++++++------------------
> net/ipv4/udp.c | 14 ++++++++------
> net/unix/af_unix.c | 7 +++++--
> net/xfrm/espintcp.c | 2 +-
> 5 files changed, 23 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 5b50278c4bc8..21749b2cdc9b 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3514,23 +3514,15 @@ int __skb_wait_for_more_packets(struct sock *sk, struct sk_buff_head *queue,
> struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> struct sk_buff_head *queue,
> unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> int *off, int *err,
> struct sk_buff **last);
> struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
> struct sk_buff_head *queue,
> - unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> - int *off, int *err,
> + unsigned int flags, int *off, int *err,
> struct sk_buff **last);
> struct sk_buff *__skb_recv_datagram(struct sock *sk,
> struct sk_buff_head *sk_queue,
> - unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> - int *off, int *err);
> + unsigned int flags, int *off, int *err);
> struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags, int noblock,
> int *err);
> __poll_t datagram_poll(struct file *file, struct socket *sock,
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index a78e7f864c1e..4213081c6ed3 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -166,8 +166,6 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
> struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> struct sk_buff_head *queue,
> unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> int *off, int *err,
> struct sk_buff **last)
> {
> @@ -198,8 +196,6 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> refcount_inc(&skb->users);
> } else {
> __skb_unlink(skb, queue);
> - if (destructor)
> - destructor(sk, skb);
> }
> *off = _off;
> return skb;
> @@ -212,7 +208,6 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> * @sk: socket
> * @queue: socket queue from which to receive
> * @flags: MSG\_ flags
> - * @destructor: invoked under the receive lock on successful dequeue
> * @off: an offset in bytes to peek skb from. Returns an offset
> * within an skb where data actually starts
> * @err: error code returned
> @@ -245,10 +240,7 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock *sk,
> */
> struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
> struct sk_buff_head *queue,
> - unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> - int *off, int *err,
> + unsigned int flags, int *off, int *err,
> struct sk_buff **last)
> {
> struct sk_buff *skb;
> @@ -269,8 +261,8 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk,
> * However, this function was correct in any case. 8)
> */
> spin_lock_irqsave(&queue->lock, cpu_flags);
> - skb = __skb_try_recv_from_queue(sk, queue, flags, destructor,
> - off, &error, last);
> + skb = __skb_try_recv_from_queue(sk, queue, flags, off, &error,
> + last);
> spin_unlock_irqrestore(&queue->lock, cpu_flags);
> if (error)
> goto no_packet;
> @@ -293,10 +285,7 @@ EXPORT_SYMBOL(__skb_try_recv_datagram);
>
> struct sk_buff *__skb_recv_datagram(struct sock *sk,
> struct sk_buff_head *sk_queue,
> - unsigned int flags,
> - void (*destructor)(struct sock *sk,
> - struct sk_buff *skb),
> - int *off, int *err)
> + unsigned int flags, int *off, int *err)
> {
> struct sk_buff *skb, *last;
> long timeo;
> @@ -304,8 +293,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk,
> timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
>
> do {
> - skb = __skb_try_recv_datagram(sk, sk_queue, flags, destructor,
> - off, err, &last);
> + skb = __skb_try_recv_datagram(sk, sk_queue, flags, off, err,
> + &last);
> if (skb)
> return skb;
>
> @@ -326,7 +315,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned int flags,
>
> return __skb_recv_datagram(sk, &sk->sk_receive_queue,
> flags | (noblock ? MSG_DONTWAIT : 0),
> - NULL, &off, err);
> + &off, err);
> }
> EXPORT_SYMBOL(skb_recv_datagram);
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 08a41f1e1cd2..a68e2ac37f26 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1671,10 +1671,11 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
> error = -EAGAIN;
> do {
> spin_lock_bh(&queue->lock);
> - skb = __skb_try_recv_from_queue(sk, queue, flags,
> - udp_skb_destructor,
> - off, err, &last);
> + skb = __skb_try_recv_from_queue(sk, queue, flags, off,
> + err, &last);
> if (skb) {
> + if (!(flags & MSG_PEEK))
> + udp_skb_destructor(sk, skb);
> spin_unlock_bh(&queue->lock);
> return skb;
> }
> @@ -1692,9 +1693,10 @@ struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
> spin_lock(&sk_queue->lock);
> skb_queue_splice_tail_init(sk_queue, queue);
>
> - skb = __skb_try_recv_from_queue(sk, queue, flags,
> - udp_skb_dtor_locked,
> - off, err, &last);
> + skb = __skb_try_recv_from_queue(sk, queue, flags, off,
> + err, &last);
> + if (skb && !(flags & MSG_PEEK))
> + udp_skb_dtor_locked(sk, skb);
> spin_unlock(&sk_queue->lock);
> spin_unlock_bh(&queue->lock);
> if (skb)
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 145a3965341e..103bf94d2ab5 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2102,9 +2102,12 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
>
> skip = sk_peek_offset(sk, flags);
> skb = __skb_try_recv_datagram(sk, &sk->sk_receive_queue, flags,
> - scm_stat_del, &skip, &err, &last);
> - if (skb)
> + &skip, &err, &last);
> + if (skb) {
> + if (!(flags & MSG_PEEK))
> + scm_stat_del(sk, skb);
> break;
> + }
>
> mutex_unlock(&u->iolock);
>
> diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
> index f15d6a564b0e..037ea156d2f9 100644
> --- a/net/xfrm/espintcp.c
> +++ b/net/xfrm/espintcp.c
> @@ -100,7 +100,7 @@ static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>
> flags |= nonblock ? MSG_DONTWAIT : 0;
>
> - skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
> + skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, &off, &err);
> if (!skb)
> return err;
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers
2020-02-28 14:50 ` Kirill Tkhai
@ 2020-02-28 16:10 ` Willem de Bruijn
0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2020-02-28 16:10 UTC (permalink / raw)
To: Kirill Tkhai; +Cc: Paolo Abeni, Network Development, David S. Miller
On Fri, Feb 28, 2020 at 9:50 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>
> On 28.02.2020 16:45, Paolo Abeni wrote:
> > The only users for such argument are the UDP protocol and the UNIX
> > socket family. We can safely reclaim the accounted memory directly
> > from the UDP code and, after the previous patch, we can do scm
> > stats accounting outside the datagram helpers.
> >
> > Overall this cleans up a bit some datagram-related helpers, and
> > avoids an indirect call per packet in the UDP receive path.
> >
> > v1 -> v2:
> > - call scm_stat_del() only when not peeking - Kirill
> > - fix build issue with CONFIG_INET_ESPINTCP
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> Reviewed-by: Kirill Tkhai <ktkhai@virtuozzo.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Nice cleanup, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2 0/2] net: cleanup datagram receive helpers
2020-02-28 13:45 [PATCH net-next v2 0/2] net: cleanup datagram receive helpers Paolo Abeni
2020-02-28 13:45 ` [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
2020-02-28 13:45 ` [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
@ 2020-02-28 20:13 ` David Miller
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2020-02-28 20:13 UTC (permalink / raw)
To: pabeni; +Cc: netdev, willemdebruijn.kernel, ktkhai
From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 28 Feb 2020 14:45:20 +0100
> Several receive helpers have an optional destructor argument, which uglify
> the code a bit and is taxed by retpoline overhead.
>
> This series refactor the code so that we can drop such optional argument,
> cleaning the helpers a bit and avoiding an indirect call in fast path.
>
> The first patch refactor a bit the caller, so that the second patch
> actually dropping the argument is more straight-forward
>
> v1 -> v2:
> - call scm_stat_del() only when not peeking - Kirill
> - fix build issue with CONFIG_INET_ESPINTCP
Series applie, thanks Paolo.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-28 20:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 13:45 [PATCH net-next v2 0/2] net: cleanup datagram receive helpers Paolo Abeni
2020-02-28 13:45 ` [PATCH net-next v2 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
2020-02-28 14:43 ` Kirill Tkhai
2020-02-28 13:45 ` [PATCH net-next v2 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
2020-02-28 14:50 ` Kirill Tkhai
2020-02-28 16:10 ` Willem de Bruijn
2020-02-28 20:13 ` [PATCH net-next v2 0/2] net: cleanup datagram receive helpers 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.