Re: [PATCH mptcp-net 1/3] net: factor-out _sk_charge() helper

[Matthieu Baerts <matttbe@kernel.org>]

Hi Paolo,

On 07/11/2025 22:55, Paolo Abeni wrote:
> Move out of __inet_accept() the code dealing charging newly
> accepted socket to memcg. MPTCP will soon use it to on a per
> subflow basis, in different contexts.
> 
> No functional changes intended.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  include/net/sock.h |  2 ++
>  net/core/sock.c    | 18 ++++++++++++++++++
>  net/ipv4/af_inet.c | 17 +----------------
>  3 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index a5f36ea9d46f..38d48cfe0741 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1631,6 +1631,8 @@ static inline void sk_mem_uncharge(struct sock *sk, int size)
>  	sk_mem_reclaim(sk);
>  }
>  
> +void __sk_charge(struct sock *sk, gfp_t gfp);
> +
>  #if IS_ENABLED(CONFIG_PROVE_LOCKING) && IS_ENABLED(CONFIG_MODULES)
>  static inline void sk_owner_set(struct sock *sk, struct module *owner)
>  {
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 3b74fc71f51c..b26a6cdc9bcd 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -3448,6 +3448,24 @@ void __sk_mem_reclaim(struct sock *sk, int amount)
>  }
>  EXPORT_SYMBOL(__sk_mem_reclaim);
>  
> +void __sk_charge(struct sock *sk, gfp_t gfp)
> +{
> +	int amt;
> +
> +	gfp |= __GFP_NOFAIL;
> +	if (mem_cgroup_from_sk(sk)) {
> +		/* The socket has not been accepted yet, no need
> +		 * to look at newsk->sk_wmem_queued.
> +		 */
> +		amt = sk_mem_pages(sk->sk_forward_alloc +
> +				   atomic_read(&sk->sk_rmem_alloc));
> +		if (amt)
> +			mem_cgroup_sk_charge(sk, amt, gfp);
> +	}
> +
> +	kmem_cache_charge(sk, gfp);
> +}
> +
>  int sk_set_peek_off(struct sock *sk, int val)
>  {
>  	WRITE_ONCE(sk->sk_peek_off, val);
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index a31b94ce8968..08d811f11896 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -756,23 +756,8 @@ EXPORT_SYMBOL(inet_stream_connect);
>  void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *newsk)
>  {
>  	if (mem_cgroup_sockets_enabled) {
> -		gfp_t gfp = GFP_KERNEL | __GFP_NOFAIL;
> -
>  		mem_cgroup_sk_alloc(newsk);
> -
> -		if (mem_cgroup_from_sk(newsk)) {
> -			int amt;
> -
> -			/* The socket has not been accepted yet, no need
> -			 * to look at newsk->sk_wmem_queued.
> -			 */
> -			amt = sk_mem_pages(newsk->sk_forward_alloc +
> -					   atomic_read(&newsk->sk_rmem_alloc));
> -			if (amt)
> -				mem_cgroup_sk_charge(newsk, amt, gfp);
> -		}
> -
> -		kmem_cache_charge(newsk, gfp);

Mmh, this code has been moved from inet_csk_accept() to __inet_accept()
in net-next only, see commit 4a997d49d92a ("tcp: Save lock_sock() for
memcg in inet_csk_accept()."):

  https://lore.kernel.org/20251014235604.3057003-2-kuniyu@google.com

Should we only apply these patches in our export branch, for net-next?
If yes, then I guess we should remove the Fixes tag in patch 3/3.

> +		__sk_charge(newsk, GFP_KERNEL);
>  	}
>  
>  	sock_rps_record_flow(newsk);

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.