From 48851e9e802de3ede4ce81c3f7896c63e841adbc Mon Sep 17 00:00:00 2001 From: Jules Irenge Date: Sun, 23 Feb 2020 23:16:56 +0000 Subject: af_unix: Add missing annotation for unix_wait_for_peer() Sparse reports a warning unix_wait_for_peer() warning: context imbalance in unix_wait_for_peer() - unexpected unlock The root cause is the missing annotation at unix_wait_for_peer() Add the missing annotation __releases(&unix_sk(other)->lock) Signed-off-by: Jules Irenge Signed-off-by: David S. Miller --- net/unix/af_unix.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/unix/af_unix.c') diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 62c12cb5763e..cbd7dc01e147 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1207,6 +1207,7 @@ out: } static long unix_wait_for_peer(struct sock *other, long timeo) + __releases(&unix_sk(other)->lock) { struct unix_sock *u = unix_sk(other); int sched; -- cgit v1.2.3 From 7782040b950b5d0433f734fb2bba8b8b5ed6ce5a Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Fri, 28 Feb 2020 14:45:21 +0100 Subject: unix: uses an atomic type for scm files accounting 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 Reviewed-by: Kirill Tkhai Signed-off-by: David S. Miller --- include/net/af_unix.h | 2 +- net/unix/af_unix.c | 21 ++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) (limited to 'net/unix/af_unix.c') diff --git a/include/net/af_unix.h b/include/net/af_unix.h index e51d727cc3cd..f42fdddecd41 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 9d0518d9bdd4..c46fa271fc4a 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -690,7 +690,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)); } } #else @@ -1602,10 +1603,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) @@ -1613,10 +1612,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); } /* @@ -1805,10 +1802,8 @@ restart_locked: 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); @@ -1910,10 +1905,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; @@ -2409,9 +2402,7 @@ unlock: 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); } -- cgit v1.2.3 From e427cad6eee47e2daf207cd7a4156ae72496ee07 Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Fri, 28 Feb 2020 14:45:22 +0100 Subject: net: datagram: drop 'destructor' argument from several helpers 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 Reviewed-by: Kirill Tkhai Reviewed-by: Willem de Bruijn Signed-off-by: David S. Miller --- 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(-) (limited to 'net/unix/af_unix.c') 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 @@ done: 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 c46fa271fc4a..3385a7a0b231 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2106,9 +2106,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; -- cgit v1.2.3