Re: [PATCH v2 net] tcp/dccp: Don't use timer_pending() in reqsk_queue_unlink().

[Martin KaFai Lau <martin.lau@linux.dev>]

On 10/9/24 10:42 AM, Kuniyuki Iwashima wrote:
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 2c5632d4fddb..23cff5278a64 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1045,12 +1045,13 @@ static bool reqsk_queue_unlink(struct request_sock *req)
>   		found = __sk_nulls_del_node_init_rcu(sk);
>   		spin_unlock(lock);
>   	}
> -	if (timer_pending(&req->rsk_timer) && del_timer_sync(&req->rsk_timer))
> -		reqsk_put(req);
> +
>   	return found;
>   }
>   
> -bool inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req)
> +static bool __inet_csk_reqsk_queue_drop(struct sock *sk,
> +					struct request_sock *req,
> +					bool from_timer)
>   {
>   	bool unlinked = reqsk_queue_unlink(req);
>   
> @@ -1058,8 +1059,17 @@ bool inet_csk_reqsk_queue_drop(struct sock *sk, struct request_sock *req)
>   		reqsk_queue_removed(&inet_csk(sk)->icsk_accept_queue, req);
>   		reqsk_put(req);
>   	}
> +
> +	if (!from_timer && timer_delete_sync(&req->rsk_timer))

timer_delete_sync() is now done after the above reqsk_queue_removed().
The reqsk_timer_handler() may do the "req->num_timeout++" while the above 
reqsk_queue_removed() needs to check for req->num_timeout. Would it race?

Others lgtm. Thanks for the patch.

> +		reqsk_put(req);
> +
>   	return unlinked;
>   }