netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net: cleanup datagram receive helpers
@ 2020-02-27 11:30 Paolo Abeni
  2020-02-27 11:30 ` [PATCH net-next 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
  0 siblings, 2 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-02-27 11:30 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

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     | 27 ++++++++++-----------------
 5 files changed, 28 insertions(+), 52 deletions(-)

-- 
2.21.1


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] unix: uses an atomic type for scm files accounting
  2020-02-27 11:30 [PATCH net-next 0/2] net: cleanup datagram receive helpers Paolo Abeni
@ 2020-02-27 11:30 ` Paolo Abeni
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
  1 sibling, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-02-27 11:30 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] 8+ messages in thread
* [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 11:30 [PATCH net-next 0/2] net: cleanup datagram receive helpers Paolo Abeni
  2020-02-27 11:30 ` [PATCH net-next 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
@ 2020-02-27 11:30 ` Paolo Abeni
  2020-02-27 12:31   ` Kirill Tkhai
                     ` (3 more replies)
  1 sibling, 4 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-02-27 11:30 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.

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     |  6 ++++--
 4 files changed, 21 insertions(+), 36 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..194e7b93e404 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2102,9 +2102,11 @@ 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) {
+			scm_stat_del(sk, skb);
 			break;
+		}
 
 		mutex_unlock(&u->iolock);
 
-- 
2.21.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
@ 2020-02-27 12:31   ` Kirill Tkhai
  2020-02-27 14:36     ` Paolo Abeni
  2020-02-27 19:51   ` kbuild test robot
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Kirill Tkhai @ 2020-02-27 12:31 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: David S. Miller, Willem de Bruijn

On 27.02.2020 14:30, 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.
> 
> 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     |  6 ++++--
>  4 files changed, 21 insertions(+), 36 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..194e7b93e404 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2102,9 +2102,11 @@ 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) {
> +			scm_stat_del(sk, skb);

Shouldn't we care about MSG_PEEK here?

>  			break;
> +		}
>  
>  		mutex_unlock(&u->iolock);
>  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 12:31   ` Kirill Tkhai
@ 2020-02-27 14:36     ` Paolo Abeni
  0 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-02-27 14:36 UTC (permalink / raw)
  To: Kirill Tkhai, netdev; +Cc: David S. Miller, Willem de Bruijn

On Thu, 2020-02-27 at 15:31 +0300, Kirill Tkhai wrote:
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index 145a3965341e..194e7b93e404 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> > @@ -2102,9 +2102,11 @@ 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) {
> > +			scm_stat_del(sk, skb);
> 
> Shouldn't we care about MSG_PEEK here?

Thank you for checking this! You are right, I'll fix in the next
iteration.

Cheers,

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
  2020-02-27 12:31   ` Kirill Tkhai
