Re: [PATCH mptcp-next v2 6/7] mptcp: cleanup mem accounting.

[Mat Martineau <martineau@kernel.org>]

On Fri, 6 Dec 2024, Paolo Abeni wrote:

> After the previous patch, updating sk_forward_memory is cheap and
> we can drop a lot of complexity from the MPTCP memory acconting,
> removing the custom fwd mem allocations for rmem.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
> - keep 'snd_una' and recovery-related fields under the msk
>   data lock
> - dropped unneeded code in __mptcp_move_skbs()
> ---
> net/mptcp/fastopen.c |   2 +-
> net/mptcp/protocol.c | 115 +++----------------------------------------
> net/mptcp/protocol.h |   4 +-
> 3 files changed, 10 insertions(+), 111 deletions(-)
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index ad940cc1f26f..a0d46b69746d 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -278,7 +278,6 @@ struct mptcp_sock {
> 	u64		rcv_data_fin_seq;
> 	u64		bytes_retrans;
> 	u64		bytes_consumed;
> -	int		rmem_fwd_alloc;
> 	int		snd_burst;
> 	int		old_wspace;
> 	u64		recovery_snd_nxt;	/* in recovery mode accept up to this seq;
> @@ -293,7 +292,6 @@ struct mptcp_sock {
> 	u32		last_ack_recv;
> 	unsigned long	timer_ival;
> 	u32		token;
> -	int		rmem_released;
> 	unsigned long	flags;
> 	unsigned long	cb_flags;
> 	bool		recovery;		/* closing subflow write queue reinjected */
> @@ -384,7 +382,7 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
>  */
> static inline int __mptcp_rmem(const struct sock *sk)
> {
> -	return atomic_read(&sk->sk_rmem_alloc) - READ_ONCE(mptcp_sk(sk)->rmem_released);
> +	return atomic_read(&sk->sk_rmem_alloc);
> }

Hi Paolo -

Minor change: this helper is now exactly the same as sk_rmem_alloc_get(), 
so use that existing helper instead.

Thats actually the only suggestion I have for v2! Good to delete code and 
improve performance. I also think the release_cb approach on rx is a good 
idea because it could be further leveraged to help with redundant 
schedulers (our main problem there was trying to trigger sends on other 
subflows when we couldn't acquire the msk lock).

- Mat

>
> static inline int mptcp_win_from_space(const struct sock *sk, int space)
> -- 
> 2.45.2
>
>
>