diff mbox series

[v1,net-next] af_unix: Don't check last_len in unix_stream_data_wait().

Message ID 20240530164256.40223-1-kuniyu@amazon.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v1,net-next] af_unix: Don't check last_len in unix_stream_data_wait(). | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 908 this patch: 908
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 906 this patch: 906
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 912 this patch: 912
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-05-31--03-00 (tests: 1040)

Commit Message

Kuniyuki Iwashima May 30, 2024, 4:42 p.m. UTC 
When commit 869e7c62486e ("net: af_unix: implement stream sendpage
support") added sendpage() support, data could be appended to the last
skb in the receiver's queue.

That's why we needed to check if the length of the last skb was changed
while waiting for new data in unix_stream_data_wait().

However, commit a0dbf5f818f9 ("af_unix: Support MSG_SPLICE_PAGES") and
commit 57d44a354a43 ("unix: Convert unix_stream_sendpage() to use
MSG_SPLICE_PAGES") refactored sendmsg(), and now data is always added
to a new skb.

Now we no longer need to check the length of the last skb, so let's
remove the dead logic in unix_stream_data_wait().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

Comments

Simon Horman June 1, 2024, 1:30 p.m. UTC | #1 
On Thu, May 30, 2024 at 09:42:56AM -0700, Kuniyuki Iwashima wrote:
> When commit 869e7c62486e ("net: af_unix: implement stream sendpage
> support") added sendpage() support, data could be appended to the last
> skb in the receiver's queue.
> 
> That's why we needed to check if the length of the last skb was changed
> while waiting for new data in unix_stream_data_wait().
> 
> However, commit a0dbf5f818f9 ("af_unix: Support MSG_SPLICE_PAGES") and
> commit 57d44a354a43 ("unix: Convert unix_stream_sendpage() to use
> MSG_SPLICE_PAGES") refactored sendmsg(), and now data is always added
> to a new skb.
> 
> Now we no longer need to check the length of the last skb, so let's
> remove the dead logic in unix_stream_data_wait().
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

...

> @@ -2744,8 +2738,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
>  
>  			mutex_unlock(&u->iolock);
>  
> -			timeo = unix_stream_data_wait(sk, timeo, last,
> -						      last_len, freezable);
> +			timeo = unix_stream_data_wait(sk, timeo, last, freezable);

Hi Iwashima-san,

A minor nit from my side. In the case that you have to reason perhaps
keep the line above to <= 80 columns wide.

...
diff mbox series

Patch

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e4af6616e1df..6b710043dd41 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2505,11 +2505,9 @@  static int unix_read_skb(struct sock *sk, skb_read_actor_t recv_actor)
  *	Sleep until more data has arrived. But check for races..
  */
 static long unix_stream_data_wait(struct sock *sk, long timeo,
-				  struct sk_buff *last, unsigned int last_len,
-				  bool freezable)
+				  struct sk_buff *last, bool freezable)
 {
 	unsigned int state = TASK_INTERRUPTIBLE | freezable * TASK_FREEZABLE;
-	struct sk_buff *tail;
 	DEFINE_WAIT(wait);
 
 	unix_state_lock(sk);
@@ -2517,9 +2515,7 @@  static long unix_stream_data_wait(struct sock *sk, long timeo,
 	for (;;) {
 		prepare_to_wait(sk_sleep(sk), &wait, state);
 
-		tail = skb_peek_tail(&sk->sk_receive_queue);
-		if (tail != last ||
-		    (tail && tail->len != last_len) ||
+		if (skb_peek_tail(&sk->sk_receive_queue) != last ||
 		    sk->sk_err ||
 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
 		    signal_pending(current) ||
@@ -2671,7 +2667,6 @@  static int unix_stream_read_generic(struct unix_stream_read_state *state,
 	long timeo;
 	int skip;
 	size_t size = state->size;
-	unsigned int last_len;
 
 	if (unlikely(sk->sk_state != TCP_ESTABLISHED)) {
 		err = -EINVAL;
@@ -2710,7 +2705,6 @@  static int unix_stream_read_generic(struct unix_stream_read_state *state,
 			goto unlock;
 		}
 		last = skb = skb_peek(&sk->sk_receive_queue);
-		last_len = last ? last->len : 0;
 
 again:
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
@@ -2744,8 +2738,7 @@  static int unix_stream_read_generic(struct unix_stream_read_state *state,
 
 			mutex_unlock(&u->iolock);
 
-			timeo = unix_stream_data_wait(sk, timeo, last,
-						      last_len, freezable);
+			timeo = unix_stream_data_wait(sk, timeo, last, freezable);
 
 			if (signal_pending(current)) {
 				err = sock_intr_errno(timeo);
@@ -2763,7 +2756,7 @@  static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		while (skip >= unix_skb_len(skb)) {
 			skip -= unix_skb_len(skb);
 			last = skb;
-			last_len = skb->len;
+
 			skb = skb_peek_next(skb, &sk->sk_receive_queue);
 			if (!skb)
 				goto again;
@@ -2854,7 +2847,7 @@  static int unix_stream_read_generic(struct unix_stream_read_state *state,
 
 			skip = 0;
 			last = skb;
-			last_len = skb->len;
+
 			unix_state_lock(sk);
 			skb = skb_peek_next(skb, &sk->sk_receive_queue);
 			if (skb)