@ 2020-02-27 19:51   ` kbuild test robot
  2020-02-27 22:19   ` kbuild test robot
  2020-02-27 23:29   ` kbuild test robot
  3 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2020-02-27 19:51 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: kbuild-all, netdev

[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]

Hi Paolo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.6-rc3 next-20200227]
[cannot apply to ipvs/master sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-cleanup-datagram-receive-helpers/20200227-214333
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 2b99e54b30ed56201dedd91e6049ed83aa9d2302
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net//xfrm/espintcp.c: In function 'espintcp_recvmsg':
>> net//xfrm/espintcp.c:103:8: error: too many arguments to function '__skb_recv_datagram'
     skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
           ^~~~~~~~~~~~~~~~~~~
   In file included from include/linux/tcp.h:17:0,
                    from include/net/tcp.h:20,
                    from net//xfrm/espintcp.c:2:
   include/linux/skbuff.h:3523:17: note: declared here
    struct sk_buff *__skb_recv_datagram(struct sock *sk,
                    ^~~~~~~~~~~~~~~~~~~

vim +/__skb_recv_datagram +103 net//xfrm/espintcp.c

e27cca96cd68fa Sabrina Dubroca 2019-11-25   91  
e27cca96cd68fa Sabrina Dubroca 2019-11-25   92  static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
e27cca96cd68fa Sabrina Dubroca 2019-11-25   93  			    int nonblock, int flags, int *addr_len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25   94  {
e27cca96cd68fa Sabrina Dubroca 2019-11-25   95  	struct espintcp_ctx *ctx = espintcp_getctx(sk);
e27cca96cd68fa Sabrina Dubroca 2019-11-25   96  	struct sk_buff *skb;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   97  	int err = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   98  	int copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   99  	int off = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  100  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  101  	flags |= nonblock ? MSG_DONTWAIT : 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  102  
e27cca96cd68fa Sabrina Dubroca 2019-11-25 @103  	skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  104  	if (!skb)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  105  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  106  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  107  	copied = len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  108  	if (copied > skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  109  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  110  	else if (copied < skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  111  		msg->msg_flags |= MSG_TRUNC;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  112  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  113  	err = skb_copy_datagram_msg(skb, 0, msg, copied);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  114  	if (unlikely(err)) {
e27cca96cd68fa Sabrina Dubroca 2019-11-25  115  		kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  116  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  117  	}
e27cca96cd68fa Sabrina Dubroca 2019-11-25  118  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  119  	if (flags & MSG_TRUNC)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  120  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  121  	kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  122  	return copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  123  }
e27cca96cd68fa Sabrina Dubroca 2019-11-25  124  

:::::: The code at line 103 was first introduced by commit
:::::: e27cca96cd68fa2c6814c90f9a1cfd36bb68c593 xfrm: add espintcp (RFC 8229)

:::::: TO: Sabrina Dubroca <sd@queasysnail.net>
:::::: CC: Steffen Klassert <steffen.klassert@secunet.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52815 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
  2020-02-27 12:31   ` Kirill Tkhai
  2020-02-27 19:51   ` kbuild test robot
@ 2020-02-27 22:19   ` kbuild test robot
  2020-02-27 23:29   ` kbuild test robot
  3 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2020-02-27 22:19 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: kbuild-all, netdev

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master v5.6-rc3 next-20200227]
[cannot apply to ipvs/master sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-cleanup-datagram-receive-helpers/20200227-214333
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 2b99e54b30ed56201dedd91e6049ed83aa9d2302
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-173-ge0787745-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> net/xfrm/espintcp.c:103:34: sparse: sparse: too many arguments for function __skb_recv_datagram

vim +103 net/xfrm/espintcp.c

e27cca96cd68fa Sabrina Dubroca 2019-11-25   91  
e27cca96cd68fa Sabrina Dubroca 2019-11-25   92  static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
e27cca96cd68fa Sabrina Dubroca 2019-11-25   93  			    int nonblock, int flags, int *addr_len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25   94  {
e27cca96cd68fa Sabrina Dubroca 2019-11-25   95  	struct espintcp_ctx *ctx = espintcp_getctx(sk);
e27cca96cd68fa Sabrina Dubroca 2019-11-25   96  	struct sk_buff *skb;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   97  	int err = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   98  	int copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   99  	int off = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  100  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  101  	flags |= nonblock ? MSG_DONTWAIT : 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  102  
e27cca96cd68fa Sabrina Dubroca 2019-11-25 @103  	skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  104  	if (!skb)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  105  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  106  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  107  	copied = len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  108  	if (copied > skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  109  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  110  	else if (copied < skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  111  		msg->msg_flags |= MSG_TRUNC;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  112  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  113  	err = skb_copy_datagram_msg(skb, 0, msg, copied);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  114  	if (unlikely(err)) {
e27cca96cd68fa Sabrina Dubroca 2019-11-25  115  		kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  116  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  117  	}
e27cca96cd68fa Sabrina Dubroca 2019-11-25  118  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  119  	if (flags & MSG_TRUNC)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  120  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  121  	kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  122  	return copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  123  }
e27cca96cd68fa Sabrina Dubroca 2019-11-25  124  

:::::: The code at line 103 was first introduced by commit
:::::: e27cca96cd68fa2c6814c90f9a1cfd36bb68c593 xfrm: add espintcp (RFC 8229)

:::::: TO: Sabrina Dubroca <sd@queasysnail.net>
:::::: CC: Steffen Klassert <steffen.klassert@secunet.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers
  2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
                     ` (2 preceding siblings ...)
  2020-02-27 22:19   ` kbuild test robot
@ 2020-02-27 23:29   ` kbuild test robot
  3 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2020-02-27 23:29 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: kbuild-all, clang-built-linux, netdev

