Re: [PATCH mptcp-next 1/3] mptcp: optimize release_cb for the common case

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Fri, 18 Mar 2022, Paolo Abeni wrote:

> The mptcp release callback checks several flags in atomic
> context, but only MPTCP_CLEAN_UNA can be up frequently.
>
> Reorganize the code to avoid multiple conditionals in the
> most common scenarios.
>
> Additional clarify a related comment.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>

This one LGTM:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fbb14dfe62b3..bdba1ddee2a7 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3118,15 +3118,17 @@ static void mptcp_release_cb(struct sock *sk)
> 		spin_lock_bh(&sk->sk_lock.slock);
> 	}
>
> -	/* be sure to set the current sk state before tacking actions
> -	 * depending on sk_state
> -	 */
> -	if (__test_and_clear_bit(MPTCP_CONNECTED, &msk->cb_flags))
> -		__mptcp_set_connected(sk);
> 	if (__test_and_clear_bit(MPTCP_CLEAN_UNA, &msk->cb_flags))
> 		__mptcp_clean_una_wakeup(sk);
> -	if (__test_and_clear_bit(MPTCP_ERROR_REPORT, &msk->cb_flags))
> -		__mptcp_error_report(sk);
> +	if (unlikely(&msk->cb_flags)) {
> +		/* be sure to set the current sk state before tacking actions
> +		 * depending on sk_state, that is processing MPTCP_ERROR_REPORT
> +		 */
> +		if (__test_and_clear_bit(MPTCP_CONNECTED, &msk->cb_flags))
> +			__mptcp_set_connected(sk);
> +		if (__test_and_clear_bit(MPTCP_ERROR_REPORT, &msk->cb_flags))
> +			__mptcp_error_report(sk);
> +	}
>
> 	__mptcp_update_rmem(sk);
> }
> -- 
> 2.35.1
>
>
>

--
Mat Martineau
Intel