forked from mirrors/linux
		
	af_unix: Don't call skb_get() for OOB skb.
Since introduced, OOB skb holds an additional reference count with no special reason and caused many issues. Also, kfree_skb() and consume_skb() are used to decrement the count, which is confusing. Let's drop the unnecessary skb_get() in queue_oob() and corresponding kfree_skb(), consume_skb(), and skb_unref(). Now unix_sk(sk)->oob_skb is just a pointer to skb in the receive queue, so special handing is no longer needed in GC. Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> Link: https://patch.msgid.link/20240816233921.57800-1-kuniyu@amazon.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
		
							parent
							
								
									2862c9349d
								
							
						
					
					
						commit
						8594d9b85c
					
				
					 2 changed files with 7 additions and 36 deletions
				
			
		|  | @ -693,10 +693,7 @@ static void unix_release_sock(struct sock *sk, int embrion) | |||
| 	unix_state_unlock(sk); | ||||
| 
 | ||||
| #if IS_ENABLED(CONFIG_AF_UNIX_OOB) | ||||
| 	if (u->oob_skb) { | ||||
| 		kfree_skb(u->oob_skb); | ||||
| 		u->oob_skb = NULL; | ||||
| 	} | ||||
| 	u->oob_skb = NULL; | ||||
| #endif | ||||
| 
 | ||||
| 	wake_up_interruptible_all(&u->peer_wait); | ||||
|  | @ -2226,13 +2223,9 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other | |||
| 	} | ||||
| 
 | ||||
| 	maybe_add_creds(skb, sock, other); | ||||
| 	skb_get(skb); | ||||
| 
 | ||||
| 	scm_stat_add(other, skb); | ||||
| 
 | ||||
| 	spin_lock(&other->sk_receive_queue.lock); | ||||
| 	if (ousk->oob_skb) | ||||
| 		consume_skb(ousk->oob_skb); | ||||
| 	WRITE_ONCE(ousk->oob_skb, skb); | ||||
| 	__skb_queue_tail(&other->sk_receive_queue, skb); | ||||
| 	spin_unlock(&other->sk_receive_queue.lock); | ||||
|  | @ -2640,8 +2633,6 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state) | |||
| 
 | ||||
| 	if (!(state->flags & MSG_PEEK)) | ||||
| 		WRITE_ONCE(u->oob_skb, NULL); | ||||
| 	else | ||||
| 		skb_get(oob_skb); | ||||
| 
 | ||||
| 	spin_unlock(&sk->sk_receive_queue.lock); | ||||
| 	unix_state_unlock(sk); | ||||
|  | @ -2651,8 +2642,6 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state) | |||
| 	if (!(state->flags & MSG_PEEK)) | ||||
| 		UNIXCB(oob_skb).consumed += 1; | ||||
| 
 | ||||
| 	consume_skb(oob_skb); | ||||
| 
 | ||||
| 	mutex_unlock(&u->iolock); | ||||
| 
 | ||||
| 	if (chunk < 0) | ||||
|  | @ -2694,12 +2683,10 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk, | |||
| 			if (copied) { | ||||
| 				skb = NULL; | ||||
| 			} else if (!(flags & MSG_PEEK)) { | ||||
| 				if (sock_flag(sk, SOCK_URGINLINE)) { | ||||
| 					WRITE_ONCE(u->oob_skb, NULL); | ||||
| 					consume_skb(skb); | ||||
| 				} else { | ||||
| 				WRITE_ONCE(u->oob_skb, NULL); | ||||
| 
 | ||||
| 				if (!sock_flag(sk, SOCK_URGINLINE)) { | ||||
| 					__skb_unlink(skb, &sk->sk_receive_queue); | ||||
| 					WRITE_ONCE(u->oob_skb, NULL); | ||||
| 					unlinked_skb = skb; | ||||
| 					skb = skb_peek(&sk->sk_receive_queue); | ||||
| 				} | ||||
|  | @ -2710,10 +2697,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk, | |||
| 
 | ||||
| 		spin_unlock(&sk->sk_receive_queue.lock); | ||||
| 
 | ||||
| 		if (unlinked_skb) { | ||||
| 			WARN_ON_ONCE(skb_unref(unlinked_skb)); | ||||
| 			kfree_skb(unlinked_skb); | ||||
| 		} | ||||
| 		kfree_skb(unlinked_skb); | ||||
| 	} | ||||
| 	return skb; | ||||
| } | ||||
|  | @ -2756,7 +2740,6 @@ static int unix_stream_read_skb(struct sock *sk, skb_read_actor_t recv_actor) | |||
| 		unix_state_unlock(sk); | ||||
| 
 | ||||
| 		if (drop) { | ||||
| 			WARN_ON_ONCE(skb_unref(skb)); | ||||
| 			kfree_skb(skb); | ||||
| 			return -EAGAIN; | ||||
| 		} | ||||
|  |  | |||
|  | @ -337,18 +337,6 @@ static bool unix_vertex_dead(struct unix_vertex *vertex) | |||
| 	return true; | ||||
| } | ||||
| 
 | ||||
| static void unix_collect_queue(struct unix_sock *u, struct sk_buff_head *hitlist) | ||||
| { | ||||
| 	skb_queue_splice_init(&u->sk.sk_receive_queue, hitlist); | ||||
| 
 | ||||
| #if IS_ENABLED(CONFIG_AF_UNIX_OOB) | ||||
| 	if (u->oob_skb) { | ||||
| 		WARN_ON_ONCE(skb_unref(u->oob_skb)); | ||||
| 		u->oob_skb = NULL; | ||||
| 	} | ||||
| #endif | ||||
| } | ||||
| 
 | ||||
| static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist) | ||||
| { | ||||
| 	struct unix_vertex *vertex; | ||||
|  | @ -371,11 +359,11 @@ static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist | |||
| 				struct sk_buff_head *embryo_queue = &skb->sk->sk_receive_queue; | ||||
| 
 | ||||
| 				spin_lock(&embryo_queue->lock); | ||||
| 				unix_collect_queue(unix_sk(skb->sk), hitlist); | ||||
| 				skb_queue_splice_init(embryo_queue, hitlist); | ||||
| 				spin_unlock(&embryo_queue->lock); | ||||
| 			} | ||||
| 		} else { | ||||
| 			unix_collect_queue(u, hitlist); | ||||
| 			skb_queue_splice_init(queue, hitlist); | ||||
| 		} | ||||
| 
 | ||||
| 		spin_unlock(&queue->lock); | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Kuniyuki Iwashima
						Kuniyuki Iwashima