[-- Attachment #1: Type: text/plain, Size: 4209 bytes --]

Hi Paolo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.6-rc3 next-20200227]
[cannot apply to ipvs/master sparc-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/net-cleanup-datagram-receive-helpers/20200227-214333
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 2b99e54b30ed56201dedd91e6049ed83aa9d2302
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (git://gitmirror/llvm_project 949134e2fefd34a38ed71de90dffe2300e2e1139)
reproduce:
        # FIXME the reproduce steps for clang is not ready yet

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/xfrm/espintcp.c:103:68: error: too many arguments to function call, expected 5, have 6
           skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
                 ~~~~~~~~~~~~~~~~~~~                                         ^~~~
   include/linux/skbuff.h:3523:17: note: '__skb_recv_datagram' declared here
   struct sk_buff *__skb_recv_datagram(struct sock *sk,
                   ^
   1 error generated.

vim +103 net/xfrm/espintcp.c

e27cca96cd68fa Sabrina Dubroca 2019-11-25   91  
e27cca96cd68fa Sabrina Dubroca 2019-11-25   92  static int espintcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
e27cca96cd68fa Sabrina Dubroca 2019-11-25   93  			    int nonblock, int flags, int *addr_len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25   94  {
e27cca96cd68fa Sabrina Dubroca 2019-11-25   95  	struct espintcp_ctx *ctx = espintcp_getctx(sk);
e27cca96cd68fa Sabrina Dubroca 2019-11-25   96  	struct sk_buff *skb;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   97  	int err = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   98  	int copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25   99  	int off = 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  100  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  101  	flags |= nonblock ? MSG_DONTWAIT : 0;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  102  
e27cca96cd68fa Sabrina Dubroca 2019-11-25 @103  	skb = __skb_recv_datagram(sk, &ctx->ike_queue, flags, NULL, &off, &err);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  104  	if (!skb)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  105  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  106  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  107  	copied = len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  108  	if (copied > skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  109  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  110  	else if (copied < skb->len)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  111  		msg->msg_flags |= MSG_TRUNC;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  112  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  113  	err = skb_copy_datagram_msg(skb, 0, msg, copied);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  114  	if (unlikely(err)) {
e27cca96cd68fa Sabrina Dubroca 2019-11-25  115  		kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  116  		return err;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  117  	}
e27cca96cd68fa Sabrina Dubroca 2019-11-25  118  
e27cca96cd68fa Sabrina Dubroca 2019-11-25  119  	if (flags & MSG_TRUNC)
e27cca96cd68fa Sabrina Dubroca 2019-11-25  120  		copied = skb->len;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  121  	kfree_skb(skb);
e27cca96cd68fa Sabrina Dubroca 2019-11-25  122  	return copied;
e27cca96cd68fa Sabrina Dubroca 2019-11-25  123  }
e27cca96cd68fa Sabrina Dubroca 2019-11-25  124  

:::::: The code at line 103 was first introduced by commit
:::::: e27cca96cd68fa2c6814c90f9a1cfd36bb68c593 xfrm: add espintcp (RFC 8229)

:::::: TO: Sabrina Dubroca <sd@queasysnail.net>
:::::: CC: Steffen Klassert <steffen.klassert@secunet.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72233 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-02-28  1:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 11:30 [PATCH net-next 0/2] net: cleanup datagram receive helpers Paolo Abeni
2020-02-27 11:30 ` [PATCH net-next 1/2] unix: uses an atomic type for scm files accounting Paolo Abeni
2020-02-27 11:30 ` [PATCH net-next 2/2] net: datagram: drop 'destructor' argument from several helpers Paolo Abeni
2020-02-27 12:31   ` Kirill Tkhai
2020-02-27 14:36     ` Paolo Abeni
2020-02-27 19:51   ` kbuild test robot
2020-02-27 22:19   ` kbuild test robot
2020-02-27 23:29   ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).