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

[Matthieu Baerts <matttbe@kernel.org>]

Hi Paolo,

Thank you for your reply!

On 12/11/2025 10:04, Paolo Abeni wrote:
> 
> On 11/11/25 6:38 PM, Matthieu Baerts wrote:
>> 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.

(...)

>>> 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
> 
> Right you are!
>
>> 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.
>
> Technically is a fix. Even prior to the backlog introduction bad thing
> could happen, as passive msk does not have memory by the subflow only
> accounted.
> 
> TCP-level OoO could potentially use a lot of system memory that will not
> be memaccounted.
> 
> Unfortunately it's not easy to have a clean net patch.
> 
> I suppose with can have this as net-next fixes (including the fixes tag
> in 3/3) as the change is invasive and the thing is broken since the
> beginning.
> 
> WDYT?

It sounds good to me. When these patches will be in Linus tree, we can
ask to backport these 3 patches from this series, plus commit
4a997d49d92a ("tcp: Save lock_sock() for memcg in inet_csk_accept()."), no?

If yes, do you think I can add a "Cc: stable", and explaining in the
comments that it is because it depends on a patch that is only in
net-next, and the fix is not urgent?

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